1240804 - [meta] Re-enable unhandled promise rejection tracking in DevTools

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Spreadsheet listing current rejections:

https://docs.google.com/spreadsheets/d/1ZKXW9HqnxJNdeEVG7s33_TGSV94PdUJCsQsZe1GGqjY/edit

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Re-enable unhandled promise rejection tracking

This patch can be used to re-enable unhandled promise rejection tracking, so that test fixes can be verified.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

:jlong suggests re-testing bug 1149910 before landing to ensure stepping in browser debugger doesn't trap you in promises after the proposed change.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Updated try attempt: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7312ddcb6d6e

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Re-enable unhandled promise rejection tracking (v2)

Rebasing patch now that srcdir loader was removed.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1240804 - Use common instance of Promise.jsm for test harness promise rejection handling. r=ochameau

Review commit: https://reviewboard.mozilla.org/r/42809/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42809/

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1240804 - Allow remaining unhandled rejections. r=ejpbruel

Review commit: https://reviewboard.mozilla.org/r/42811/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42811/

Comment 9

[Alexandre Poirot [:ochameau]]

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

[Eddy Bruel [:ejpbruel]]

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.

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Sounds like we sorted out MozReview access on IRC.

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3494779d0ce5

Comment 13

[Eddy Bruel [:ejpbruel]]

Comment on attachment 8735510 [details]
MozReview Request: Bug 1240804 - Allow remaining unhandled rejections. r=ejpbruel

https://reviewboard.mozilla.org/r/42811/#review39485

LGTM

Comment 14

[Eddy Bruel [:ejpbruel]]

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

[Brian Grinstead [:bgrins]]

(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

[Pulsebot]

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 17

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d2d93c1d2ee3

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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