Closed Bug 1024812 Opened 10 years ago Closed 10 years ago

Exceptions in hawkclient will cause sync to silently hang

Categories

(Firefox :: Firefox Accounts, defect)

Product:
Firefox
Component:
Firefox Accounts
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 33
Tracking Flags:
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)

0001-Bug-1024812-unhandled-exceptions-in-hawkclient-no-lo.patch
2.14 KB, patch
rnewman
: review+
rnewman
: feedback+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review
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: 10 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.

Attachment

General

Creator:
Created:
Updated:
Size: