Closed Bug 980468 Opened 10 years ago Closed 10 years ago

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


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



(Not tracked)



(Reporter: als, Assigned: als)



(2 files)

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.
Version 1.15
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?
Flags: needinfo?(als)
Attached file mm.js
The PageMod call is shown. The content script is anything with a syntax error so that it fails to load.
Flags: needinfo?(als)
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?
Flags: needinfo?(rFobic)
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')
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.
Flags: needinfo?(rFobic)
> 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?
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:

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).
Assignee: nobody → als
Ever confirmed: true
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[';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/d3b03bc9-c751-49e8-a065-3b9313443aa1@jetpack.xpi!/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
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.
Attached patch Patch for reviewSplinter Review
Attachment #8399134 - Flags: review?(tomica+amo)
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 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..

Attachment #8399134 - Flags: review?(tomica+amo) → review+
Commits pushed to master at
bug 980468: Ensure that content script syntax errors can be caught.
Merge pull request #1449 from als123/bug_980468

Bug 980468 - Ensure that content script syntax errors can be caught, r=@zombie
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.