Skip to content

Fix JSInterop DefaultAsyncTimeout swallow issue#67013

Open
GOVINSAGA wants to merge 2 commits into
dotnet:mainfrom
GOVINSAGA:fix/jsinterop-timeout
Open

Fix JSInterop DefaultAsyncTimeout swallow issue#67013
GOVINSAGA wants to merge 2 commits into
dotnet:mainfrom
GOVINSAGA:fix/jsinterop-timeout

Conversation

@GOVINSAGA
Copy link
Copy Markdown

Fixes #21384.

Root Cause: When DefaultAsyncTimeout fires, the CancellationTokenSource triggers tcs.TrySetCanceled(), producing a task with IsCanceled = true. ComponentBase then swallows it in its catch blocks (since it ignores canceled tasks to handle normal component disposal).

My approach: Modify only the InvokeAsync overload that creates the DefaultAsyncTimeout CTS. Wrap the inner call in a try/catch that catches OperationCanceledException only when cts.IsCancellationRequested is true, and re-throws as a JSException.

Why this avoids the previous issues:

  • No breaking changes — the InvokeAsync(CancellationToken) overload is untouched
  • ComponentBase won't swallow it — JSException produces a faulted task (IsCanceled = false)
  • User-supplied tokens still behave as before
  • No new public API types required
  • No changes to ComponentBase.cs

@GOVINSAGA GOVINSAGA requested a review from a team as a code owner June 4, 2026 09:18
@github-actions github-actions Bot added the area-blazor Includes: Blazor, Razor Components label Jun 4, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Jun 4, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Thanks for your PR, @GOVINSAGA. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@javiercn
Copy link
Copy Markdown
Member

javiercn commented Jun 4, 2026

Hmm, not sure how I feel about this one.

This now means that if a JS interop call timed out, ComponentBase will throw instead of no-op, which might result in the session terminating (the circuit getting disposed).

Changing the exception is not a breaking change in the strict sense but its still a behavior breaking change. Anyone that used to do

try
{
}
catch (OperationCanceledException ex)
{
}

will now have to update to JSException. We might want to consider an AppCompat switch here just in case, but no need to do that right now.

That said, reading through #21384 (comment) (might want to include this on the description here next time) I think it's ok. I would suggest that for completeness we set the InnerException to the TCS, as that makes it easier to distinguish against another JSException cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Blazor] Throw meaningful exception for JSRuntime calls that timeout

2 participants