Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Message for unhandled promise rejection is not very helpful #19021

Closed
foolip opened this issue Sep 12, 2019 · 5 comments · Fixed by #19092
Closed

Message for unhandled promise rejection is not very helpful #19021

foolip opened this issue Sep 12, 2019 · 5 comments · Fixed by #19092
Labels
infra priority:backlog

Comments

@foolip
Copy link
Member

foolip commented Sep 12, 2019

In #18932 there was a harness error that said "The user aborted a request" or "The operation was aborted". This turned out to be because of an unhandled promise rejection and fixed in #19001.

That it was an unhandled promise rejection wasn't at all clear. That's because the handling of this is to simply use event.reason.message as the message:

wpt/resources/testharness.js

Lines 3368 to 3399 in 152adb0

var error_handler = function(e) {
if (tests.tests.length === 0 && !tests.allow_uncaught_exception) {
tests.set_file_is_test();
}
var stack;
if (e.error && e.error.stack) {
stack = e.error.stack;
} else {
stack = e.filename + ":" + e.lineno + ":" + e.colno;
}
if (tests.file_is_test) {
var test = tests.tests[0];
if (test.phase >= test.phases.HAS_RESULT) {
return;
}
test.set_status(test.FAIL, e.message, stack);
test.phase = test.phases.HAS_RESULT;
// The following function invocation is superfluous.
// TODO: Remove.
test.done();
} else if (!tests.allow_uncaught_exception) {
tests.status.status = tests.status.ERROR;
tests.status.message = e.message;
tests.status.stack = stack;
}
done();
};
addEventListener("error", error_handler, false);
addEventListener("unhandledrejection", function(e){ error_handler(e.reason); }, false);

Prefixing the error with "Unhandled rejection" would really help.

@web-platform-tests/wpt-core-team for opinions.
@foolip foolip added the infra label Sep 12, 2019
@foolip foolip mentioned this issue Sep 12, 2019
Closed
@foolip
Copy link
Member Author

foolip commented Sep 12, 2019

Drafted #19023 to demonstrate this. resources/test/tests/unit/exceptional-cases.html doesn't test the messages, but with infrastructure/ tests it's easy to see them at least.

@jgraham can messages be tested by .ini files?

@gsnedders
Copy link
Member

can messages be tested by .ini files?

@foolip no

@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 16, 2019

At least in theory, the messages are not guaranteed to be stable. Adding the extra text makes sense to me.

foolip added a commit that referenced this issue Sep 16, 2019
@foolip
[testharness.js] add "Unhandled rejection" to error message
8a1ae7d 
Fixes #19021.
@foolip foolip mentioned this issue Sep 16, 2019
Merged
@foolip
Copy link
Member Author

foolip commented Sep 16, 2019

Toying with it in #19092 to see how much it affects.

@jugglinmike
Copy link
Contributor

Assertion messages are not considered stable, but Chromium currently depends on them to some extent: gh-17974

foolip added a commit that referenced this issue Sep 20, 2019
@foolip
[testharness.js] add "Unhandled rejection" to error message
8af87b6 
Fixes #19021.
foolip added a commit that referenced this issue Sep 20, 2019
@foolip
[testharness.js] add "Unhandled rejection" to error message
135c54f 
Fixes #19021.
foolip added a commit that referenced this issue Sep 23, 2019
@foolip
[testharness.js] add "Unhandled rejection" to error message (#19092)
e84c738 
Adds an infrastructure/ test to demo message.
This doesn't test the message, just shows what it is.

Fixes #19021.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 24, 2019
@foolip @moz-wptsync-bot
Bug 1581563 [wpt PR 19092] - [testharness.js] add "Unhandled rejectio…
2bf096c 
…n" to error message, a=testonly

Automatic update from web-platform-tests
[testharness.js] add "Unhandled rejection" to error message (#19092)

Adds an infrastructure/ test to demo message.
This doesn't test the message, just shows what it is.

Fixes web-platform-tests/wpt#19021.
--

wpt-commits: e84c738d2dab6b8a291e57c56be62f4c4a8f7381
wpt-pr: 19092
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Sep 25, 2019
@foolip @moz-wptsync-bot
Bug 1581563 [wpt PR 19092] - [testharness.js] add "Unhandled rejectio…
cd37899 
…n" to error message, a=testonly

Automatic update from web-platform-tests
[testharness.js] add "Unhandled rejection" to error message (#19092)

Adds an infrastructure/ test to demo message.
This doesn't test the message, just shows what it is.

Fixes web-platform-tests/wpt#19021.
--

wpt-commits: e84c738d2dab6b8a291e57c56be62f4c4a8f7381
wpt-pr: 19092
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
@marco-c
Bug 1581563 [wpt PR 19092] - [testharness.js] add "Unhandled rejectio…
17c1c6b 
…n" to error message, a=testonly

Automatic update from web-platform-tests
[testharness.js] add "Unhandled rejection" to error message (#19092)

Adds an infrastructure/ test to demo message.
This doesn't test the message, just shows what it is.

Fixes web-platform-tests/wpt#19021.
--

wpt-commits: e84c738d2dab6b8a291e57c56be62f4c4a8f7381
wpt-pr: 19092

UltraBlame original commit: 88f6f32cd7cee0159a34aa1081084257a351e4b1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
@marco-c
Bug 1581563 [wpt PR 19092] - [testharness.js] add "Unhandled rejectio…
ce09a57 
…n" to error message, a=testonly

Automatic update from web-platform-tests
[testharness.js] add "Unhandled rejection" to error message (#19092)

Adds an infrastructure/ test to demo message.
This doesn't test the message, just shows what it is.

Fixes web-platform-tests/wpt#19021.
--

wpt-commits: e84c738d2dab6b8a291e57c56be62f4c4a8f7381
wpt-pr: 19092

UltraBlame original commit: 88f6f32cd7cee0159a34aa1081084257a351e4b1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 5, 2019
@marco-c
Bug 1581563 [wpt PR 19092] - [testharness.js] add "Unhandled rejectio…
4ec7145 
…n" to error message, a=testonly

Automatic update from web-platform-tests
[testharness.js] add "Unhandled rejection" to error message (#19092)

Adds an infrastructure/ test to demo message.
This doesn't test the message, just shows what it is.

Fixes web-platform-tests/wpt#19021.
--

wpt-commits: e84c738d2dab6b8a291e57c56be62f4c4a8f7381
wpt-pr: 19092

UltraBlame original commit: 88f6f32cd7cee0159a34aa1081084257a351e4b1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
infra priority:backlog
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[testharness.js] add "Unhandled rejection" to error message web-platform-tests/wpt
4 participants
@Ms2ger @gsnedders @foolip @jugglinmike