Closed Bug 1024812 Opened 5 years ago Closed 5 years ago

Exceptions in hawkclient will cause sync to silently hang

Categories

(Firefox :: Firefox Accounts, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 33
Tracking Status
firefox30 --- wontfix
firefox31 + fixed
firefox32 + fixed
firefox33 + fixed

People

(Reporter: markh, Assigned: markh)

Details

(Whiteboard: p=2 s=33.1 [qa-])

Attachments

(1 file)

See bug 1024811 for an example of an exception that isn't reported and causes sync to hang.  We should add an exception handler that always rejects the promise on unhandled exceptions.
Flags: firefox-backlog+
Only feedback for now as I wanted to see how you felt about a lack of tests - I tried, but failed to come up with a test that wasn't so artificial as to be useless without leading to extensive mocking of RESTRequest itself.  If you are fine with the lack of tests, feel free to treat this as a review request ;)
Assignee: nobody → mhammond
Status: NEW → ASSIGNED
Attachment #8439651 - Flags: feedback?(rnewman)
Marco, please add this to the current iteration
Flags: needinfo?(mmucci)
Whiteboard: p=2 [qa-]
Added to Iteration 33.1
Flags: needinfo?(mmucci)
Whiteboard: p=2 [qa-] → p=2 s=33.1 [qa-]
Comment on attachment 8439651 [details] [diff] [review]
0001-Bug-1024812-unhandled-exceptions-in-hawkclient-no-lo.patch

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

If existing tests pass with this, it's good enough for me. Keep an eye on this for uplift.
Attachment #8439651 - Flags: review+
Attachment #8439651 - Flags: feedback?(rnewman)
Attachment #8439651 - Flags: feedback+
https://hg.mozilla.org/mozilla-central/rev/b2ae20690ef2
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment on attachment 8439651 [details] [diff] [review]
0001-Bug-1024812-unhandled-exceptions-in-hawkclient-no-lo.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): FxA Sync
User impact if declined: Sync may hang on network errors
Testing completed (on m-c, etc.): Landed m-c
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None
Attachment #8439651 - Flags: approval-mozilla-beta?
Attachment #8439651 - Flags: approval-mozilla-aurora?
Attachment #8439651 - Flags: approval-mozilla-beta?
Attachment #8439651 - Flags: approval-mozilla-beta+
Attachment #8439651 - Flags: approval-mozilla-aurora?
Attachment #8439651 - Flags: approval-mozilla-aurora+
Product: Core → Firefox
Target Milestone: mozilla33 → Firefox 33
You need to log in before you can comment on or make changes to this bug.