Croway commented on code in PR #26309:
URL: https://github.com/apache/camel/pull/26309#discussion_r3989012038
##########
components/camel-quickjs/src/main/java/org/apache/camel/language/quickjs/QuickjsLanguage.java:
##########
@@ -104,40 +128,65 @@ private QuickjsExpression createQuickjsExpression(String
expression) {
* Evaluates {@code script} with optional {@code bindings} as JavaScript
function parameters. Binding names must be
* valid JavaScript identifiers; invalid names fail with {@link
ExpressionEvaluationException} rather than a raw
* JavaScript {@code SyntaxError}. Route expressions do not use this map —
they always bind {@code body},
- * {@code headers}, {@code properties}, and {@code exchangeId}.
+ * {@code headers}, {@code properties}, {@code exchangeId}, {@code
variables} and {@code exception}. There is no
+ * current exchange, so the {@code camel} API is not usable from this
entry point.
*/
@Override
public <T> T evaluate(String script, Map<String, Object> bindings,
Class<T> resultType) {
script = loadResource(script);
+ Map<String, Object> jsonBindings =
QuickjsHelper.toJsonCompatibleBindings(bindings, null);
+ EngineState state = currentEngine();
+ state.stderr.reset();
+ Object result;
try {
- Object result = eval(script,
QuickjsHelper.toJsonCompatibleBindings(bindings, null));
- return convert(result, resultType, getCamelContext(), null);
+ result = state.engine.invokeGuestFunction(
+ QuickjsHelper.MODULE_NAME,
+ QuickjsHelper.FUNCTION_NAME,
+ List.of(jsonBindings, script),
+ QuickjsHelper.EVAL_WRAPPER);
} catch (Exception e) {
+ discard(state);
throw QuickjsHelper.wrapFailure(script, null, e);
+ } finally {
+ state.stderr.reset();
}
+ return convert(result, resultType, getCamelContext(), null);
}
Object evaluateExpression(String script, Exchange exchange) {
+ Exchange previous = currentExchange.get();
+ currentExchange.set(exchange);
try {
- Object result = eval(script,
QuickjsHelper.exchangeBindings(exchange));
+ Map<String, Object> bindings =
QuickjsHelper.exchangeBindings(exchange);
+ EngineState state = currentEngine();
+ Object result;
+ try {
+ byte[] compiled = state.compiled(script);
+ state.stderr.reset();
+ result = state.engine.invokePrecompiledGuestFunction(
+ QuickjsHelper.MODULE_NAME,
+ QuickjsHelper.FUNCTION_NAME,
+ List.of(bindings, ""),
+ compiled);
+ if (state.exhausted(engineMaxMemory, engineMaxEvaluations)) {
+ discard(state);
+ }
+ } catch (Exception e) {
+ // a script that throws leaves the QuickJS runtime in an
undefined state (its next compile
+ // panics with "RefCell already borrowed"), so this thread
gets a fresh engine on its next use
+ discard(state);
+ throw QuickjsHelper.wrapFailure(script, exchange, e);
Review Comment:
Confirmed and applied in feb95873ac95. I probed each failure mode on the
branch and got your table: a JavaScript throw or TypeError surfaces as a
GuestException and the engine keeps compiling and running; a host function that
throws surfaces as the raw Java exception (a trap) and the next compile fails.
The discard is now selective (GuestException and a script that does not parse
keep the engine), the exhaustion check runs in the finally block for both the
route and the generic evaluate path, and the branch is rebased on #26308 so the
two land together. Tests: guestExceptionKeepsTheEngine (three JS failures,
engine kept, still compiles new scripts),
hostTrapDiscardsTheEngineAndTheNextEvaluationRecovers, and #26308's
throwingScriptsCountTowardsRecycling covers failure-then-recycle.
_Claude Code on behalf of Croway_
##########
components/camel-quickjs/src/main/java/org/apache/camel/language/quickjs/QuickjsHelper.java:
##########
@@ -114,15 +227,86 @@ private static GuestFunction evalFunction() {
return new GuestFunction(FUNCTION_NAME, List.of(Object.class,
String.class), Object.class);
}
+ /**
+ * The JavaScript library for one route expression: the script is
embedded, so QuickJS compiles it once per engine
+ * instead of on every evaluation. An expression is compiled as {@code
return (script)}; a script that is not a
+ * single expression (statements, a trailing semicolon) falls back to
{@code eval} so it keeps its completion value,
+ * exactly as before.
+ */
+ static String scriptLibrary(String script, boolean expressionForm) {
+ StringBuilder sb = new StringBuilder(CAMEL_FACADE);
+ sb.append("function __camelScript(").append(String.join(", ",
EXCHANGE_BINDINGS))
+ .append(", camel, java_invoke, quickjs4j_engine) {\n \"use
strict\";\n");
+ if (expressionForm) {
+ sb.append(" return (\n").append(script).append("\n );\n}\n");
+ } else {
+ sb.append(" return eval(__camelSource);\n}\n");
+ sb.append("const __camelSource =
").append(jsStringLiteral(script)).append(";\n");
+ }
+ sb.append("export function camelEval(b, s) {\n");
+ sb.append(indent(SANDBOX_ENTER, 1));
+ sb.append(" return __camelScript(");
+ for (String name : EXCHANGE_BINDINGS) {
+ sb.append("b.").append(name).append(", ");
+ }
+ sb.append("__camelFacade, __camel_stub_invoke, undefined);\n");
+ sb.append(indent(SANDBOX_EXIT, 1));
+ sb.append("}\n");
+ return sb.toString();
+ }
+
+ static byte[] compileScript(Engine engine, ByteArrayOutputStream stderr,
String script) {
+ stderr.reset();
+ try {
+ return engine.compilePortableGuestFunction(scriptLibrary(script,
true));
+ } catch (RuntimeException e) {
+ if (!isSyntaxError(e)) {
Review Comment:
Good catch; applied in feb95873ac95. QuickJS never puts the word SyntaxError
in a compile failure (it only reports "Failed to compile JS code"), so the
compile path now has its own check and, when both the expression and the
statement form fail to compile, it compiles a trivial function first: if that
fails too the engine is broken (IllegalStateException, which discards it),
otherwise the script is reported as ExpressionIllegalSyntaxException.
isSyntaxError itself now only matches SyntaxError for runtime errors. Covered
by hostTrapDiscardsTheEngineAndTheNextEvaluationRecovers (a valid script after
a trap is neither rejected nor blamed).
_Claude Code on behalf of Croway_
##########
components/camel-quickjs/src/main/java/org/apache/camel/language/quickjs/QuickjsHelper.java:
##########
@@ -96,13 +86,136 @@ export function camelEval(bindings, script) {
"protected", "public", "return", "static", "super", "switch",
"this", "throw", "true", "try", "typeof",
"var", "void", "while", "with", "yield");
+ /**
+ * The controlled Camel API. Each entry is one host function: its
JavaScript name, its parameter names (the arity is
+ * fixed so the JSON argument array always has every slot) and the Java
implementation, which receives the current
+ * {@link Exchange} and the JSON-decoded arguments.
+ */
+ private static final List<CamelFunction> CAMEL_API = List.of(
+ new CamelFunction(
+ "getBody", List.of(), Object.class,
+ (exchange, args) ->
toJsonCompatible(exchange.getMessage().getBody(), exchange, false)),
Review Comment:
Applied in feb95873ac95: camel.getBody() now uses the same rules as the body
binding, so a streaming body raises the explicit error instead of returning
null; the API table says so. Test streamingBodyIsAnErrorThroughTheApiAsWell
checks both paths on one exchange.
_Claude Code on behalf of Croway_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
