Closed Bug 868278 Opened 11 years ago Closed 9 years ago

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

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: jimb, Assigned: jimb)

References

Details

Attachments

(1 file)

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)
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?
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
Blocks: dbg-client
Nick, didn't you get rid of this code completely?
Flags: needinfo?(nfitzgerald)
Yup, gone in bug 1090594.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(nfitzgerald)
Resolution: --- → INVALID
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: