980468 - error event on page-mod or worker cannot catch a syntax error in the content script

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Anthony Shipman]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20140212131424

Steps to reproduce:

I added  onError: function(){} to the PageMod options to catch a SyntaxError in a content script. I also did worker.on('error', ...) in the onAttach function.




Actual results:

The error event handler in the PageMod options does not fire. The syntax error
is thrown while running the Worker constructor. At that time the only handler is
the  _onUncaughtError() handler passed to it by PageMod.  This handler passes the error to console.exception().

When writing to stdout the standard console.exception() method fails to show the file and line where the syntax error occurred so I need to catch it and log it myself. The Browser Console only barely understands an Error object.



Expected results:

An error while importing  a script into the content work (Worker._importScripts) should arrive at the onError handler in PageMod.

Comment 1

[Anthony Shipman]

Version 1.15

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Can you attach a copy of your main.js (and your content script files, if they're separate) to the bug so we can see exactly what's going on?

Comment 3

[Anthony Shipman]

Attachment: mm.js

The PageMod call is shown. The content script is anything with a syntax error so that it fails to load.

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I don't see an onError option supported for page-mods, so I don't know that it would do anything to add that.

Irakli, can you confirm that?

Comment 5

[Anthony Shipman]

The source code of PageMod says:

    if ('contentScriptWhen' in options)
      this.contentScriptWhen = options.contentScriptWhen;
    if ('onAttach' in options)
      this.on('attach', options.onAttach);
    if ('onError' in options)
      this.on('error', options.onError);
    if ('attachTo' in options) {
      if (typeof options.attachTo == 'string')

Comment 6

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

I beleive als original description is right, syntax errors thrown by content scripts are weird & they are
not regular objects. For module loader we have some workarounds to normalize them, which I guess we would
have to do for content scripts as well.

als do you wanna take stab at it and provide pull request ? Given what you already looked at it should
not be hard to fix.

Comment 7

[Anthony Shipman]

> als do you wanna take stab at it and provide pull request ? Given what you
> already looked at it should
> not be hard to fix.

I have a fix for this bug. But I can't assign it to me. Does someone have
to do something to the status first?

Comment 8

[Tomislav Jovanovic :zombie]

i have assigned the bug to you (you don't have rights to simply "take" bugs yet).

here is a guide for contributing to the Jetpack project:

https://github.com/mozilla/addon-sdk/wiki/contribute

in short: fork addon-sdk from github, commit your fix, issue a Pull Request, attach a link to that PR to this bug, and flag it for review? from someone here (like me).

Comment 9

[Anthony Shipman]

I am trying
cfx -f test-page-mod:testDetachOnUnload -b /usr/local/firefox-29.0b3/firefox test

FF 27 and 28 fail with:

Running tests on Firefox 28.0/Gecko 28.0 ({ec8030f7-c20a-464f-9b0e-13a3a9e97384}) under linux/x86.
Error: TypeError: Cc['@mozilla.org/memory-reporter-manager;1'].getService(...).getReportsForThisProcess is not a function
 Traceback (most recent call last):
  File "resource", line NaN, in notify
  File "resource", line NaN, in runTests/<
  File "resource", line NaN, in runTests
  File "resource", line NaN, in getPotentialLeaks
0 of 1 tests passed.


FF 29.0b3 fails with 

Running tests on Firefox 29.0/Gecko 29.0 ({ec8030f7-c20a-464f-9b0e-13a3a9e97384}) under linux/x86.
Error: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm -> jar:file:///tmp/tmpWTDFsv.mozrunner/extensions/[e-mail]!/bootstrap.js -> resource://extensions.modules.d3b03bc9-c751-49e8-a065-3b9313443aa1-at-jetpack.commonjs.path/toolkit/loader.js -> resource://extensions.modules.d3b03bc9-c751-49e8-a065-3b9313443aa1-at-jetpack.commonjs.path/sdk/loader/sandbox.js :: <TOP_LEVEL> :: line 20"  data: no]

Using -o with cfx doesn't help

Comment 10

[Tomislav Jovanovic :zombie]

you should be testing with Nightly firefox (version 31 atm), and the master branch of the SDK from github.

also, you must use -o for your module changes to take effect.

Comment 11

[Anthony Shipman]

Attachment: Patch for review

Comment 12

[Tomislav Jovanovic :zombie]

Comment on attachment 8399134 [details] [diff] [review]
Patch for review

Anthony: overall, this looks good, thanks!

sorry, i didn't finish my review last night, so there are a couple of more (minor) comments in the PR after your last commit.

r+ when you are done with that. also, can you please squash down your commits into one, as we prefer to merge bugfixes as a single contribution.

Comment 13

[Tomislav Jovanovic :zombie]

Comment on attachment 8399134 [details] [diff] [review]
Patch for review

thank you for your contribution, good work.

you can search for other bugs with "good first bug" flag [1], if you wish to continue contributing..

[1] https://bugzilla.mozilla.org/buglist.cgi?list_id=9786500&resolution=---&resolution=DUPLICATE&status_whiteboard_type=casesubstring&query_format=advanced&status_whiteboard=[good%20first%20bug]&product=Add-on%20SDK

Comment 14

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/d5bccb845483d6d1d0488c6bf0838b7000155195
bug 980468: Ensure that content script syntax errors can be caught.

https://github.com/mozilla/addon-sdk/commit/e944ea7d253542b0bf0741a67845be68ee7c4140
Merge pull request #1449 from als123/bug_980468

Bug 980468 - Ensure that content script syntax errors can be caught, r=@zombie