Open Bug 1156474 Opened 9 years ago Updated 2 years ago

Break on unhandled promise rejection

Categories

(DevTools :: Debugger, enhancement, P3)

x86
macOS
enhancement

Tracking

(Not tracked)

REOPENED

People

(Reporter: fitzgen, Unassigned)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [polish-backlog][difficulty=medium])

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.
I can try to take a look at this this week.
Assignee: nobody → jlong
Depends on: 1156851
This feature requires more thought so I'm going to focus on smaller fixes for now (see more discussion in bug 1156851)
Assignee: jlong → nobody
Priority: -- → P1
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.
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.
Whiteboard: [devedition-40][difficulty=medium] → [polish-backlog][difficulty=medium]
Yulia, Jason: is this implemented in the new Debugger now?
Flags: needinfo?(yulia.startsev)
Flags: needinfo?(jlaster)
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?
Severity: normal → enhancement
Priority: P1 → P3
Thanks Patrick. I added it to our product backlog.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(jlaster)
Resolution: --- → INVALID
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.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Jason, perhaps a link to the product backlog this was moved to would help here?
Flags: needinfo?(yulia.startsev) → needinfo?(jlaster)
: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 :)
Flags: needinfo?(jlaster)
Product: Firefox → DevTools

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

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

This could be a nice quick win.

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?

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.

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

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