Uncaught async Promise errors should cause sdk tests to fail

RESOLVED FIXED

Status

defect
P2
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Yoric, Assigned: jsantell)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

No description provided.
Yoric, can you give example tests where this is actually a concern? most (if not all) async tests already have something like

  somePromie().then(done);

which to my understanding already fails in the case of async promise error, though not explicitly, but because of a test timeout.
Well, that's assuming that you have added a terminator to all promises.
With xpcshell tests and mochitests we found out that we are missing a number of errors simply because ensuring that we have caught all async errors is hard/impossible. Promise.jsm provides a way to detect most of these errors, but if the test suite doesn't take this into account, that's not very useful.
so, we are usually careful about stuff like that, but i understand this is more of a "defense in depth" thing..

then i guess it depends on bug 881047.
Depends on: 881047
Priority: -- → P2
Test with the patch in bug 1016387
Assignee: nobody → jsantell
Patch in bug 1016387 does not work for SDK tests, we'll have to hook into Promise.jsm's `Debugger.addUncaughtErrorObserver` into the SDK test harness.
Depends on: 1052633
Depends on: 1052667
Posted file GH PR 1582
Attachment #8471857 - Flags: review?(evold)
Attachment #8471857 - Flags: review?(evold) → review+

Comment 7

5 years ago
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/fc3d108c8ca86b213f95fb5cc3dc7a4f06239cb7
Bug 998277 - Enable tests breaking on uncaught promise rejections

https://github.com/mozilla/addon-sdk/commit/91c8e46257a43296620f1a5bab328687c73515d0
Merge pull request #1582 from jsantell/998277

Bug 998277 - Enable tests breaking on uncaught promise rejections, r=@erikvold
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
No longer depends on: 1054399
You need to log in before you can comment on or make changes to this bug.