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

Global exception handling #13452

Closed
julienGrd opened this issue Aug 26, 2019 · 60 comments
Closed

Global exception handling #13452

julienGrd opened this issue Aug 26, 2019 · 60 comments
Labels
affected-most This issue impacts most of the customers area-blazor Includes: Blazor, Razor Components Components Big Rock This issue tracks a big effort which can span multiple issues enhancement This issue represents an ask for new feature or an enhancement to an existing one Priority:1 Work that is critical for the release, but we could probably ship without severity-major This label is used by an internal tool

Comments

@julienGrd
Copy link

Hello guys, I wanted to know what is the better way to catch exception at application level, to avoid put try catch in all my method and component, and centralize the exception management in one point (typically show an error notification and log the exception).

so there is a better way to do this in preview8, or something planned for the RTM ?

thanks !

@Eilon Eilon added the area-blazor Includes: Blazor, Razor Components label Aug 26, 2019
@pranavkm
Copy link
Contributor

All unhandled exceptions should end up in the Logger. If you're seeing otherwise, please let us know and we'd be happy to investigate. Looking at the logs should be the way to go here.

@pranavkm pranavkm added this to the Discussions milestone Aug 26, 2019
@julienGrd
Copy link
Author

OK, what i understand is i have to provide my own implementation of ILogger ? and its on my implementation i have to deal with the exception ? (for example show notification, write exception in a file for server side and send it to controller for client-side)
actually the only thing if found is this package https://github.com/BlazorExtensions/Logging
I don't really understand how to use it, it seem we can only log in the console with this.

But even with this extension, if there is an unhandled exception in the code, the app crash.

so if i understand, i don't have the choice to surround all of my code with try catch ?

thanks for your explanation

@pranavkm
Copy link
Contributor

Any of the logger providers listed here: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/logging/?view=aspnetcore-2.2#built-in-logging-providers as also community developed providers (https://github.com/aspnet/Extensions/tree/master/src/Logging#providers) should work.

But even with this extension, if there is an unhandled exception in the code, the app crash.

For server-side Blazor, an unhandled exception should disconnect the circuit, but not crash the application. If a block is meant to throw an exception you can tolerate, a try-catch seems like the most reasonable way to proceed. What do you plan on doing with unhandled exceptions if you do catch arbitrary exception?

@rborosak
Copy link

rborosak commented Sep 25, 2019

Client side blazor does not log any exceptions. They are just printed to console. Here is the code from WebAssemblyRenderer

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);
    }
}

Only way I found that can intercept exceptions is described here
https://remibou.github.io/Exception-handling-in-Blazor/

But I would not call this exception handling...

I would like to have a global exception handler so I could write my code without catch blocks...

try
{
   IsLoading = true;
   await SomeAsync();
}
finally
{
   IsLoading = false;
   //force rerender after exception
    StateHasChanged();
}

If SomeAsync method throws an exeption my IsLoading varialbe is set to false but my view is not rerendered. You have to call SetStateHasChanged in finally block to force view rerender

@julienGrd
Copy link
Author

julienGrd commented Sep 26, 2019

for client side blazor, i finally write my own implementation on ILogger (inspired by the code of default implementation which log in the console) .
With this implementation, i can call a LoggerController on my server, wich a ILogger is inject too (but this time, its a NLog implementation whish is inject, so i can properly write all exception on the server)

I abandon the idea of global exception handling, which seem not be the logic of blazor, and put try/catch everywhere and inject an ILogger in all component.

Except if you think there is better to do, you can close this issue.

@rborosak
Copy link

rborosak commented Sep 26, 2019

I think this is a missing feature.. When exception is thrown I would also like to have a centralized place to capture all unhandled exceptions and do something.. Either display it in a popup, log it somewhere, send it to the server or whatever.. without any harm to app state
Something like UnhandledException event in WPF
https://docs.microsoft.com/en-us/dotnet/api/system.appdomain.unhandledexception?redirectedfrom=MSDN&view=netframework-4.8

@SteveSandersonMS
Copy link
Member

Isn't the ILogger already that centralized place?

@rborosak
Copy link

rborosak commented Oct 1, 2019

@SteveSandersonMS look at my comment above
#13452 (comment)
Exceptions (in client side blazor) are written to console using Console.Error.WriteLine, they are not logged using ILogger.. You could implement a custom TextWriter and replace Console.Error
https://remibou.github.io/Exception-handling-in-Blazor/
but you get a string representation of an exception.. Parsing that string, which inludes a stack trace, and trying to get (for example) only an exception message is cumbersome to say at least.
I would like to get an access to exception object so I can do my own logging (or what ever)

@SteveSandersonMS
Copy link
Member

@rborosak Thanks for the clarification!

@hagaygo
Copy link

hagaygo commented Nov 14, 2019

Any update on this issue ?

Currently my sever side blazor app just stopped responding to user input after an exception is thrown.

Its not a good practice to cover all lines of code with try-catch (nor to expect the developer to do so) , in case of "un-predicted" exception we should have a generic handler that allows us :

1.Choose whether to break the circuit.
2.Show our user a "friendly" GUI message or just redirect him to generic error page.

Currently after un-handleded exception the app can not notify the user on a problem and the user can only re-run the app (close the browser and goto to the app URL again) or refresh the page and the loose all the state (which is sometimes good , but the app developer should choose this).

@Postlagerkarte
Copy link

@hagaygo Very good summary!

@SteveSandersonMS Please consider this for the 3.1.x milestone ! Please do not move this to 5.0.0

@bansalankit2601
Copy link

Hi,

I get this kind of error when the circuit breaks in the Console of Developer Window .

image

I dont think there anyway presently that can notify the user that an error has occurred after this in the screen and ask to refresh it without manually opening the developer window to check and refreshing the page

@HrDahl
Copy link

HrDahl commented Jan 30, 2020

@SteveSandersonMS Could you clarify wheter or not the app.UseExceptionHandler() works for server-side blazor?

I have seen several cases where my custom ErrorHandler does not catch exceptions being thrown by my application. Example code

Startup.cs:

public void Configure(IApplicationBuilder app, IWebHostEnvironment env, IServiceProvider serviceProvider)
{
    ...
    app.UseExceptionHandler(new ExceptionHandlerOptions { ExceptionHandler = ErrorHandler.HandleError });
    ...
}

ErrorHandler.cs:

public static async Task HandleError(HttpContext context)
{
    var error = context.Features.Get<IExceptionHandlerFeature>()?.Error;
    var message = error?.Message ?? "[EXCEPTION NOT FOUND]";        
    return;
}

An example are when my repository are throwing an exception as such:
The instance of entity type cannot be tracked because another instance with the same key value for {'Id'} is already being tracked

My MVC solution are catching all exceptions and it is using similar ErrorHandling implementations.

@mkArtakMSFT mkArtakMSFT added enhancement This issue represents an ask for new feature or an enhancement to an existing one and removed question labels Feb 13, 2020
@mkArtakMSFT
Copy link
Member

Moving this to 5.0 release to distill the specific asks here and see whether we can address some of these.

@gulbanana
Copy link

the biggest issue for me is error boundaries. i want to be able to use a third party component and, somehow, wrap my use of that component in error handling so that a bug in the library doesn’t kill my whole web application. right now there’s no “place to put the try catch” - the whole component tree is supervised by the blazor renderer, which will die if anything deep in the tree messes up.

@sylvainonweb
Copy link

+1 for this feature.
Having to put try/catch block in all methods isn't a reasonible way to go.

I personnaly used to throw exception to manage business errors. At a high level, I catch the exception and according to its type, I decide if the exception must be logged, displayed to the user and if displayed if I display the complete message (business exceptions) or just a "Something goes wrong").

Actually, the only way I found to do something like this is to create a ComponentBaseEx class which inherit to ComponentBase and override OnInitialized(Async), OnParametersSet(Async), ...
Unfortunately, even with this way to go, I have to manage manually all user's actions (delete a customer, ...)

Do you think

  1. something can be done ?
  2. if yes, when ?

@mikefayer
Copy link

Here's my scenario: Blazor desktop/hybrid app using Blazor server. One user logged in at a time and state persists across reloads. So, if there's an unhandled exception, there's no way to get the app back to a trustworthy state other than restarting it. We need to catch unhandled exceptions, show appropriate UX, and then shut down. Standard desktop-app pattern.

Temporary hack: we have logging set up with Serilog. Serilog has an API that lets you filter all log messages and decide whether they are emitted. We abuse this API by checking whether a log event has a CircuitId key in its properties-dictionary. If it does, that means a circuit has died - ie., there's an unhandled exception. We can get the offending exception out of the log message object that we examined. Presto - you have your hands on the unhandled exception at the very top and can do whatever you want with it.

@julienGrd
Copy link
Author

Hello guys, like the others, i'm realy worried if you move this to the .NET 5 milestone.
After one year of work on blazor by rewriting an old silverlight app, i'm suppose to deploy my app in production in may/june (when webasssembly will be production ready).

But i definitively can't do that without a strong way to catch or at least log all unexpected errors.

Please consider make something, even not perfect, before the 5.0 release (juste a method with the exception is OK for me, at least we can log...)

Thans you very much !

@eliudgerardo
Copy link

Hello, I'm in the same situation that @julienGrd
Please do something to help/guide us.
Regards!

@bansalankit2601
Copy link

bansalankit2601 commented Mar 3, 2020 via email

@SteveSandersonMS
Copy link
Member

The intention is that you should be able to log all uncaught exceptions using the ILogger interface. This means you can plug in your own custom logging system that does whatever you want with the exception info.

However there's still a piece of this we haven't yet implemented. This is being tracked by #19533. We'll be sure to get this done soon.

@ghost
Copy link

ghost commented Oct 9, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@RobertBouillon
Copy link

RobertBouillon commented Oct 20, 2020

For anyone looking for a client-side workaround:

function reportError(error) {
  if (DotNet) {
    DotNet.invokeMethodAsync('MyApp.Client', 'NotifyError', error);
  }
}

var exLog = console.error;
console.error = function (msg) {
  exLog.apply(console, arguments);
  reportError(msg);
}

window.addEventListener("unhandledrejection", function (promiseRejectionEvent) {
  reportError(promiseRejectionEvent.reason.message);
});
  public partial class AppLayout
  {
    [JSInvokable("NotifyError")]
    public static void NotifyError(string error)
    {
      //Handle errors
    }
  }

Browser support

This is a critically important feature for a production application and I wouldn't feel comfortable releasing an application where I couldn't control the application's behavior in the event of an unhandled error, or at least be able to reliably log it. Fortunately, I'm sure there's enough support for this in JavaScript we can leverage, however it's a big enough gap to make me question whether this technology's really mature enough to be adopted in serious applications.

I hope this a global error handler is prioritized in future releases.

@Lixan
Copy link

Lixan commented Dec 5, 2020

What a pity that a major issue like this one, opened in Aug. 2019, is still delayed
Did most of people give up and surrounded all of their methods with try/catch ?
It's ironic to go on a recent techno if it's to write ugly code

@sylvainonweb
Copy link

@Lixan For sure. If an essential issue like this isn’t take into account, imagine others...
Would prefer that Microsoft focuses on process these kind of issues that add new features.

It’s already a big deal to choose Blazor as web framework because of its youth but if Microsoft doesn’t listen to developpers !!!

@akovac35
Copy link

akovac35 commented Dec 6, 2020

Perhaps it is not technically feasible to fix this issue as we need without some kind of AOP or attribute + compiler feature.

It would be an improvement if components could implement an optional interface which exposes a handle error method. Then components would invoke this method on unhandled error.

@mkArtakMSFT, is at least some solution planned for real for this issue?

@mkArtakMSFT mkArtakMSFT added Priority:1 Work that is critical for the release, but we could probably ship without User Story A single user-facing feature. Can be grouped under an epic. labels Jan 31, 2021
@mkArtakMSFT mkArtakMSFT added this to Committed in .NET 6.0 Jan 31, 2021
@rasaconsulting-sandy
Copy link

I am looking for some global error handling solution to both my Blazor server and WASM projects. Is there any forecast date for when .Net 6 will be out?

@SteveSandersonMS
Copy link
Member

Yes, the .NET ship dates are listed at https://github.com/dotnet/core/blob/master/roadmap.md

Please note that you already can catch errors and log them globally. That's what the ILogger interface is for. The work we're planning for .NET 6 is somewhat more specific and described at #26953

Since #26953 reflects the work we plan to do, I'm closing this issue now. If anyone is concerned that #26953 (and the existing ILogger mechanism) doesn't meet their needs, could you please file a new issue and provide specific details about what different thing you are looking for besides #26953 and ILogger? Thanks!

.NET 6.0 automation moved this from Committed to Completed Feb 15, 2021
@SteveSandersonMS SteveSandersonMS removed this from Completed in .NET 6.0 Feb 15, 2021
@SteveSandersonMS SteveSandersonMS removed this from the Next sprint planning milestone Feb 15, 2021
@mkArtakMSFT mkArtakMSFT removed Bottom Up Work User Story A single user-facing feature. Can be grouped under an epic. labels Feb 16, 2021
@hikalkan
Copy link

@SteveSandersonMS you are pointing to ILogger implementation. This works for WebAssembly. We've already done it for the ABP Framework (see). We are showing a messagebox to the user.

However, for Blazor Serverside, it doesn't help much because the SignalR connection is broken in anway. I investigated the source code (of aspnetcore), but there is no way to prevent this. CircuitRegistry has no extension point. For ASP.NET Core team, it is pretty easy to provide an extension point for it. So, the end developer checks the exception type/message and decide to break the circuit or just show a message and keep the application continue to work.

@SteveSandersonMS
Copy link
Member

So, the end developer checks the exception type/message and decide to break the circuit or just show a message and keep the application continue to work.

I'm afraid that's not a valid solution. Continuing after an arbitrary unhandled exception compromises the server's stability and may open security vulnerabilities. If there's an unhandled exception, the only safe action is to terminate the circuit, since there's no way to know what state things within that circuit have been left in. We already send a notification about the error to JS code running in the browser, so it's possible to display some custom UI at that point (which the project template already does by default). I know this makes things seem less convenient, but helping customers deliver secure applications is a very high priority for us.

@akovac35
Copy link

@SteveSandersonMS Would the following suggestion work?

ComponentBase is modified to include a virtual method which would be invoked when BuildRenderTree method encounters a problem. This method would provide alternative render. If it too fails, break the circuit.

This may work if RenderTreeBuilder is not immediately committing contents between OpenComponent and CloseComponent statements?

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Mar 10, 2021

@akovac35 That's interesting. We could do that, but I think the plan we already have in #26953 gives you the same benefits but even better still, since you don't have to repeat your error handling in every single component, but rather can plug in an error renderer at any set of points in your component hierarchy. Errors caught within an error boundary will not be fatal to the circuit, since we know exactly what state is affected and the framework can clear it up so the app can safely continue.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-most This issue impacts most of the customers area-blazor Includes: Blazor, Razor Components Components Big Rock This issue tracks a big effort which can span multiple issues enhancement This issue represents an ask for new feature or an enhancement to an existing one Priority:1 Work that is critical for the release, but we could probably ship without severity-major This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests