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

Follow Up: Error handling improvements #8613

Closed
7 of 8 tasks
rynowak opened this issue Mar 18, 2019 · 6 comments
Closed
7 of 8 tasks

Follow Up: Error handling improvements #8613

rynowak opened this issue Mar 18, 2019 · 6 comments
Labels
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
Milestone

Comments

@rynowak
Copy link
Member

rynowak commented Mar 18, 2019

Summary

This is a work item to track issues and improvements related to error handing and diagnostics in general for Components applications. Since we received strong user feedback in this area, I'm focusing on server-side components, but will consider applications to Blazor applications as well.

Goals:

  • Leverage what users already know for errors during prerendering
  • Provide simple diagnostics (exception message + call stack) for interactive rendering during development in logs and in the client
  • Provide better configuration and documentation for SignalR diagnostics so we can understand users' problems

Misc bugs

Prerendering problems

You don't get a good experience when prerendering throws an exception - you just get a while screen. We should show the developer exception page - it's an existing paradigm that works well and is familiar to users.

Unhandled exceptions in server-side components

I propose that we add a piece of built-in client-side UI for diagnostics and error information as part of the components/blazor development experience through the templates. This would involve adding some very general hooks and plumbing that anyone can leverage, as well as some new assets (script) strictly intended for diagnostics and inner loop.

Why new UI you say? I think folks are still developing their mental model of how all of this fits together. UI is the most approachable way to surface information they will need - without adding some UI we're going to tell people over and over - "you need to open the developer console". Giving ourselves a foothold in some kind of developer-experience UI will scale up if we want to add more information (SignalR diagnostics/renderer tracing).

And best of all... this can work in exactly the same way for both server-side and client.

The secondary nice thing about this is that the inclusion of the script says to us "this is a developer inner-loop scenario". We have a good mechanism for this on the server, but not on the client (and not in a Blazor app). Do we think this is a good mechanism?

What would it look like: (warning, terrible UI)
image

image

Obviously we should do a better job with the UI

What this would require:

  • Add an event/callback to the Blazor global object for unhandled exceptions
  • Make sure server.js and Blazor invoke this new callback
  • Add a new placeholder div and css to the templates
  • Add a new script components.developer.js that hooks unhandled exceptions and manipulates that div (based on id) - conditional on environment
  • Make sure that server-side exceptions are flowing to the client in development mode. Sanitise the errors for production mode

Users who don't want this can replace the UI if they want by writing their own exception logic, and just delete the script reference and div.

Open Question: do we make the script conditional on something in Blazor? It's easy to make conditional in server-side because of Razor Pages.

Open Question: should we separate error handling of connectivity from exceptions that happened on the server?

SignalR diagnostics and troubleshooting

We've had a number of issues reported related to SignalR - in particular we have a strong need for more guidance and diagnostics. See #8384 #6920

SignalR's docs here: https://docs.microsoft.com/en-us/aspnet/core/signalr/diagnostics?view=aspnetcore-2.2#javascript-client-logging are pretty simple to follow, but we don't provide a way for users to do this.

I'd propose that we do the following:

Then we continue to engage with users in preview4 now that it's possible to get real diagnostics from SignalR. I don't think we know enough about the problems users face that this point to design or build more things.

@rynowak rynowak added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-blazor Includes: Blazor, Razor Components labels Mar 18, 2019
@rynowak rynowak self-assigned this Mar 18, 2019
@rynowak rynowak added the Components Big Rock This issue tracks a big effort which can span multiple issues label Mar 18, 2019
@rynowak rynowak mentioned this issue Mar 18, 2019
56 tasks
@analogrelay
Copy link
Contributor

  • Sanitise the errors for production mode

From some experience, it's hard to "Sanitize" exceptions. Our approach in SignalR has generally been to suppress all exception messages (replace with "An error has occurred") unless they are derived from some well-defined base class (in SignalR: HubException). Thus users have to opt-in to sending details in production. In dev mode, we do the same as you propose, dump the whole shebang in.

  • Provide a callback for users to configure the JS client (might be needed for more than logging)

👍 In theory this could be more than logging so I'd definitely suggest an advanced mode where you expose the HubConnectionBuilder.

  • Document how to do this, and point to the SignalR docs about logging

Party On

@rynowak
Copy link
Member Author

rynowak commented Mar 18, 2019

From some experience, it's hard to "Sanitize" exceptions. Our approach in SignalR has generally been to suppress all exception messages (replace with "An error has occurred") unless they are derived from some well-defined base class (in SignalR: HubException). Thus users have to opt-in to sending details in production. In dev mode, we do the same as you propose, dump the whole shebang in.

Yeah, that's the kind of thing I had in mind. I said sanitize, what I meant was to replace the exception with a generic "error is happen" kind of thing.

@mkArtakMSFT mkArtakMSFT added this to the 3.0.0-preview6 milestone Mar 19, 2019
@mkArtakMSFT
Copy link
Member

Let's review in preview6 what parts to do when.

@javiercn
Copy link
Member

To add to what we listed here. When a component fails to render we can offer the ability to render an alternative error component. The idea is that a failed component doesn't bring down the entire application but that instead a different component is rendered (maybe with an error image or an error message) allowing the rest of the application to work.

@rynowak rynowak mentioned this issue Apr 3, 2019
@mkArtakMSFT mkArtakMSFT removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels May 9, 2019
@mkArtakMSFT mkArtakMSFT modified the milestones: 3.0.0-preview7, 3.1.0 May 24, 2019
@mkArtakMSFT
Copy link
Member

Closing this as nothing left here to be done or untracked.

@danroth27
Copy link
Member

@rynowak @mkArtakMSFT Should this be in a 3.0 milestone instead of 3.1?

@rynowak rynowak modified the milestones: 3.1.0, 3.0.0 Sep 16, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 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 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
Projects
None yet
Development

No branches or pull requests

5 participants