1156474 - Break on unhandled promise rejection

Status: REOPENED

(DevTools :: Debugger, enhancement, P3)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I swear we already had a bug for this, but I can't find it. Feel free to dupe if you know it.

We should use the API added in bug 1083361 to break on unhandled promise rejections.

This should probably be separate from break-on-exceptions (or maybe another modifier for break-on-exceptions like the uncaught modifier?) because we need to break at the time of rejection, but it could still be handled sometime later.

Comment 1

[James Long (:jlongster)]

I can try to take a look at this this week.

Comment 2

[James Long (:jlongster)]

This feature requires more thought so I'm going to focus on smaller fixes for now (see more discussion in bug 1156851)

Comment 4

[James Burke [:jrburke]]

Bug 1174026 was marked as a duplicate to this bug, but I believe it to be different:

Bug 1174026 is about pausing if reject() is called, where this one seems to be about pausing if reject() called and there is no error registration on the promise.

I wanted to pause when some code purposely called reject(), but was not sure which pathway that caused reject() was called, and I wanted to inspect the local scope state at the reject() call. 

There were error listeners on the promise chain, so not an unhandled rejection, but due to the chain involved, which had a few chances to reject() in different ways, it was not immediately obvious to me where I should place a manual breakpoint to catch the reject. 

The scenario has passed now, so going on memory, but there may have been enough of a trace that I was able to manually walk back to figure out what reject() was called, but it took some more manual work that would have been saved by just asking the debugger to pause when any reject() was called.

If that sounds like a different set of work than this bug, then I can add this info to bug 1174026 and reopen it.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

I don't think we have fully flashed out the UX of this feature yet, so I think it makes sense to have a single bug, at least for now. FWIW in my view the pause-on-reject-call and pause-on-unhandled-rejection nicely mirror the existing pause-on-exception and pause-on-unhandled-exception features of the debugger. A similar pair of options in the debugger option menu wouldn't be the worst way to implement this IMO.

Comment 6

[Soledad Penades [:sole] [:spenades]]

Yulia, Jason: is this implemented in the new Debugger now?

Comment 7

[Patrick Brosset <:pbro>]

Clearly not a P1 anymore. Moving back to the backlog.
Yulia or Jason, Sole already pinged you a couple months ago. Could you please reply. Maybe we need to move this to GitHub instead?

Comment 8

[Jason Laster [:jlast]]

Thanks Patrick. I added it to our product backlog.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Wait, hold on.  "INVALID" means "this isn't even a bug".  Is this still planned to be fixed at some point?  If so, it should be open.  If it's not planned to be fixed, it should be WONTFIX or something, presumably.

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Jason, perhaps a link to the product backlog this was moved to would help here?

Comment 11

[Jason Laster [:jlast]]

:jryans thanks. wasn't sure there if the airtable was visible. https://airtable.com/shrLDOSJKIfsTToEq

:bz that's a good point. In Github we close issues that are added to our backlog so that the queue is just prioritized work. Forgot the context :)

Comment 12

[:Harald Kirschner :digitarald]

The DOM event side landed in 69, so it would be good for devtools to follow up.

Comment 13

[Jason Laster [:jlast]]

What would be a good UI for this? Another checkbox in the breakpoints pane?

This could be a nice quick win.

Comment 14

[:Harald Kirschner :digitarald]

Another checkbox in the breakpoints pane?

Options for UI are another 2nd-level checkbox that appears when "Pause on exception" is selected; or top-level (as it doesn't have much to do with exceptions). Both take some space.

On the parity side, Chrome does cover promise rejections with the pause on exception checkboxes; which seems most straightforward (vs a growing wall of checkboxes). Would that be an easy v0 to expose the functionality?

Comment 15

[:Harald Kirschner :digitarald]

On the parity side, Chrome does cover promise rejections with the pause on exception checkboxes; which seems most straightforward (vs a growing wall of checkboxes).

Discussed with a few people; this is a good start for this initial bug.

Comment 16

[nightpool]

Any update on this? It's a big pain point when debugging async code.