Open Bug 1589265 Opened 5 years ago Updated 2 years ago

Prevent debugger from going entirely white due to unexpected errors

Categories

(DevTools :: Debugger, enhancement, P2)

enhancement

Tracking

(Not tracked)

People

(Reporter: loganfsmyth, Unassigned)

References

Details

https://bugzilla.mozilla.org/show_bug.cgi?id=1581530 was caused because we assumed that the server would emit a newSource event before sending us a paused notification in a given source. We've generally relied on that, but it may be worth considering whether receiving a paused packet should actively fetch the sources if they are not found in the store already.

Alternatively, we should auto-resume if we receive a pause in an unknown location, but that may just delay the problem.

EDIT:
Another option would be that we can accept that these exceptions are going to happen sometimes, and just try to make it so that the debugger recovers on page refresh, instead of staying white forever. That may be the best option.

We upgraded to React 16 a while back, which will unmount the whole application if an exception during rendering reaches the root component. Instead, we should have an Error Boundary (https://reactjs.org/docs/error-boundaries.html) to catch the exception. An exception is an exception, so we can still put the UI into an un-useful state, but at least having the boundary would let us re-render the application once the user navigates the page.

We should also probably explore having boundaries without the application panels, but I think it's not obvious what granularity we would choose.

Thanks for opening this bug.

I'm having a hard time understanding why the debugger becomes 100% blank because of a missing event?
Shouldn't it always be rendering its complete UI, but may be empty in same places?
I would expect the following components to always be rendered, even if server's behavior is confusing:

  • The three panels: left sidebar, sources and right sidebar
  • empty source tree
  • breakpoint control toolbar
  • all the breakpoint control UI (watchpoints, breakpoints, XHR, Event, DOM), possibly empty

Doing that, might help limiting breakages to a very particular part of the debugger instead of making it 100% unusable.
I'm seeing more and more reports of blank debuggers, leading to a totally unusable product rather than a partially broken one.

(In reply to Logan Smyth [:loganfsmyth] from comment #0)

https://bugzilla.mozilla.org/show_bug.cgi?id=1581530 was caused because we assumed that the server would emit a newSource event before sending us a paused notification in a given source. We've generally relied on that, but it may be worth considering whether receiving a paused packet should actively fetch the sources if they are not found in the store already.

Alternatively, we should auto-resume if we receive a pause in an unknown location, but that may just delay the problem.

To be clear, it looks like the debugger frontend would miss a more systemic pattern to prevent becoming blank.
I don't think this has to be specific to bug 1581530's very particular breakage.
It sounds like something at Redux level, or within bootstrap code, should be done so that, in general, the base debugger UI renders.

Blocks: dbg-72
Priority: -- → P2

It sounds like something at Redux level, or within bootstrap code, should be done so that, in general, the base debugger UI renders.

You're right, sorry I was hasty in filing this.

I'm having a hard time understanding why the debugger becomes 100% blank because of a missing event?
Shouldn't it always be rendering its complete UI, but may be empty in same places?
I would expect the following components to always be rendered, even if server's behavior is confusing:

Thanks for making me do some research here. SO. The reason it goes totally white is because that is what React 16 does when there is an uncaught exception during rendering. This was not the case in earlier React versions, so I didn't realize until now that this all-white behavior is probably new as of https://bugzilla.mozilla.org/show_bug.cgi?id=1473842.

I've updated the bug description too, but I agree with rendering the empty UI if there is an uncaught exception instead of going all-white. Perhaps we can also let the user know that devtools have been disabled until refresh.

Summary: Ensure that the Redux store is populated with all referenced actors before emitting events → Prevent debugger from going entirely white due to unexpected errors

I think this is a great idea. I think react 16 has a hook for uncaught exceptions which we could use in App to render a reasonable UI. I believe David advocated for this six months ago as well.

Flags: needinfo?(dwalsh)

Julien, I'm wondering if you are doing anything in the profiler UI to mitigate this?

Flags: needinfo?(felash)

Yeah, I'd love to implement error boundaries for the debugger. What do we want to put there? Do we want to expose an error if we can catch one? Is there a way we can provide a button that can reload the debugger panel?

Flags: needinfo?(dwalsh)

It'd be great to get victoria's thoughts, but in terms of content. I assume it would be nice to have a general message about

Oops, something went wrong. maybe an otter :/

and then a button to reload, which might just be implented via window.location = ""

Flags: needinfo?(victoria)

Sorry for the delay!

Yes, sounds great to have an error display with button to reload.

"Oops, something went wrong" is nice - I prefer it to Firefox's less sympathetic "Gah. Your tab just crashed."

We could possibly use this crashy bird illustration - I'm checking with UX to see if this is already in product somewhere https://design.firefox.com/photon/visuals/illustration.html

Flags: needinfo?(victoria)

To be a bit more user-friendly, would it make sense to just first try to recover by reloading the Debugger? Safari shows a bar when it happens to inform the user: https://imgur.com/a/t2aDzfr

I guess the risk with that is if the user has Inspector changes or other page states that could be lost when reloading?

Are we assuming the tool is still usable while we show an error? Error boundaries usually mean that the UI can not be rendered and some error page is rendered instead.

My argument would be that if the tool isn't usable, as I assumed, an automatic reload recovers users more seamlessly then an "oopsie, click here to fix your tool" interstitial.

But then we need to take care to not enter an infinite loop...

Yes, that would be a good technical requirement.

David had some thoughts on this in https://github.com/firefox-devtools/debugger/issues/6384 from way back too.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.