Exception thrown and uncaught in a Task: TypeError: data.set is not a function

RESOLVED FIXED in Firefox 31

Status

Firefox Health Report
Client: Desktop
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Irving, Assigned: Benjamin Smedberg)

Tracking

Trunk
Firefox 33
Points:
---

Firefox Tracking Flags

(firefox30 unaffected, firefox31+ fixed, firefox32+ fixed, firefox33+ fixed)

Details

Attachments

(2 attachments)

Came back to a Nightly that I had left running in a terminal window, and saw this:

*************************
A coding exception was thrown and uncaught in a Task.

Full message: TypeError: data.set is not a function
Full stack: Experiments.ExperimentEntry.prototype.initFromCacheData@resource://app/modules/experiments/Experiments.jsm:1456:9
Experiments.Experiments.prototype._populateFromCache@resource://app/modules/experiments/Experiments.jsm:990:12
Experiments.Experiments.prototype._loadFromCache<@resource://app/modules/experiments/Experiments.jsm:971:7
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:866:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:745:7
exports.Utils.yield@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/ireid/Library/Application%20Support/Firefox/Profiles/sngia9xd.debug/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/bootstrap.js -> jar:file:///Users/ireid/Library/Application%20Support/Firefox/Profiles/sngia9xd.debug/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/lib/utils.js:380:12
INIParser.prototype.process@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/ireid/Library/Application%20Support/Firefox/Profiles/sngia9xd.debug/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/bootstrap.js -> jar:file:///Users/ireid/Library/Application%20Support/Firefox/Profiles/sngia9xd.debug/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/lib/filterStorage.js:892:7
exports.IO.readFromFile/onProgress@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/ireid/Library/Application%20Support/Firefox/Profiles/sngia9xd.debug/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/bootstrap.js -> jar:file:///Users/ireid/Library/Application%20Support/Firefox/Profiles/sngia9xd.debug/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/lib/io.js:104:15
exports.IO.readFromFile/<@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/ireid/Library/Application%20Support/Firefox/Profiles/sngia9xd.debug/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/bootstrap.js -> jar:file:///Users/ireid/Library/Application%20Support/Firefox/Profiles/sngia9xd.debug/extensions/%7Bd10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d%7D.xpi!/lib/io.js:213:13
TaskImpl_run@resource://gre/modules/Task.jsm:330:41
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:866:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:745:7
(Assignee)

Comment 1

3 years ago
Crap. I thought it was a Map() but it's just an object. Upgrade code sucks.
Assignee: nobody → benjamin
(Assignee)

Comment 2

3 years ago
Created attachment 8447187 [details] [diff] [review]
experiments-not-map
Attachment #8447187 - Flags: review?(georg.fritzsche)
Comment on attachment 8447187 [details] [diff] [review]
experiments-not-map

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

Can we please add basic test-coverage for this?
Attachment #8447187 - Flags: review?(georg.fritzsche) → review+
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
(Assignee)

Comment 4

3 years ago
Created attachment 8447229 [details] [diff] [review]
experiments-not-map, rev. 2

How does this look? It's very unit-testy, but in this case I think that's better than trying to write a custom manifest and then read it.
Attachment #8447229 - Flags: review?(georg.fritzsche)
Comment on attachment 8447229 [details] [diff] [review]
experiments-not-map, rev. 2

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

I think we have enough coverage of the cache writing/reading path, so directly checking this seems good to me.

::: browser/experiments/test/xpcshell/test_upgrade.js
@@ +4,5 @@
> +"use strict";
> +Cu.import("resource:///modules/experiments/Experiments.jsm");
> +
> +const SEC_IN_ONE_DAY  = 24 * 60 * 60;
> +const MS_IN_ONE_DAY   = SEC_IN_ONE_DAY * 1000;

The constants are unused.

@@ +6,5 @@
> +
> +const SEC_IN_ONE_DAY  = 24 * 60 * 60;
> +const MS_IN_ONE_DAY   = SEC_IN_ONE_DAY * 1000;
> +
> +let cd = {

|let cacheData = ...|?
Attachment #8447229 - Flags: review?(georg.fritzsche) → review+
(Assignee)

Comment 6

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/529a9b954b72
status-firefox30: --- → unaffected
status-firefox31: --- → affected
status-firefox32: --- → affected
status-firefox33: --- → affected
tracking-firefox31: --- → +
tracking-firefox32: --- → +
tracking-firefox33: --- → +
(Assignee)

Comment 7

3 years ago
Comment on attachment 8447229 [details] [diff] [review]
experiments-not-map, rev. 2

Approval Request Comment
[Feature/regressing bug #]: regressed with bug 1017806
[User impact if declined]: Users who used Firefox prior to bug 1017806 landing will not be able to run any experiments until this bug is fixed.
[Describe test coverage new/current, TBPL]: basic automated tests
[Risks and why]: all risk localized to the experiment system
[String/UUID change made/needed]: none
Attachment #8447229 - Flags: approval-mozilla-beta?
Attachment #8447229 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1031531
https://hg.mozilla.org/mozilla-central/rev/529a9b954b72
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox33: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
I wonder if we could get an expedited approval for this patch? We are about to release two experiments this week (translation on aurora and search on beta), and experiments don't work on builds affected by this bug.
Flags: needinfo?(lmandel)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> [User impact if declined]: Users who used Firefox prior to bug 1017806
> landing will not be able to run any experiments until this bug is fixed.

Does this mean any user on a build older than June 26 (when bug 1017806 landed on Aurora and Beta) cannot participate in the experiment? Any user who was on a build from before bug 1017806 landed and updated to a build with bug 1017806 can not participate in experiments?
Flags: needinfo?(lmandel)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> [Describe test coverage new/current, TBPL]: basic automated tests

Given that this just landed on m-c, have you verified that experiments function as expected with this fix? Are you confident that further last minute (for the two experiments that Felipe listed) changes won't be required?
Felipe, beta 6 will gtb tomorrow. Could you answer to Lawrence's questions?
Thanks
Flags: needinfo?(felipc)
I had replied on an e-mail, pasting here:

> 1. Does this mean any user on a build older than June 26 (when bug 1017806
> landed on Aurora and Beta) cannot participate in the experiment? Any user
> who was on a build from before bug 1017806 landed and updated to a build
> with bug 1017806 can not participate in experiments?

Users could get the experiment before June 26 (i.e, the translation experiment doesn't require bug 1017806). But if, after having receiving the experiment, they update to a build after June 26, the experiment system won't initialize properly and the translation experiment will be broken until they update again to a build with a fix for bug 1031326.

Users who had not yet received the experiment won't receive them until they are on a build with 1031326 fixed.

>
> Felipe asked for some uplift approvals last week in order to have adequate
> time for people to update before the experiment began. Does bug 1031326 also
> require that people update to the latest build in order for the experiment
> to function correctly? If so, do we need to push out the start date for the
> experiment in order to get enough uptake of suitable Aurora and beta builds?

Very likely. But we do not need to push the start date for deploying it, as the system supports setting a "minimum build date" in which the experiment should be activated. Previously I had set it to the build from 2014-06-21 which is the date when the approvals I requested last week landed. But we will probably need to bump it to 2014-06-30 (assuming I push it today) and then as users gradually update to this build they will start receiving the experiment.


>
> 2. Given that this just landed on m-c, have you verified that experiments
> function as expected with this fix? Are you confident that further last
> minute (for the two experiments that Felipe listed) changes won't be
> required?
>
> I don't think our typical process of bake time on Nightly is going to buy us
> anything if experiments are not enabled. Has the experiment been tested by
> yourselves or QA to ensure that it functions as expected? Can we shake out
> any other bugs before this hits our trial audience?

Right, there's not much point in letting it bake on Nightly untested. I have verified that the translation experiment works properly with the fix. I haven't checked the search experiment yet but I have no reason to believe it would be any different.
Flags: needinfo?(felipc)
Comment on attachment 8447229 [details] [diff] [review]
experiments-not-map, rev. 2

While later in beta than I would usually like to see a non critical change, given the nature of the change (one line and contained to experiments) and the desire to kick off an experiment in this cycle, I'm going to plus this one. As we need to lock down beta, if there are further changes required, we should consider whether we can push the search experiment to the next cycle.

Aurora+ as well. I look forward to seeing the results of these experiments.
Attachment #8447229 - Flags: approval-mozilla-beta?
Attachment #8447229 - Flags: approval-mozilla-beta+
Attachment #8447229 - Flags: approval-mozilla-aurora?
Attachment #8447229 - Flags: approval-mozilla-aurora+
Benjamin, Felipe - Can one of you please handle the landing to ensure this gets in a build ASAP?
Flags: needinfo?(felipc)
Flags: needinfo?(benjamin)
Done! Thanks for the approvals.

https://hg.mozilla.org/releases/mozilla-aurora/rev/44d31566a3a6
https://hg.mozilla.org/releases/mozilla-beta/rev/c06faa2cf62b
status-firefox31: affected → fixed
status-firefox32: affected → fixed
Flags: needinfo?(felipc)
Flags: needinfo?(benjamin)
You need to log in before you can comment on or make changes to this bug.