davsclaus commented on code in PR #26309:
URL: https://github.com/apache/camel/pull/26309#discussion_r3988738733
##########
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:
Minor, but it made the probing confusing enough to be worth raising:
`isSyntaxError` matches any message containing `"Failed to compile JS code"`,
so a compile that fails for a reason *other* than bad syntax is reported to the
user as a syntax error in their script. With an engine left in a bad state I
saw a perfectly valid expression come back as:
```
org.apache.camel.ExpressionIllegalSyntaxException: Illegal syntax: body + 103
```
Narrowing the match to `SyntaxError` (the QuickJS-reported class), or
keeping the generic compile failure as an `ExpressionEvaluationException`,
would stop a runtime problem from being blamed on the route's script.
##########
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:
**Only a host-side trap poisons the runtime — a guest exception does not.**
I removed this `discard(state)` on your branch and re-probed: after `throw
new Error(...)`, after a `TypeError`, and after a statement-form `throw`, the
same engine happily re-executed a cached script *and* compiled a new one. Only
`camel.getHeader(null)` (host function throwing) and `call stack exhausted`
left it unusable.
Measured on this branch — failing evaluations of `body.nope.deeper`:
```
as written: fail 4,659 us (214 ops/s) 16x the
success path
discard only for non-GuestException: fail 357 us (2,794 ops/s) 1.3x
```
All 55 tests in the module pass with the narrower condition.
Note that narrowing it also means `exhausted(...)` is no longer reached when
the script throws — and a failed evaluation still grows the engine (~5.6 KB
each, measured on #26308). So the check needs to move into the `finally` at the
same time, which is the fix I suggested on #26308 as well:
```java
EngineState state = currentEngine();
Object result;
boolean discarded = false;
try {
byte[] compiled = state.compiled(script);
state.stderr.reset();
result = state.engine.invokePrecompiledGuestFunction(
QuickjsHelper.MODULE_NAME,
QuickjsHelper.FUNCTION_NAME,
List.of(bindings, ""),
compiled);
} catch (Exception e) {
// a guest exception leaves the QuickJS runtime usable; a
host-side trap does not
if (!(e instanceof GuestException)) {
discard(state);
discarded = true;
}
throw QuickjsHelper.wrapFailure(script, exchange, e);
} finally {
// Drop this evaluation's WASI stderr so a reused Engine
cannot accumulate it.
state.stderr.reset();
if (!discarded && state.exhausted(engineMaxMemory,
engineMaxEvaluations)) {
discard(state);
}
}
```
A test that drives a throwing script past `engineMaxEvaluations` would cover
both halves at once.
##########
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:
`camel.getBody()` passes `failOnUnsupported = false`, so a `StreamCache` or
`InputStream` body silently comes back as `null`. The `body` binding on the
very same exchange goes through `exchangeBindings(...)` with `true` and raises
the explicit "Streaming type ... cannot be exposed to camel-quickjs without
consuming the message body".
Same exchange, two different answers for the same value — a script using
`camel.getBody()` on a streamed body just sees `null` and has no idea why.
Worth making the two paths agree, or calling it out in the `camel` API table in
the docs.
--
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]
