2007587 - Mochitests time out when an unexpected crash dump exists, instead of logging "unexpected-crash-dump-found"

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Rob Wu [:robwu]]

STR:

1. Simulate an unexpected crash by adding || true to the condition at https://searchfox.org/firefox-main/rev/20a1fb35a4d5c2f2ea6c865ecebc8e4bee6f86c9/testing/mochitest/tests/SimpleTest/TestRunner.js#758
2. Run a mochitest, e.g. ./mach test toolkit/components/extensions/test/mochitest/test_check_startupcache.html --log-mach-verbose

Expected:

• Test runs to completion and logs ERROR unexpected-crash-dump-found - This test left crash dumps behind, but we weren't expecting it to!

Actual:

• Logs JavaScript error: , line 0: uncaught exception: Object
• Test gets stuck, and logs FAIL Test timed out. - after 5 minutes.

This is a reduced test case from bug 2002676, https://bugzilla.mozilla.org/show_bug.cgi?id=2002676#c44

Explanation:

• The await SpecialPowers.findUnexpectedCrashDumpFiles() call in TestRunner.js resolves to a Proxy instance (from WrapPrivileged). This value is passed to TestRunner.structuredLogger.testStatus(...), and ends up being passed to Cu.cloneInto.
• Because cloneInto is not able to clone a Proxy instance, it raises an error Error: Encountered unsupported value type writing stack-scoped structured clone. For some reason this error cannot be displayed, and we get a useless JavaScript error: , line 0: uncaught exception: Object instead.

This is a regression by bug 1801368, which introduced the cloneInto call. Previously, the value would be passed, and callees who did not care would just ignore it.

To fix the issue, I propose:

• to modify findUnexpectedCrashDumpFiles to return a value cloned into the right scope, to avoid the need for the wrapper. And then call SpecialPowers.unwrap on its return value, to counter the automatic wrapping applied to Promise return values.
• to prevent similar errors from blowing up the test framework, modify StructuredLogger to catch serialization errors and display a meaningful warning.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1801368

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 2007587 - Fix findUnexpectedCrashDumpFiles to avoid breaking mochitest TestRunner

Comment 3

[Rob Wu [:robwu]]

Attachment: Bug 2007587 - Gracefully handle clone errors in StructuredLogger

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/198fc64bac82
https://hg.mozilla.org/integration/autoland/rev/109a1104752b
Fix findUnexpectedCrashDumpFiles to avoid breaking mochitest TestRunner r=ahal
https://github.com/mozilla-firefox/firefox/commit/45290297179b
https://hg.mozilla.org/integration/autoland/rev/d31fa9669c92
Gracefully handle clone errors in StructuredLogger r=ahal

Comment 5

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/109a1104752b
https://hg.mozilla.org/mozilla-central/rev/d31fa9669c92

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:robwu, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox147 to wontfix.

For more information, please visit BugBot documentation.