Closed Bug 1024811 Opened 5 years ago Closed 5 years ago
hawkclient will fail if no response headers
When we try and find retry related headers, the code assumes response.headers is non null - see http://hg.mozilla.org/mozilla-central/file/95cc116fac5a/services/common/hawkclient.js#l238 Note that the current tip of that file does *not* have the problem - it was fixed independently in revision fe1432e7b895. Also worrying is the fact that the exception caused by this is not reported anywhere - we had to deduce this from other log entries - see bug 1006360 comment 61 and 62 - but I'll open yet another bug for that as trunk *is* still affected by that, just not by the "null headers" problem here. (Note I'm not 100% sure about the status flags below - the fix is in revision fe1432e7b895 - so I *think* that means 32 and 33 are already fixed.)
This patch checks we have headers before attempting to use them, and also fixes a use of the log. This patch is against beta - the same patch landed on central in revision fe1432e7b895 as part of bug 988469 and is already in Aurora.
Assignee: nobody → mhammond
Status: NEW → ASSIGNED
Attachment #8439637 - Flags: review?(rnewman)
Marco, please add this to the current iteration
Whiteboard: p=2 [qa-]
Added to Iteration 33.1
Whiteboard: p=2 [qa-] → p=2 s=33.1 [qa-]
Comment on attachment 8439637 [details] [diff] [review] 0001-Bug-1024811-hawkclient-now-checks-if-headers-are-non.patch Review of attachment 8439637 [details] [diff] [review]: ----------------------------------------------------------------- Looks sane to me.
Attachment #8439637 - Flags: review?(rnewman) → review+
Comment on attachment 8439637 [details] [diff] [review] 0001-Bug-1024811-hawkclient-now-checks-if-headers-are-non.patch [Approval Request Comment] Bug caused by (feature/regressing bug #): FxA sync User impact if declined: Sync may work incorrectly on some network errors related to auth. Testing completed (on m-c, etc.): This same patch has already landed on m-c and Aurora via bug 988469 - this patch is to add the same code to beta. Risk to taking this patch (and alternatives if risky): Low String or IDL/UUID changes made by this patch: None As above, this needs to land on beta only - central and aurora already have the fix.
Attachment #8439637 - Flags: approval-mozilla-beta?
Attachment #8439637 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Iteration: --- → 33.2
Points: --- → 2
QA Whiteboard: [qa-]
Whiteboard: p=2 s=33.1 [qa-]
Landed on 31, which is the only place it needed to, so fixed.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Iteration: --- → 34.3
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.