Closed Bug 926635 Opened 7 years ago Closed 7 years ago

Improve layout of uncaught Promise.jsm error messages


(Toolkit :: Async Tooling, defect)

Not set





(Reporter: Yoric, Assigned: tareqakhandaker)


(Whiteboard: [Async][mentor=Yoric][lang=js])


(1 file, 1 obsolete file)

A promise chain failed to handle a rejection: on Mon Oct 14 2013 16:50:15 GMT+0200 (CEST), Error: BOOM! at

That's a bit ugly.

It would be better to have:
A promise chain failed to handle a rejection.

Date of error: Mon Oct 14 2013 16:50:15 GMT+0200 (CEST)
Full message: Error: BOOM!
Full stack: @Scratchpad/2:27

The code that needs to be changed lives in function observe() of the following file:
Is there anything I can run to test my patch? Thanks,
Assignee: nobody → tareqakhandaker
Attachment #819504 - Flags: review?(dteller)
Comment on attachment 819504 [details] [diff] [review]

Review of attachment 819504 [details] [diff] [review]:

Looks good to me.
Do you have Try access?

::: toolkit/modules/Promise.jsm
@@ +224,4 @@
>    }
>    error.init(
> +             /*message*/"A promise chain failed to handle a rejection.\n\nDate: " +
> +               date + "\nFull Message: " + message,

Could you take the opportunity to cut the string after the newlines?
Attachment #819504 - Flags: review?(dteller) → review+
Like this? (so that the line is <80 characters)
Attachment #819504 - Attachment is obsolete: true
Attachment #819511 - Flags: review?(dteller)
Comment on attachment 819511 [details] [diff] [review]

Review of attachment 819511 [details] [diff] [review]:

Not exactly what I had in mind, but that looks good.
Here's the result from automated tests:
Attachment #819511 - Flags: review?(dteller) → review+
That looks good. Unless you want to add any changes, we can land this.
I did not actually see what the new error message actually looked like because I did not know where to look.
You can launch the promise tests as follows:
   ./mach xpcshell-test toolkit/modules/test/xpcshell/test_promise.js

At some point towards the end of the test, it will display the error message.
I got this error: An xpcshell.ini could not be found for the passed test path. Please select a path whose directory or subdirectories contain an xpcshell.ini file. It is possible you received this error because the tree is not built or tests are not enabled.

It's fine though. I'll take your word for it that it looks alright if you think it looks ready to land.
Have you built Firefox?
Yeah, I have.
Ah, that should have been test_Promise.js (with an uppercase P).
I got it to work. The command is ./mach xpcshell-test toolkit/modules/tests/xpcshell/test_Promise.js

It looks good to me.

 0:04.98 TEST-INFO | /Users/me/src/mozilla-central/obj-ff-dbg/_tests/xpcshell/toolkit/modules/tests/xpcshell/test_Promise.js | "Observing [JavaScript Error: "A promise chain failed to handle a rejection.

Date: Sat Oct 26 2013 17:59:49 GMT-0400 (EDT)
Full Message: Error: This is an uncaught error 962430.284671439
Full Stack: make_error_rejection@/Users/me/src/mozilla-central/obj-ff-dbg/_tests/xpcshell/toolkit/modules/tests/xpcshell/test_Promise.js:771
" {file: "/Users/me/src/mozilla-central/obj-ff-dbg/_tests/xpcshell/toolkit/modules/tests/xpcshell/test_Promise.js" line: 771 column: 0 source: "771"}]"
Looks good to me.
If you're ready, we'll mark this bug checkin-needed.
Flags: needinfo?(tareqakhandaker)
I'm ready.
Flags: needinfo?(tareqakhandaker)
Keywords: checkin-needed
Whiteboard: [Async][mentor=Yoric][lang=js] → [Async][mentor=Yoric][lang=js][fixed-in-fx-team]
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [Async][mentor=Yoric][lang=js][fixed-in-fx-team] → [Async][mentor=Yoric][lang=js]
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.