Update Jint to 4.15.3 and stop blocking the calling thread on promises (#7894)
* chore(javascript): update Jint to 4.15.3 and stop blocking on promises `Engine.Evaluate(...).UnwrapIfPromise()` blocks the calling thread while the engine's event loop drains, which is exactly the wrong thing to do inside an `async` method — an expression that awaits a .NET `Task`, such as one calling `getSecret()`, held a thread pool thread for the duration of the I/O. `Engine.EvaluateAsync` awaits the returned promise instead, and takes the cancellation token while it is at it. The Jint version is moved from 4.4.2 to 4.15.3. `EvaluateAsync` arrived in 4.14.0, but the pin lands past 4.15.2 deliberately: once expressions genuinely suspend and resume instead of draining the event loop on the calling thread, they exercise the async suspension machinery 4.15.2 corrected — an `await` on a right-hand side no longer stores the suspension sentinel, async generators and `for await...of` preserve loop iteration state across a suspension, and a suspension node is unwrapped correctly. Shipping the non-blocking change on an earlier 4.14/4.15 would enable exactly the code paths those releases fixed. One default changed along the way: since 4.14 `Interop.ArrayConversion` defaults to `LiveView`, so a CLR array reaches script as a live view over the original array rather than as a copy. That is observable — a script that sorts an array would now reorder the workflow's own array, and the value round-trips back as its original element type rather than as `object[]`. The evaluator therefore pins the previous `Copy` behaviour so the upgrade is not a behavioural change; hosts that prefer the live view can opt in through `JintOptions.ConfigureEngineOptions`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0179sA2T7HuRfRfSc2JirFik * test(javascript): pin the array copy lane and adopt JsString.Create The array audit. The parent commit fixes `ArrayConversion` to `Copy`, because 4.14 changed the default to `LiveView` and the two differ in behaviour a script can observe. The existing tests assert what a copy *produces*, which a live view also satisfies for a value nothing mutates; asking the engine how many conversions of each kind it performed (4.15.1's interop conversion counters) pins the lane itself. The second assertion is the more interesting one: an ordinary evaluation converts no CLR array at all, because Elsa converts collection-valued variables itself in `ObjectConverterHelper` long before Jint's array lane could see them. That makes the `ArrayConversion` setting a narrow compatibility pin rather than something every evaluation depends on. `JsString.Create` (public since Jint 4.15.3) is adopted in `JsonElementConverter`, where the string case was the only one still routed through `JsValue.FromObject` — re-entering the whole conversion pipeline, the registered object converters and this one included, to arrive at the same call the number and boolean cases beside it already make directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
30ab056745
commit
14173a19eb
|
|
@ -132,7 +132,7 @@
|
|||
<PackageVersion Include="Humanizer.Core" Version="3.0.10"/>
|
||||
<PackageVersion Include="IronCompress" Version="1.7.0"/>
|
||||
<PackageVersion Include="JetBrains.Annotations" Version="2025.2.4"/>
|
||||
<PackageVersion Include="Jint" Version="4.4.2"/>
|
||||
<PackageVersion Include="Jint" Version="4.15.3"/>
|
||||
<PackageVersion Include="JsonSchema.Net" Version="9.4.0"/>
|
||||
<PackageVersion Include="LinqKit.Core" Version="1.2.11"/>
|
||||
<PackageVersion Include="Loom" Version="0.0.1-preview.10"/>
|
||||
|
|
|
|||
|
|
@ -26,7 +26,11 @@ internal class JsonElementConverter : IObjectConverter
|
|||
{
|
||||
JsonValueKind.Object => JsValue.FromObject(engine, JsonObject.Create(element)),
|
||||
JsonValueKind.Array => JsValue.FromObject(engine, JsonArray.Create(element)),
|
||||
JsonValueKind.String => JsValue.FromObject(engine, element.GetString()),
|
||||
// JsString.Create is the counterpart of the JsNumber.Create and JsBoolean uses below: it produces the
|
||||
// string value directly, where JsValue.FromObject would re-enter the whole conversion pipeline — the
|
||||
// registered object converters, this one included, followed by the default converter's type switch —
|
||||
// to arrive at the same call. It became public in Jint 4.15.3.
|
||||
JsonValueKind.String => JsString.Create(element.GetString()!),
|
||||
JsonValueKind.Number => element.TryGetInt32(out var intValue) ? JsNumber.Create(intValue) : JsNumber.Create(element.GetDouble()),
|
||||
JsonValueKind.True => JsBoolean.True,
|
||||
JsonValueKind.False => JsBoolean.False,
|
||||
|
|
|
|||
|
|
@ -38,7 +38,7 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification
|
|||
{
|
||||
var engine = await GetConfiguredEngine(configureEngine, context, options, cancellationToken);
|
||||
await mediator.SendAsync(new EvaluatingJavaScript(engine, context, expression), cancellationToken);
|
||||
var result = ExecuteExpressionAndGetResult(engine, expression);
|
||||
var result = await ExecuteExpressionAndGetResultAsync(engine, expression, cancellationToken);
|
||||
await mediator.SendAsync(new EvaluatedJavaScript(engine, context, expression, result), cancellationToken);
|
||||
|
||||
return result.ConvertTo(returnType);
|
||||
|
|
@ -53,6 +53,12 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification
|
|||
ExperimentalFeatures = ExperimentalFeature.TaskInterop
|
||||
};
|
||||
|
||||
// Jint 4.14 changed this default to LiveView, which exposes a CLR array to script as a live view over
|
||||
// the original array rather than as a copy. Keeping the copy semantics means a script that mutates an
|
||||
// array does not reach back into the workflow's own data, and that an array survives a round trip as
|
||||
// object[] the way it always has. Hosts that want the live view can opt in via ConfigureEngineOptions.
|
||||
engineOptions.Interop.ArrayConversion = ArrayConversionMode.Copy;
|
||||
|
||||
ConfigureClrAccess(engineOptions);
|
||||
ConfigureObjectWrapper(engineOptions);
|
||||
ConfigureObjectConverters(engineOptions);
|
||||
|
|
@ -106,11 +112,14 @@ public class JintJavaScriptEvaluator(IConfiguration configuration, INotification
|
|||
engine.SetValue("getConfig", (Func<string, object?>)(name => configuration.GetSection(name).Value));
|
||||
}
|
||||
|
||||
private object? ExecuteExpressionAndGetResult(Engine engine, string expression)
|
||||
private async Task<object?> ExecuteExpressionAndGetResultAsync(Engine engine, string expression, CancellationToken cancellationToken)
|
||||
{
|
||||
var preparedScript = GetOrCreatePrepareScript(expression);
|
||||
var result = engine.Evaluate(preparedScript);
|
||||
return result.UnwrapIfPromise().ToObject();
|
||||
|
||||
// EvaluateAsync awaits a returned promise instead of blocking the calling thread on it, which matters
|
||||
// for expressions that await a .NET Task, such as the ones calling getSecret().
|
||||
var result = await engine.EvaluateAsync(preparedScript, cancellationToken);
|
||||
return result.ToObject();
|
||||
}
|
||||
|
||||
private Prepared<Script> GetOrCreatePrepareScript(string expression)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,87 @@
|
|||
using Elsa.Expressions.JavaScript.Contracts;
|
||||
using Elsa.Expressions.Models;
|
||||
using Elsa.Testing.Shared;
|
||||
using Jint;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Xunit;
|
||||
using Xunit.Abstractions;
|
||||
|
||||
namespace Elsa.JavaScript.IntegrationTests;
|
||||
|
||||
/// <summary>
|
||||
/// Pins how CLR arrays cross into JavaScript: a script sees a copy, so mutating it does not reach back into
|
||||
/// the workflow's own data, and the value round-trips as an <c>object[]</c>.
|
||||
/// </summary>
|
||||
public class ArrayConversionTests
|
||||
{
|
||||
private readonly IServiceProvider _serviceProvider;
|
||||
private readonly IJavaScriptEvaluator _evaluator;
|
||||
|
||||
public ArrayConversionTests(ITestOutputHelper testOutputHelper)
|
||||
{
|
||||
_serviceProvider = new TestApplicationBuilder(testOutputHelper).Build();
|
||||
_evaluator = _serviceProvider.GetRequiredService<IJavaScriptEvaluator>();
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "Sorting a CLR array from a script does not mutate the original array")]
|
||||
public async Task SortingAnArrayDoesNotMutateTheOriginal()
|
||||
{
|
||||
var numbers = new[] { 8.0, 4.0, 2.0 };
|
||||
|
||||
var result = await EvaluateAsync<object>("numbers.sort((a, b) => a - b); return numbers;", engine => engine.SetValue("numbers", numbers));
|
||||
|
||||
Assert.Equal([2.0, 4.0, 8.0], Assert.IsType<object[]>(result).Cast<double>());
|
||||
Assert.Equal([8.0, 4.0, 2.0], numbers);
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "A CLR array is readable from a script")]
|
||||
public async Task ArraysAreReadable()
|
||||
{
|
||||
var numbers = new[] { 8.0, 4.0, 2.0 };
|
||||
|
||||
Assert.Equal("14", await EvaluateAsync<string>("return '' + numbers.reduce((a, b) => a + b, 0);", engine => engine.SetValue("numbers", numbers)));
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "A CLR array crosses into a script through the copy lane, not the live view")]
|
||||
public async Task ArraysCrossThroughTheCopyLane()
|
||||
{
|
||||
// The tests above assert the behaviour a copy produces, which a live view happens to match for a value
|
||||
// nothing mutates. Asking the engine how many conversions of each kind it performed is what pins the
|
||||
// lane itself, so a future change of the ArrayConversion default cannot pass unnoticed.
|
||||
var numbers = new[] { 8.0, 4.0, 2.0 };
|
||||
Engine? engine = null;
|
||||
|
||||
await EvaluateAsync<double>("return numbers.length;", e =>
|
||||
{
|
||||
engine = e;
|
||||
e.SetValue("numbers", numbers);
|
||||
});
|
||||
|
||||
var diagnostics = engine!.Advanced.GetInteropConversionDiagnostics();
|
||||
|
||||
Assert.Equal(0L, diagnostics.ArrayLiveViewConversions);
|
||||
Assert.True(diagnostics.ArrayCopyConversions > 0, "the array should have crossed through the copy lane");
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "An expression that touches no CLR array converts none")]
|
||||
public async Task ExpressionsWithoutArraysConvertNothing()
|
||||
{
|
||||
// Elsa converts collection-valued workflow variables itself, in ObjectConverterHelper, so an ordinary
|
||||
// evaluation never reaches Jint's array-conversion lane at all. That is what makes the ArrayConversion
|
||||
// setting a narrow compatibility pin rather than something every evaluation depends on.
|
||||
Engine? engine = null;
|
||||
|
||||
await EvaluateAsync<double>("return 1 + 1;", e => engine = e);
|
||||
|
||||
var diagnostics = engine!.Advanced.GetInteropConversionDiagnostics();
|
||||
|
||||
Assert.Equal(0L, diagnostics.ArrayLiveViewConversions);
|
||||
Assert.Equal(0L, diagnostics.ArrayCopyConversions);
|
||||
}
|
||||
|
||||
private async Task<T?> EvaluateAsync<T>(string script, Action<Engine> configureEngine)
|
||||
{
|
||||
var context = new ExpressionExecutionContext(_serviceProvider, new());
|
||||
return (T?)await _evaluator.EvaluateAsync(script, typeof(T), context, configureEngine: configureEngine);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue