Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blazor WebView exception handling behavior inconsistencies #2598

Closed
Eilon opened this issue Jun 11, 2021 · 8 comments
Closed

Blazor WebView exception handling behavior inconsistencies #2598

Eilon opened this issue Jun 11, 2021 · 8 comments
Assignees
Labels
area-blazor Blazor Hybrid / Desktop, BlazorWebView
Projects

Comments

@Eilon
Copy link
Member

Eilon commented Jun 11, 2021

There are likely inconsistencies and oddities in how various types of exceptions are handled in the various BlazorWebView implementations (or WebViewManagers).

We need to check the behavior from several different types of exceptions:

  • Un-awaited task throws
  • Render exception
  • Event handler exception
  • Others?

And the goal is to have them behave as they would if it were an otherwise-equivalent scenario implemented in native UI. There could be exception (ahem) to this, but in general we want to be consistent. For example, if exceptions should normally flow to AppDomain.UnhandledException, that's what should happen. If the platform offers its own "unhandled exception" pattern, we should support that.

@Eilon Eilon self-assigned this Jun 11, 2021
@ghost
Copy link

ghost commented Jun 14, 2021

Thanks for contacting us.

We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@Eilon Eilon transferred this issue from dotnet/aspnetcore Sep 20, 2021
@Eilon Eilon added the area-blazor Blazor Hybrid / Desktop, BlazorWebView label Sep 20, 2021
@jsuarezruiz jsuarezruiz added this to New in Triage via automation Oct 25, 2021
@jsuarezruiz jsuarezruiz moved this from New to Ready For Work in Triage Oct 25, 2021
@mkArtakMSFT mkArtakMSFT assigned TanayParikh and unassigned Eilon Jan 19, 2022
@TanayParikh
Copy link
Contributor

TanayParikh commented Jan 27, 2022

Repros:

Un-awaited task throws

<button @onclick="ThrowExceptionUnawaited">Un-awaited task throws</button>

@code {
    void ThrowExceptionUnawaited()
    {
        _ = ThrowExceptionAsync();
    }

    async Task ThrowExceptionAsync()
    {
        await Task.Delay(10);
        throw new Exception("This is some ***async*** exception.");
    }
}

Render exception

@{
    throw new Exception("Rendering exception.");
}

Event handler exception

<button @onclick="ThrowException">Throw Exception</button>

@code {
    void ThrowException()
    {
        throw new Exception("This is some unhandled exception.");
    }
}

@TanayParikh
Copy link
Contributor

iOS: First one does nothing, the other two throw an unhandled exception (yellow bar).

@Eilon is the goal of this issue to ensure the other platforms exhibit the same behavior as above? Or is the goal to ensure the two unhandled exceptions that yellow bar somehow flow through as NSException / NSError through the app stack?

As a side note, do we need to consider an error recovery mechanism short of closing the app and re-opening. Ex. refresh the webview kind of like what we do for Blazor server reconnect?

@Eilon
Copy link
Member Author

Eilon commented Jan 28, 2022

@TanayParikh the goal in general is that various kinds of unhandled exceptions should be obviously visible to the developer (e.g. crash the app, yellow bar, etc.), and also loggable, and perhaps logged by default using ILogger.

So if on iOS some case "does nothing" then that needs to be addressed. Surely it does something, but it's something invisible, so that's a problem.

I'm not sure you really need error recovery per se because if such an exception handles, the app is broken. The dev should have put in their own try/catch to handle the error (they do have a place to do that, right?) and possibly do a retry etc.

@TanayParikh
Copy link
Contributor

Thanks for the clarification Eilon.

So if on iOS some case "does nothing" then that needs to be addressed. Surely it does something, but it's something invisible, so that's a problem.

I definitely agree with the sentiment that unhandled exceptions should be visible to the dev. However, for the un-awaited async throw case specifically, the silent failure is the same behavior that Blazor WASM and even a vanilla C# console application exhibits. As such, I believe our current behavior is inline with developer expectations.

@Eilon
Copy link
Member Author

Eilon commented Jan 29, 2022

Ah, sure, I guess there are some cases where it's by-design invisible. But there was the scenario in #3695 (which I re-opened) where some errors that should definitely be visible, are somehow not visible at all.

@javiercn
Copy link
Member

javiercn commented Feb 1, 2022

@TanayParikh @Eilon based on discussions on #3695 I've captured more details on #4441 which should supersede this issue

@TanayParikh
Copy link
Contributor

Thanks @javiercn, I'm going to close this issue out in favor of #4441.

Triage automation moved this from Ready For Work to Done Feb 24, 2022
@mkArtakMSFT mkArtakMSFT added this to the 6.0.300-preview.14 milestone Mar 24, 2022
@dotnet dotnet locked as resolved and limited conversation to collaborators Apr 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Blazor Hybrid / Desktop, BlazorWebView
Projects
No open projects
Development

No branches or pull requests

4 participants