Closed Bug 722596 Opened 8 years ago Closed 7 years ago

Get stack trace and error for failing async content scripts

Categories

(Testing :: Marionette, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mdas, Assigned: mdas)

References

Details

Attachments

(1 file, 2 obsolete files)

from Bug 721260, we should return the error messages when an async script has errors in content. Right now, it just throws a generic 'JavaScript Error'.  You can test it by using:

self.marionette.set_script_timeout(10000)
self.marionette.execute_async_script("""
setTimeout(function() {
  foo();
  marionetteScriptFinished(true);
}, 1000);
""")
Note: Had JS compartment isues getting window.onerror to work in content code so I had to use addEventListener. Related bug: Bug 720714
Attached patch patch and tests v1.0 (obsolete) — Splinter Review
This gets the stack trace back to the user, and includes tests. I refactored the test suite a bit since there was a bunch of boilerplate code.

I tested this in b2g on a pandaboard and it works as expected. This should help us get some visibility into what's causing some of the intermittent gaiatest failures.
Assignee: nobody → mdas
Attachment #692988 - Flags: review?(jgriffin)
Comment on attachment 692988 [details] [diff] [review]
patch and tests v1.0

Review of attachment 692988 [details] [diff] [review]:
-----------------------------------------------------------------

This patch makes me happy!

But, we should probably track the original onerror and restore it later, like we do in marionette-actors:  http://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-actors.js#850
Attachment #692988 - Flags: review?(jgriffin) → review-
Attached patch patch and tests v2.0 (obsolete) — Splinter Review
I neglected that concern! Thanks for finding that.
Attachment #692988 - Attachment is obsolete: true
Attachment #693069 - Flags: review?(jgriffin)
Comment on attachment 693069 [details] [diff] [review]
patch and tests v2.0

Review of attachment 693069 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!!
Attachment #693069 - Flags: review?(jgriffin) → review+
landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5aaf21d0a2b

Will land in mozilla-aurora and mozilla-b2g18 as a=NPOTB if this gets into m-c
Whiteboard: [leave-open]
stupid python2.6.

I'm keeping the first assertRaises to ensure that whatever exception is thrown from here is caught by unittest. If some other exception is thrown, then this test case will stop executing in a nice way, showing a FAIL instead of an ERROR. I then use the try/catch so that if the right exception is in fact thrown, then we can assess the error message. I can also do an except after the JavascriptException to catch any other exception thrown here, and then fail, but I found that ugly. I'm not really invested in either approach though.
Attachment #693069 - Attachment is obsolete: true
Attachment #693114 - Flags: review?(jgriffin)
btw, i tested it with python2.6 and it's happy.
Attachment #693114 - Flags: review?(jgriffin) → review+
Successfully landed!
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [leave-open]
You need to log in before you can comment on or make changes to this bug.