Skip to content

fix(js): convert PolyglotException to JsEvaluationException at the engine boundary#3256

Draft
amanjeetsingh150 wants to merge 1 commit intomainfrom
fix-graaljs-polyglot-boundary
Draft

fix(js): convert PolyglotException to JsEvaluationException at the engine boundary#3256
amanjeetsingh150 wants to merge 1 commit intomainfrom
fix-graaljs-polyglot-boundary

Conversation

@amanjeetsingh150
Copy link
Copy Markdown
Collaborator

Summary

GraalJsEngine now catches PolyglotException at every context.eval() call site (the IIFE evaluation path and the bootstrap script) and rethrows a plain JsEvaluationException(JsScriptError). Live polyglot internals never escape the engine.

Why

PolyglotException exposes a polyglotStackTrace: List<StackFrame> property whose getLanguage() callback re-enters the originating PolyglotContext. If a caller retains the exception on a model that's later walked by a reflective serializer (Jackson and friends) after the engine has closed, that callback hits ShouldNotReachHere and the JVM dies.

Stripping the exception to plain Strings at the boundary is a one-place fix that closes this class of failure for every downstream consumer of JsEngine.evaluateScript — without anyone having to remember to do safe-extraction at every call site.

Shape of the new types

data class JsScriptError(
    val message: String,
    val causeMessage: String?,
    val sourceClass: String,
    val stackFrames: List<String>,   // formatted, not live frames
    val isHostException: Boolean,
    val isGuestException: Boolean,
)

class JsEvaluationException(val error: JsScriptError) : RuntimeException(error.message)

Resolved entirely while the engine is still open. After the constructor returns, the original PolyglotException can be released and the engine closed without affecting the wrapper.

Behaviour change for library users

Callers that previously caught PolyglotException from JsEngine.evaluateScript now catch JsEvaluationException. Broad catches (Throwable / Exception) are unaffected — the exception still propagates as a RuntimeException. The CLI's own error formatter (ErrorViewUtils) is not impacted; it falls through to e.stackTraceToString() either way.

Migration

// before
} catch (e: PolyglotException) { ... }

// after
} catch (e: JsEvaluationException) {
    // e.message — the JS error message
    // e.error.causeMessage — host cause message, if any
    // e.error.stackFrames — formatted strings, no live frames
}

Verify

  • ./gradlew test passes across all modules.
  • New tests in GraalJsEngineTest cover the wrapping behaviour, JsScriptError shape, and Jackson round-trip with the engine closed.
  • Existing sandboxing works test updated to expect the new exception type.

…gine boundary

Catch PolyglotException at every context.eval() call site in GraalJsEngine
(the IIFE evaluation path and the bootstrap script) and rethrow as
JsEvaluationException(JsScriptError). The new JsScriptError is a plain Kotlin
data class — message, causeMessage, sourceClass, stackFrames (List<String>),
isHostException, isGuestException — resolved while the engine is still open
and entirely detached from polyglot internals.

Why: PolyglotException's polyglotStackTrace property exposes live StackFrame
objects whose getLanguage() calls back into the originating PolyglotContext.
If a caller retains the exception on a model that is later serialized by a
reflective walker (Jackson, etc.) after the engine has closed, that callback
hits ShouldNotReachHere and crashes the JVM. Stripping the exception at the
engine boundary closes that class of failure for any downstream consumer of
JsEngine.evaluateScript.

Behaviour change: callers that previously caught PolyglotException from
JsEngine.evaluateScript now catch JsEvaluationException. Broad catches
(Throwable / Exception) are unaffected.

Tests: new round-trip test asserts JsScriptError serialises through Jackson
without touching polyglot fields, plus shape assertions on the wrapped error.
The existing sandboxing test is updated to expect the new exception type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant