Closed Bug 1413989 Opened 3 years ago Closed 3 years ago

setTimeout is undefined in MigrationUtils.getRowsFromDBWithoutLocks

Categories

(Firefox :: Migration, defect, P1)

57 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 58
Tracking Status
firefox-esr52 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: dthayer, Assigned: dthayer)

References

()

Details

(Keywords: regression)

Attachments

(1 file)

This breaks our retry logic in Chrome migrations.
The setTimout was originally added in bug 1285041 in ChromeProfileMigrator.js and was moved to MigrationUtils.jsm in bug 1305770.

Doug, I see you're assigned, are you working on this?

You should just be able to import Timer.jsm. Testing it will be more difficult as you'll need to force an exception. See the test_Chrome* files in https://dxr.mozilla.org/mozilla-central/source/browser/components/migration/tests/unit
Blocks: 1285041, 1305770
Flags: needinfo?(dothayer)
Keywords: regression
Priority: -- → P1
Ugh, I am dumb sometimes. How isn't this failing eslint? :-(
I also wonder if this is the cause of https://bugzilla.mozilla.org/show_bug.cgi?id=1332225#c34 .
(In reply to :Gijs (slow, PTO recovery mode) from comment #2)
> Ugh, I am dumb sometimes. How isn't this failing eslint? :-(

That's bug 1369722. With the ESLint 4 upgrade next week, it'll be possible that we can do it in a sane way now.
Blocks: 1369722
(In reply to :Gijs (slow, PTO recovery mode) from comment #3)
> I also wonder if this is the cause of
> https://bugzilla.mozilla.org/show_bug.cgi?id=1332225#c34 .

I couldn't find a way for this to result in the migrator hanging. Forcing an error in order to hit this block just resulted in nothing being imported.

(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #1)
> The setTimout was originally added in bug 1285041 in
> ChromeProfileMigrator.js and was moved to MigrationUtils.jsm in bug 1305770.
> 
> Doug, I see you're assigned, are you working on this?
> 
> You should just be able to import Timer.jsm. Testing it will be more
> difficult as you'll need to force an exception. See the test_Chrome* files
> in
> https://dxr.mozilla.org/mozilla-central/source/browser/components/migration/
> tests/unit

Woops. I filed this while in the middle looking into the performance of migrations and promptly forgot I had created it and also assigned myself. But yes, I intended to work on it and I'll get a patch up shortly.
Flags: needinfo?(dothayer)
Comment on attachment 8926146 [details]
Bug 1413989 - include Timer.jsm in MigrationUtils.jsm

https://reviewboard.mozilla.org/r/197384/#review202640
Attachment #8926146 - Flags: review?(MattN+bmo) → review+
Thanks
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7d8c4617ad28
include Timer.jsm in MigrationUtils.jsm r=MattN
Backout by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c8659b2921a
Backed out 1 changesets for ESLint failure "browser/components/migration/tests/unit/test_MigrationUtils_timedRetry.js" r=backout on a CLOSED TREE
(In reply to Pulsebot from comment #10)
> Backout by nbeleuzu@mozilla.com:
> https://hg.mozilla.org/integration/autoland/rev/0c8659b2921a
> Backed out 1 changesets for ESLint failure
> "browser/components/migration/tests/unit/test_MigrationUtils_timedRetry.js"
> r=backout on a CLOSED TREE

Welp, that will teach me to be pushing other people's patches - sorry!
(In reply to :Gijs (slow, PTO recovery mode) from comment #11)
> Welp, that will teach me to be pushing other people's patches - sorry!

Eh, my bad for not habitually linting it before pushing to review.
Pushed by dothayer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d0280b48ef58
include Timer.jsm in MigrationUtils.jsm r=MattN
https://hg.mozilla.org/mozilla-central/rev/d0280b48ef58
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.