Closed
Bug 1240804
Opened 8 years ago
Closed 8 years ago
[meta] Re-enable unhandled promise rejection tracking in DevTools
Categories
(DevTools :: General, defect)
DevTools
General
Tracking
(firefox46 wontfix, firefox47 wontfix, firefox48 fixed)
RESOLVED
FIXED
Firefox 48
People
(Reporter: jryans, Unassigned)
References
Details
(Keywords: meta)
Attachments
(2 files, 2 obsolete files)
Unhandled promise rejection tracking was accidentally disabled in DevTools tests ~6 months ago by bug 1164564. This meta-bug tracks the work to fix tests and finally re-enable unhandled rejection tracking.
Updated•8 years ago
|
Reporter | ||
Comment 1•8 years ago
|
||
Spreadsheet listing current rejections: https://docs.google.com/spreadsheets/d/1ZKXW9HqnxJNdeEVG7s33_TGSV94PdUJCsQsZe1GGqjY/edit
Reporter | ||
Comment 2•8 years ago
|
||
This patch can be used to re-enable unhandled promise rejection tracking, so that test fixes can be verified.
Reporter | ||
Comment 3•8 years ago
|
||
:jlong suggests re-testing bug 1149910 before landing to ensure stepping in browser debugger doesn't trap you in promises after the proposed change.
Reporter | ||
Comment 4•8 years ago
|
||
Updated try attempt: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7312ddcb6d6e
Reporter | ||
Comment 5•8 years ago
|
||
Rebasing patch now that srcdir loader was removed.
Attachment #8709510 -
Attachment is obsolete: true
Reporter | ||
Comment 6•8 years ago
|
||
We're very close now. I'd like to see if it's acceptable to whitelist the two remaining failures, which are both in the debugger, so I'll post patches for review.
Reporter | ||
Comment 7•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42809/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42809/
Attachment #8735509 -
Flags: review?(poirot.alex)
Attachment #8735510 -
Flags: review?(ejpbruel)
Reporter | ||
Comment 8•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42811/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42811/
Reporter | ||
Updated•8 years ago
|
Attachment #8725277 -
Attachment is obsolete: true
Updated•8 years ago
|
Attachment #8735509 -
Flags: review?(poirot.alex) → review+
Comment 9•8 years ago
|
||
Comment on attachment 8735509 [details] MozReview Request: Bug 1240804 - Use common instance of Promise.jsm for test harness promise rejection handling. r=ochameau https://reviewboard.mozilla.org/r/42809/#review39453 Thanks for improving our tests!!
Comment 10•8 years ago
|
||
I'd love to review these, but apparently I cannot log in to reviewboard. I'm being redirected to bugzilla to log in, at which point I'm seeing the following error: "Auth delegation received an HTTP response other than 200 OK from auth consumer. Code: 500" That's an internal server error, so I don't know what the problem is, or how to work around it. Jryans, any advice? In the meantime, you can consider this patch r+'d. I agree we should whitelist these tests. Sorry that I wasn't able to look into them yet.
Flags: needinfo?(jryans)
Reporter | ||
Comment 11•8 years ago
|
||
Sounds like we sorted out MozReview access on IRC.
Flags: needinfo?(jryans)
Reporter | ||
Comment 12•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3494779d0ce5
Comment 13•8 years ago
|
||
Comment on attachment 8735510 [details] MozReview Request: Bug 1240804 - Allow remaining unhandled rejections. r=ejpbruel https://reviewboard.mozilla.org/r/42811/#review39485 LGTM
Attachment #8735510 -
Flags: review?(ejpbruel) → review+
Comment 14•8 years ago
|
||
That's the third time now that I've used mozreview, and the third time that I forgot to check the 'ship it' checkbox after giving a positive review. I wonder if we could make this more idiot proof? I can serve as test idiot, if necessary.
Comment 15•8 years ago
|
||
(In reply to Eddy Bruel [:ejpbruel] from comment #14) > That's the third time now that I've used mozreview, and the third time that > I forgot to check the 'ship it' checkbox after giving a positive review. Take a look at Bug 1246611 and also Bug 1197879, they are addressing this part of the UI
Comment 16•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/4317688957ef https://hg.mozilla.org/integration/fx-team/rev/90e78a157794 https://hg.mozilla.org/integration/fx-team/rev/c6faa92b435d https://hg.mozilla.org/integration/fx-team/rev/57e1fe3dfc6d https://hg.mozilla.org/integration/fx-team/rev/76e4e7620f83
Comment 18•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4317688957ef https://hg.mozilla.org/mozilla-central/rev/90e78a157794 https://hg.mozilla.org/mozilla-central/rev/c6faa92b435d https://hg.mozilla.org/mozilla-central/rev/57e1fe3dfc6d https://hg.mozilla.org/mozilla-central/rev/76e4e7620f83 https://hg.mozilla.org/mozilla-central/rev/d2d93c1d2ee3
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Reporter | ||
Updated•8 years ago
|
status-firefox47:
--- → wontfix
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•