614753 - reddit-panel example addon throws "Object.getPrototypeOf(event.constructor.prototype) is null"

Status: RESOLVED FIXED

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

Description

[Myk Melez [:myk] [@mykmelez]]

Running the reddit-panel example addon on both the Firefox tip and 4.0b7 throws an exception:

error: An exception occurred.
Traceback (most recent call last):
  File "resource://anonid0-reddit-panel-jetpack-core-lib/timer.js", line 64, in notifyOnTimeout
    this._callback.apply(null, this._params);
  File "resource://anonid0-reddit-panel-jetpack-core-lib/content/worker.js", line 61, in emitter
    emit.apply(scope, params);
  File "resource://anonid0-reddit-panel-jetpack-core-lib/events.js", line 131, in _emit
    'Error' == Object.getPrototypeOf(event.constructor.prototype).name
TypeError: Object.getPrototypeOf(event.constructor.prototype) is null

The exception does not occur when running the addon on Firefox 4.0b6, so it appears to be a regression between b6 and b7.  Per bug 614613, comment 0, Irakli suspects it is compartments-related and similar to bug 608959, since the code in question is calling Object.getPrototypeOf on an object from another sandbox.

I haven't been able to reduce a testcase yet, but to reproduce the problem, enter the SDK's examples/reddit-panel/ directory and execute "cfx run".

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Irakli: you mentioned last week that you were trying to reduce a testcase for this.  Have you made any progress?

Note that the fix for bug 615081 suppresses the exception in the content script that reveals this problem.  That doesn't mean it isn't still a problem, only that it won't manifest in the reddit-panel example addon after that fix.

Comment 2

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

(In reply to comment #1)
> Irakli: you mentioned last week that you were trying to reduce a testcase for
> this.  Have you made any progress?
> 
> Note that the fix for bug 615081 suppresses the exception in the content script
> that reveals this problem.  That doesn't mean it isn't still a problem, only
> that it won't manifest in the reddit-panel example addon after that fix.

I did not manged to find out why does this bug occurs unfortunately and I really would like to find it out, but at the moment I would like to suggest a different fix, I actually thought of changing default behavior of `EventEmitter` from throwing on unhandled exception to a logging it, since all the consumers just log it anyway. Also I think that there is nothing that object derived from EventEmitter can do when one of the listeners throw. So here is a patch with a fix: https://github.com/mozilla/addon-sdk/pull/55

I also think that finding a reason why Object.getPrototypeOf(event.constructor.prototype) misbehaves should not be a beta blocker.

Comment 3

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

Attachment: https://github.com/mozilla/addon-sdk/pull/55

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 493670 [details] [diff] [review]
https://github.com/mozilla/addon-sdk/pull/55

> I did not manged to find out why does this bug occurs unfortunately and I
> really would like to find it out, but at the moment I would like to suggest a
> different fix, I actually thought of changing default behavior of
> `EventEmitter` from throwing on unhandled exception to a logging it, since all
> the consumers just log it anyway. Also I think that there is nothing that
> object derived from EventEmitter can do when one of the listeners throw. So
> here is a patch with a fix: https://github.com/mozilla/addon-sdk/pull/55

Yup, that makes sense.


> I also think that finding a reason why
> Object.getPrototypeOf(event.constructor.prototype) misbehaves should not be a
> beta blocker.

Agreed, since your patch works around that problem.


>+    if (type === ERROR_TYPE && !listeners.length)
>+      console.exception(event)

Nit: semi-colon after statement.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

I have cherry-picked your change with the nit fixed <https://github.com/mozilla/addon-sdk/commit/958959f0573c2128be541665e9953693ab81c49f>.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Urk, this is causing test bustage.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Change backed out due to test bustage <https://github.com/mozilla/addon-sdk/commit/7333003537a4b7a0ec1eff447c22ee3f6e1f5ada>.

Comment 8

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

Attachment: https://github.com/mozilla/addon-sdk/pull/63

Fixes test failures