ProtocolCompatibility should only use promise rejections to indicate exceptions, not false results

RESOLVED INVALID

Status

DevTools
Debugger
P2
normal
RESOLVED INVALID
5 years ago
7 days ago

People

(Reporter: jimb, Assigned: jimb)

Tracking

(Blocks: 2 bugs)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 744962 [details] [diff] [review]
In the debugger client's ProtocolCompatibility, save promise rejection for exceptions, not false results.

If we're going to log unhandled promise rejections, and treat them as test failures, then the debugger client's ProtocolCompatibility should not use rejection to announce that a feature isn't present. Instead, it should pass 'true' or 'false' to its resolution handler.
Attachment #744962 - Flags: review?(nfitzgerald)
(Assignee)

Updated

5 years ago
Blocks: 868174
Comment on attachment 744962 [details] [diff] [review]
In the debugger client's ProtocolCompatibility, save promise rejection for exceptions, not false results.

Review of attachment 744962 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great!

Would like to see a green try push just as a formality.

The previous behavior wasn't causing problems was it? If so, sorry!

Final note, I'm not a module peer so you need to get one more person to sign off on this before it can land ;)

::: toolkit/devtools/debugger/dbg-client.jsm
@@ +1182,5 @@
> +              return {
> +                sources: [
> +                  { url: url, actor: sourceActorsByURL[url] }
> +                  for (url of Object.keys(sourceActorsByURL))
> +                    ]

Nit: indentation looks a little off here with the closing square bracket.
Attachment #744962 - Flags: review?(nfitzgerald) → review+
Priority: -- → P2
Are we still doing this?

Updated

4 years ago
Summary: JS debugger: ProtocolCompatibility should save promise rejection for exceptions, not false results → ProtocolCompatibility should only use promise rejections to indicate exceptions, not false results

Updated

4 years ago
Blocks: 1074526
Nick, didn't you get rid of this code completely?
Flags: needinfo?(nfitzgerald)
Yup, gone in bug 1090594.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(nfitzgerald)
Resolution: --- → INVALID

Updated

7 days ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.