Skip to content

Developer exception page #5476

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

Closed
SteveSandersonMS opened this issue Feb 22, 2018 · 20 comments
Closed

Developer exception page #5476

SteveSandersonMS opened this issue Feb 22, 2018 · 20 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Feb 22, 2018

Currently if there are any unhandled client-side .NET exceptions, Mono logs them to the console (which is reasonable for Mono to do). But within Blazor it would be nice to display the exception details in a friendly, formatted way within the browser UI itself, perhaps looking like an ASP.NET server-side developer exception page.

In the previous Blazor prototype this was implemented as a non-dismissable full-screen overlay. The way you made it go away was to change your code which (via live reloading) triggers a page refresh. Update: That's not true. It only did that for compilation errors. We probably will do the same thing again when we have live reload, but that's totally separate to this.

As a separate work item, we should consider catching exceptions thrown by components during their known lifecycle methods (e.g., rendering). In this case we might choose not to display a full-screen error overlay, but instead display the affected component in some kind of "dead" state (e.g., exclamation mark icon). This is more suitable for production time errors where just because one component died doesn't mean you necessarily want the rest of your app to disappear.

@grahamehorner
Copy link
Contributor

It would be good to have this as a customisable page that follows the developer/application styling and possibly extended to post details of the client side exception back to a server api for logging/monitoring similar to application insights

@danielmeza
Copy link

@grahamehorner That will be wrate, in order to see the logs...
We can adopt the Loggin estratgy used in ASP.NET Core that way we can use build or custom Logger implementation.

@iAmBipinPaul
Copy link

in the case of server side Blazor also it does not show any exception on UI but it does show exception on terminal window , for example if required services are not register it will be better to show exceptions in browser as well.

for example in the case of default server side template , if we are removing
services.AddSingleton<WeatherForecastService>();
it does not show any exception on browser.

@aspnet-hello aspnet-hello transferred this issue from dotnet/blazor Dec 17, 2018
@aspnet-hello aspnet-hello added this to the Backlog milestone Dec 17, 2018
@aspnet-hello aspnet-hello added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one area-blazor Includes: Blazor, Razor Components labels Dec 17, 2018
@RemiBou
Copy link
Contributor

RemiBou commented Jan 9, 2019

If anyone is interested I found a workaround for handling exception : https://remibou.github.io/Exception-handling-in-Blazor/. It's not perfect as the extension point are minimal but still.

@pranavkm
Copy link
Contributor

Possible follow out from addressing #4047. Disposing components may throw and we allow it to bubble up. It would be useful to log and swallow these instead.

@mkArtakMSFT mkArtakMSFT removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels May 9, 2019
@rynowak
Copy link
Member

rynowak commented May 26, 2019

Closing this in favor of #8613 which has a more concrete proposal about how this would work.

@rynowak rynowak closed this as completed May 26, 2019
@rynowak rynowak added the ✔️ Resolution: Duplicate Resolved as a duplicate of another issue label May 26, 2019
@mkArtakMSFT
Copy link
Member

Reopening to use for tracking Developer exception page work.

@mkArtakMSFT mkArtakMSFT reopened this Aug 5, 2019
@mkArtakMSFT mkArtakMSFT modified the milestones: Backlog, 3.1.0-preview1 Aug 5, 2019
@mkArtakMSFT mkArtakMSFT added PRI: 1 - Required and removed ✔️ Resolution: Duplicate Resolved as a duplicate of another issue labels Aug 5, 2019
@danroth27 danroth27 mentioned this issue Sep 7, 2019
10 tasks
@mkArtakMSFT mkArtakMSFT changed the title Developer exception page Developer exception page for Server-side Blazor Sep 9, 2019
@mkArtakMSFT mkArtakMSFT changed the title Developer exception page for Server-side Blazor Developer exception page for server-side Blazor Sep 9, 2019
@mkArtakMSFT mkArtakMSFT changed the title Developer exception page for server-side Blazor Developer exception page Sep 9, 2019
@mkArtakMSFT mkArtakMSFT added the Needs: Design This issue requires design work before implementating. label Sep 9, 2019
@SteveSandersonMS
Copy link
Member Author

SteveSandersonMS commented Sep 19, 2019

@mkArtakMSFT I'll be providing some basic design notes for this, but am not assigning this issue to myself, because it's likely we'll need someone else to implement it.

@mrpmorris
Copy link

I'm not sure about the idea of not showing a single component of we encounter an unhandled exception.

Won't this mean we will be letting an app continue to run despite not knowing how that component may have put some shared state in an unknown and potentially unsafe state?

I don't think it's ever safe for a framework to decide it's safe to ignore an unexpected exception.

@SteveSandersonMS
Copy link
Member Author

SteveSandersonMS commented Sep 24, 2019

OK, so a lot of things have changed since this issue and #8613 were first written. Mainly, we're much better at getting the right exception info to the browser now (correctly redacted in the case of server-side Blazor in production). In cases I'm aware of, the right information already goes to the browser console now, so the remaining matter is whether to display some additional UI for it.

Thoughts

  1. I don't think it's our place to create a new error information experience within the browser. There's already a place for errors to be logged - the dev tools - and web devs have known for many years to look there. The dev tools also do a better job of displaying this info than we can (e.g., they can link to the relevant JS sources, and provide some means of correlating with failing network requests). It's standard, and not something we should try to replace.
  2. However,
    • In production, when a server-side Blazor app has its circuit terminated due to an unhandled exception, there should always be some UI to tell the user the app has stopped. We shouldn't expect developers to write code for that manually in every case. Some default UI makes sense.
    • In development, for both server and client Blazor (and even more so in Electron), it would be nice not to be running with the dev tools open constantly just so you can see if an exception occurred. Plus you want to get familiar with what UI will appear to real users if an unhandled exception occurs there, so no surprises in production.

Design proposal

I suggest a simplified version of @rynowak's original design. The main reason it can be simplified is that since then we've already solved various details about how error information gets from the server to the client.

At a high level, I think we should respond to unhandled errors just by displaying a predefined <div>. The contents, styling, and behavior of that element will be completely application defined. The template will include a simple one that makes sense in production and development, and is purely just a message to say an error occurred. Developers will get used to seeing that as a cue to open the dev tools for details.

Customizability:

  • Developers who don't want this at all can simply delete the predefined <div> from their host page, and the feature is totally gone

  • Developers who want to customize the message or styling just edit the template-provided markup/CSS. For example, you could put in a link to "Report this error to us" that goes off to somewhere that users can type in details of what they were doing when the problem occurred.

  • Developers who want to add more advanced features can do so through custom JS/CSS. We're not expressing any opinion on the right set of advanced features here.

    • Example 1: If you want to put in a checkbox for "Don't show this again", you'd add it to your markup, and write JS that responds to it being checked by setting a flag in localStorage, and also write some JS that does a DOM mutation observer to prevent the element being shown if the flag is set.
    • Example 2: If you want the element to display the exception details, you can write some JS that intercepts console.error.log calls to capture that info and set it on some child elements inside the <div> that we're showing.

    I know this extensibility relies on JS, but (1) I consider it unusual, almost all apps don't need to embed more functionality here, and (2) it can't be .NET because .NET isn't running after fatal errors.

Potential drawbacks:

  • Some apps get into a position where they routinely throw irrelevant and recoverable exceptions, and developers get used to ignoring those ones. For example, they might have some polling logic that terminates by throwing an unhandled TaskCancelledException when the user navigates away, and the developer doesn't consider that problematic. With this change, we'll start yellow-bar-ing them all the time.
    • They can either turn off the whole feature if they don't like it (see above), or they can fix their code
    • They won't be in this situation in server-side Blazor anyway, because for security we're much more aggressive about terminating on unhandled exceptions there.

Implementation suggestion

  • When an unhandled .NET exception occurs, meaning:

    • for server-side Blazor, when Boot.Server.ts's unhandledError function runs (which already gets called from the JS.Error handler)
    • for client-side Blazor, when Mono's Module.printErr function runs (see MonoPlatform.ts)
    • for Electron, we probably need to define something equivalent to the JS.Error handler

    ... then the JS-side code:

    • Evaluates document.querySelector('#error-ui'). If any such element is found, it sets thatElement.style.display = 'block';.
    • It also evaluates document.querySelectorAll('#error-ui .reload') and for any matching element(s), it adds a click event handler that triggers location.reload() and does evt.preventDefault(). Only add this handler once, not on each error.
    • It also evaluates document.querySelectorAll('#error-ui .dismiss') and for any matching element(s), it adds a click event handler that does .style.display = 'none' on #error-ui plus evt.preventDefault(). Only add this handler once, not on each error.
    • For server-side Blazor, we should not display the unhandled error UI and the reconnection UI at the same time. If we know there's been a fatal error, there's no point attempting to reconnect since we know it will always fail. I think our implementation should be able to distinguish these cases well enough, because it's the JS-side code that triggers the disconnection in the existing unhandledError code, so in that scenario it should know not to show the disconnection UI after this.
  • In the Blazor WebAssembly standalone template, in index.html, we add:

<div id="error-ui">
    An unhandled error has occurred.
    <a class='reload'>Reload</a>
    <a class='dismiss'>Dismiss</a>
</div>
  • In the ASP.NET Core-hosted templates (server-side Blazor, plus hosted WebAssembly), in _Host.cshtml, we add (just guessing the syntax here):
<div id="error-ui">
    <environment include="Staging,Production">
        An error has occurred. This application may no longer respond until reloaded.
    </environment>
    <environment include="Development">
        An unhandled exception has occurred. See browser dev tools for details.
    </environment>
    <a class='reload'>Reload</a>
    <a class='dismiss'>Dismiss</a>
</div>
  • We use CSS inside the template-provided site.css that:

    • Makes #error-ui appear as a yellow bar stuck to the bottom of the page and a high z-index, though of course with display: none initially.
    • Makes the .dismiss child element appear as a little "x" on the right
    • Possibly use ::before to insert a extra element with a data: background URL that displays a warning icon or similar

    This CSS needs to be simple enough for people to edit comfortably. We won't be able to service it directly, so any future changes will require back-compatibility and optional manual upgrade steps. I don't think this will be problematic because our requirements are sufficiently simple.

Notice that we do not also add a full-screen overlay that grays/whites-out the rest of the UI, because that would interfere with debugging.

The upgrade steps for people with 3.0 Blazor apps would be:

  • Add the <div id="error-ui">
  • Add the new CSS

For anyone who doesn't do this, they retain their existing behavior of not having the feature, because the runtime code won't find #error-ui.

@SteveSandersonMS
Copy link
Member Author

cc @rynowak for any opinions. Do you think this retains the usefulness of your original suggestion?

@ryanbrandenburg
Copy link
Contributor

@rynowak could you confirm this design so I can get cracking?

@rborosak
Copy link

Would it be possible to capture all unhandled exception in a central place so the developer could decide what to do next. Currently we can only intercept console.error but that is not of much use.
When an unhandled exception is thrown I would like to have a centralized place to capture (and handle?) it and do something.. Either display it in a popup, log it somewhere other then devtools, send it to the server so I'm aware that my useres are getting unhandled exceptions..

@rynowak
Copy link
Member

rynowak commented Sep 27, 2019

Sounds great, I think this is very thoughtful. Most of the feedback I thought of while reading this was already addressed.

@rynowak
Copy link
Member

rynowak commented Sep 27, 2019

Hi @rborosak - some answers

When an unhandled exception is thrown I would like to have a centralized place to capture (and handle?) it and do something..

We currently have no plans to provide a global way to silence exceptions. Blazor is stateful and it's important for security reasons that the user-agent can no longer send commands to the server if the program ends up in an unknown (faulted) state.

Either display it in a popup, log it somewhere other then devtools,

If I'm not mistake this feature addresses all of that. Let me know if I'm missing something.

send it to the server so I'm aware that my useres are getting unhandled exceptions..

Blazor already logs unhandled exceptions in the server logs. Is there something else you're looking for?

@rborosak
Copy link

rborosak commented Sep 27, 2019

@rynowak sorry... I'm talking about client side blazor.. when an unhandled exception is thrown, exception message and stack trace are printed in the console.. there is not much we can do with that.. I would like to have a global exception handler so I don't have to clutter my code with catch blocks

try{
DisplayLoadingIndicator = true;
await DoSomeWorkAsync();
}
finally{
DisplayLoadingIndicator = false;
}

If DoSomeWorkAsync throws an exception I would like to handle it in a global exception handler and based od that exception do something (send exception to server where I can store it to database, maybe redirect user somewhere, maybe display some generic message, maybe nothing just swallow (ignore) exception and continue with work, etc..) and Blazor (client) side would not be in unknow state and would go to finally block, set DisplayLoadingIndicator to false and rerender the page and hide the loading indicator..

EDIT:
This is the code from WebAssemblyRenderer that currently prints exceptions..

protected override void HandleException(Exception exception)
{
    Console.Error.WriteLine($"Unhandled exception rendering component:");
    if (exception is AggregateException aggregateException)
    {
        foreach (var innerException in aggregateException.Flatten().InnerExceptions)
        {
            Console.Error.WriteLine(innerException);
        }
    }
    else
    {
        Console.Error.WriteLine(exception);
    }
}

I would like to be able to provide my own implementation of HandleExcepetion..

WebAssemblyRenderer takes IServiceProvider as input parameter in constructor..
Maybe you could provide an IExceptionHandler interface with a HandleException method that would replace the code above.. That would give us the ability to replace the buildin implementation with our own

@SteveSandersonMS
Copy link
Member Author

@rborosak The idea of being able to continue after a throw (without there being any catch) would completely change and break the semantics of the C# language. It would wreck huge amounts of the .NET ecosystem by changing the fundamentals of exception behavior. We couldn't do it even if we wanted to, I'm afraid!

@SteveSandersonMS
Copy link
Member Author

Or perhaps you just mean you want the framework to implicitly put a try/catch around your OnInitializedAsync method (or similar).

If so, this is something you can do yourself in a base class. Create your own component base class that overrides OnInitializedAsync and calls your own virtual method to handle it. Then your base class can do a try/catch if you want, and that will be shared by all the components that inherit from it.

@SteveSandersonMS
Copy link
Member Author

In any case, the idea of a global exception handler is separate from this issue, so for any further discussion about it, could you please file a separate issue? Thanks!

@rborosak
Copy link

rborosak commented Oct 1, 2019

@SteveSandersonMS Thanks, I don't want to continue after a throw.. I'm looking for something like AppDomain.UnhandledException Event in WPF
AppDomain.UnhandledException Event

There is a separate issue opened here
https://github.com/aspnet/AspNetCore/issues/13452

@ryanbrandenburg ryanbrandenburg added Done This issue has been fixed and removed Needs: Design This issue requires design work before implementating. Working labels Oct 4, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests