Closed Bug 980468 Opened 11 years ago Closed 11 years ago

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

Categories

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

defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla31

People

(Reporter: als, Assigned: als)

Details

Attachments

(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: 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).
Assignee: nobody → als
Status: UNCONFIRMED → ASSIGNED
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['@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/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.. [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
Attachment #8399134 - Flags: review?(tomica+amo) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: