Throw meaningful exception when async JSInvokable method is called synchronously from JS#65880
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the developer experience for JS interop by detecting when an async [JSInvokable] method is invoked via the synchronous JS path (invokeMethod()), and throwing a clear InvalidOperationException instead of letting Task/ValueTask be serialized into misleading JSON-related errors.
Changes:
- Add sync-path detection for
Task,ValueTask, andValueTask<T>return values inDotNetDispatcher.Invoke(), throwing a targeted error message. - Add new unit tests covering sync invocation of
Task-returning andValueTask-returning[JSInvokable]methods. - Add a small test helper
[JSInvokable]method that returnsTask.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs | Adds sync-path check for async return types and throws a clearer exception before serialization. |
| src/JSInterop/Microsoft.JSInterop/test/Infrastructure/DotNetDispatcherTest.cs | Adds tests to validate the new exception behavior for Task and ValueTask sync invocation. |
bfacf3d to
f28aff8
Compare
…nchronously from JS When a [JSInvokable] method that returns Task or ValueTask is invoked synchronously from JavaScript via invokeMethod(), the raw Task/ValueTask object gets serialized, producing misleading JSON errors like "SerializeTypeInstanceNotSupported, System.Action". This makes debugging difficult as the error is unrelated to the actual issue. Add a check in DotNetDispatcher.Invoke() to detect async return types (Task, ValueTask, ValueTask<T>) and throw an InvalidOperationException with a clear message guiding the developer to use invokeMethodAsync(). Fixes dotnet#46811
|
Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. |
ilonatommy
left a comment
There was a problem hiding this comment.
Thank you for your contribution. A few optimization suggestions.
| return null; | ||
| } | ||
|
|
||
| // When an async [JSInvokable] method is called synchronously via invokeMethod(), the raw |
There was a problem hiding this comment.
Let's remove the comment, the code is self-explainatory.
| // When an async [JSInvokable] method is called synchronously via invokeMethod(), the raw | ||
| // Task/ValueTask is returned and serialization produces misleading JSON errors. Detect this | ||
| // and throw a clear exception instead. | ||
| if (syncResult is Task || syncResult is ValueTask) |
There was a problem hiding this comment.
| if (syncResult is Task || syncResult is ValueTask) | |
| if (result is Task or ValueTask | |
| || (result?.GetType() is { IsGenericType: true } resultType | |
| && resultType.GetGenericTypeDefinition() == typeof(ValueTask<>))) |
We can shorten it to one condition.
| targetInstance = jsRuntime.GetObjectReference(invocationInfo.DotNetObjectId); | ||
| } | ||
|
|
||
| var syncResult = InvokeSynchronously(jsRuntime, invocationInfo, targetInstance, argsJson); |
There was a problem hiding this comment.
In many cases we could even avoid invocation. If we added
private static bool IsAsyncMethod(MethodInfo methodInfo)
{
var returnType = methodInfo.ReturnType;
return typeof(Task).IsAssignableFrom(returnType)
|| returnType == typeof(ValueTask)
|| (returnType.IsGenericType
&& returnType.GetGenericTypeDefinition() == typeof(ValueTask<>));
}
it would cover the most frequent cases and avoid side effects of calling the method. For tricky constructions like
[JSInvokable]
public static object Tricky() => Task.CompletedTask;
we could still keep the check of syncResult. Then we extract throwing to one method and call it in case async is detected in return type or in syncResult.
Summary
Fixes #46811
When a
[JSInvokable]method that returnsTaskorValueTaskis invoked synchronously from JavaScript viainvokeMethod(), the raw Task/ValueTask object gets serialized, producing misleading JSON errors like:"SerializeTypeInstanceNotSupported, System.Action""ConstructorContainsNullParameterNames, System.Threading.Tasks.Task'1"These errors make debugging difficult as they are unrelated to the actual problem.
Changes
DotNetDispatcher.cs- In theInvoke()method (sync code path), added a check afterInvokeSynchronously()returns to detect async return types (Task,ValueTask,ValueTask<T>). When detected, throws anInvalidOperationExceptionwith a clear message:DotNetDispatcherTest.cs- Added 2 tests and 1 test helper:SyncInvokeOfAsyncMethod_Task_ThrowsMeaningfulExceptionSyncInvokeOfAsyncMethod_ValueTask_ThrowsMeaningfulExceptionInvokableAsyncReturningTask()- simple[JSInvokable]test methodTest results
All 148 JSInterop tests pass with 0 failures.
Test plan
SyncInvokeOfAsyncMethod_Task- verifies Task-returning methods throw clear errorSyncInvokeOfAsyncMethod_ValueTask- verifies ValueTask-returning methods throw clear error