1036198 - rejection is not defined in this.FxAccountsManager._handleGetAssertionError

Status: RESOLVED FIXED

(Firefox :: Firefox Accounts, defect)

Description

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

FxAccountsManager.jsm, line 207; 'rejection' is not defined.

Mud in my eye, as I reviewed this patch and this error slipped by me.

Comment 1

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: fix default case in _handleGetAssertionError; unit test

Hi, Sam,

I think this does the trick?

Thanks for taking a look,
j

Comment 2

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Oh - it appears that xpcshell tests with our httpd_server are currently broken in trunk.  I have no idea why.  I get this error when running test_fxaccounts.js:

 0:09.04 [24755] WARNING: cannot post event if not initialized: file /Users/zeus/code/firefox/mozilla-central/netwerk/protocol/http/nsHttpConnectionMgr.cpp, line 192
 0:09.05 Assertion failed: (GLOBAL(int, mutexIsInit)), function sqlite3MutexAlloc, file /Users/zeus/code/firefox/mozilla-central/db/sqlite3/src/sqlite3.c, line 18408.

It appears to get thrown when invoking server.stop().

It's not related to this patch.

Comment 3

[Sam Penrose]

Comment on attachment 8452806 [details] [diff] [review]
fix default case in _handleGetAssertionError; unit test

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

Thank you very much for catching this. My bad for not properly linting.

Comment 4

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Thanks, Sam!  No worries.  We both missed it :)

Here's ye try run:

https://tbpl.mozilla.org/?tree=Try&rev=bd294e351808

Comment 5

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

I seem to have lost some bugzilla privileges, and am unable to assign this bug to me or change the Status.

Comment 6

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Hey Jed or Sam - is there any user impact from this bug?

Comment 7

[Sam Penrose]

(In reply to Jared Hirsch [:_6a68] [@6a68] from comment #6)
> Hey Jed or Sam - is there any user impact from this bug?

We don't know. I made a mistake in an error-handling code path that "shouldn't be reached." If it is reached, we'll get a ReferenceError thrown. Jed's fix means that proper error-handling will proceed. Seems like risk-reducer to me.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5cbe17e56507

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5cbe17e56507

Comment 10

[bhavana bajaj [:bajaj]]

Please seek aurora approval here to get this uplifted since user impact is not clear.

Comment 11

[bhavana bajaj [:bajaj]]

(In reply to bhavana bajaj [:bajaj] [NOT reading Bugmail, needInfo please] from comment #10)
> Please seek aurora approval here to get this uplifted since user impact is
> not clear.

hit return to fast..

Please seek aurora approval here to get this uplifted since user impact is not clear to mkae it a blocker and this looks a trivial check to me

Comment 12

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Comment on attachment 8452806 [details] [diff] [review]
fix default case in _handleGetAssertionError; unit test

Approval Request Comment
[Feature/regressing bug #]: fixes an error from bug 1004319
[User impact if declined]: low; this fixes a ReferenceError that would be thrown if the user followed an unlikely code path
[Describe test coverage new/current, TBPL]: added unit test to patch
[Risks and why]: none that I see.  This improves stability overall.
[String/UUID change made/needed]: none

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/629db6ea4788