Skip to content

Commit a62c811

Browse files
authored
browser_tests: Report exceptions as test results. NFC (#16901)
I noticed that exceptions in browser tests were not being reported as test failure, but showing up as `[no http server activity]`. With this change, exceptions are treated as results and will cause synchronous test failure with more meaningful errors.
1 parent 97a44e1 commit a62c811

7 files changed

+41
-24
lines changed

tests/browser/async_bad_list.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
int main() {
1010
int x = EM_ASM_INT({
11+
window.disableErrorReporting = true;
1112
window.onerror = function(e) {
1213
var message = e.toString();
1314
var success = message.indexOf("unreachable") >= 0 || // firefox

tests/browser/async_returnvalue.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ extern "C" int sync_tunnel_bool(bool);
3232
int main() {
3333
#ifdef BAD
3434
EM_ASM({
35+
window.disableErrorReporting = true;
3536
window.onerror = function(e) {
3637
var success = e.toString().indexOf("import sync_tunnel was not in ASYNCIFY_IMPORTS, but changed the state") > 0;
3738
if (success && !Module.reported) {

tests/browser_reporting.js

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,26 +39,30 @@ function reportErrorToServer(message) {
3939
}
4040
}
4141

42-
if (typeof window === 'object' && window) {
43-
function report_error(e) {
44-
// MINIMAL_RUNTIME doesn't handle exit or call the below onExit handler
45-
// so we detect the exit by parsing the uncaught exception message.
46-
var message = e.message || e;
47-
console.error("got top level error: " + message);
48-
var offset = message.indexOf('exit(');
49-
if (offset != -1) {
50-
var status = message.substring(offset + 5);
51-
offset = status.indexOf(')')
52-
status = status.substr(0, offset)
53-
console.error(status);
54-
maybeReportResultToServer('exit:' + status);
55-
} else {
56-
if (hasModule) Module['pageThrewException'] = true;
57-
var xhr = new XMLHttpRequest();
58-
xhr.open('GET', encodeURI('http://localhost:8888?exception=' + message + ' / ' + e.stack));
59-
xhr.send();
60-
}
42+
function report_error(e) {
43+
// MINIMAL_RUNTIME doesn't handle exit or call the below onExit handler
44+
// so we detect the exit by parsing the uncaught exception message.
45+
var message = e.message || e;
46+
console.error("got top level error: " + message);
47+
if (window.disableErrorReporting) return;
48+
if (message.includes('unwind')) return;
49+
var offset = message.indexOf('exit(');
50+
if (offset != -1) {
51+
var status = message.substring(offset + 5);
52+
offset = status.indexOf(')')
53+
status = status.substr(0, offset)
54+
console.error(status);
55+
var result = 'exit:' + status;
56+
} else {
57+
if (hasModule) Module['pageThrewException'] = true;
58+
result = 'exception:' + message + ' / ' + e.stack;
6159
}
60+
// FIXME: Ideally we would just reportResultToServer rather than the `maybe`
61+
// form but some browser tests currently report exceptions after exit.
62+
maybeReportResultToServer(result);
63+
}
64+
65+
if (typeof window === 'object' && window) {
6266
window.addEventListener('error', event => report_error(event));
6367
window.addEventListener('unhandledrejection', event => report_error(event.reason));
6468
}

tests/common.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1330,7 +1330,7 @@ def do_GET(self):
13301330
self.end_headers()
13311331
self.wfile.write(b'OK')
13321332

1333-
elif 'stdout=' in self.path or 'stderr=' in self.path or 'exception=' in self.path:
1333+
elif 'stdout=' in self.path or 'stderr=' in self.path:
13341334
'''
13351335
To get logging to the console from browser tests, add this to
13361336
print/printErr/the exception handler in src/shell.html:

tests/emscripten_throw_number_pre.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
addEventListener('error', function(event) {
2-
event.preventDefault();
3-
event.stopPropagation();
2+
window.disableErrorReporting = true;
43
var result = event.error === 42 ? 0 : 1;
54
var xhr = new XMLHttpRequest();
65
xhr.open('GET', 'http://localhost:8888/report_result?' + result, true);

tests/emscripten_throw_string_pre.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
addEventListener('error', function(event) {
2-
event.preventDefault();
3-
event.stopPropagation();
2+
window.disableErrorReporting = true;
43
var result = event.error === 'Hello!' ? 0 : 1;
54
var xhr = new XMLHttpRequest();
65
xhr.open('GET', 'http://localhost:8888/report_result?' + result, true);

tests/test_browser.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,7 @@ def setup(assetLocalization):
659659
<hr><div id='output'></div><hr>
660660
<script type='text/javascript'>
661661
window.onerror = function(error) {
662+
window.disableErrorReporting = true;
662663
window.onerror = null;
663664
var result = error.indexOf("test.data") >= 0 ? 1 : 0;
664665
var xhr = new XMLHttpRequest();
@@ -2587,6 +2588,18 @@ def test_doublestart_bug(self):
25872588
})
25882589
@requires_threads
25892590
def test_html5_core(self, opts):
2591+
if '-sHTML5_SUPPORT_DEFERRING_USER_SENSITIVE_REQUESTS=0' in opts:
2592+
# In this mode an exception can be thrown by the browser, and we don't
2593+
# want the test to fail in that case so we override the error handling.
2594+
create_file('pre.js', '''
2595+
window.disableErrorReporting = true;
2596+
window.addEventListener('error', (event) => {
2597+
if (!event.message.includes('exception:fullscreen error')) {
2598+
report_error(event);
2599+
}
2600+
});
2601+
''')
2602+
self.emcc_args.append('--pre-js=pre.js')
25902603
self.btest(test_file('test_html5_core.c'), args=opts, expected='0')
25912604

25922605
@requires_threads

0 commit comments

Comments
 (0)