Closed
Bug 857393
Opened 12 years ago
Closed 12 years ago
XHR in Web Workers fails to throw XMLHttpRequest::Send failures to callers (ex: NS_ERROR_FILE_NOT_FOUND), exception may be reported erroneously elsewhere
Categories
(Core :: DOM: Workers, defect)
Tracking
()
People
(Reporter: asuth, Assigned: asuth)
References
Details
Attachments
(1 file)
1.96 KB,
patch
|
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
It is possible for (non-worker) XMLHttpRequest::Send to return a non-NS_OK value. For example, in bug 827243, NS_ERROR_FILE_NOT_FOUND can be returned because AsyncOpen may be able to resolve that a file does not exist in an app:// protocol jar in a synchronous fasion.
(worker) XMLHttpRequest::SendInternal currently returns when !runnable->Dispatch(cx) without modifying aRv, and as a result the error is not propagated to the caller. However, JS_SetPendingException does get called, so an exception will be tracked on the context, and can end up showing up in misleading places. For example, :jrburke observed that we might observe the error in importScripts if we tried to perform a synchronous XHR request.
The attached patch created with the guidance of bent resolves the problem affecting the gaia e-mail app. The specific diff provided is for mozilla-central with crazy context, but the patch applies to mozilla-b2g18 (at least with a smaller context).
I have pushed the patch to try at:
https://tbpl.mozilla.org/?tree=Try&rev=1757a26ab7b4
This blocks the tef+ bug 814257 to move most of the e-mail app's backend to a worker thread and should therefore be tracked as a blocker and allowed to uplift to v1.0.1 and v1-train. It's a 1-line fix to properly propagate an error code, so I would argue that it is low risk, although bent can be more authoritative on the matter.
Attachment #732617 -
Flags: review?(bent.mozilla)
Comment on attachment 732617 [details] [diff] [review]
fix as discussed with bent; modify aRV
Review of attachment 732617 [details] [diff] [review]:
-----------------------------------------------------------------
r=me. Sorry about that!
Attachment #732617 -
Flags: review?(bent.mozilla) → review+
Assignee | ||
Comment 2•12 years ago
|
||
landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e418e5123168
Comment 4•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Assignee | ||
Comment 5•12 years ago
|
||
uplifted:
https://hg.mozilla.org/releases/mozilla-b2g18/rev/68caf3b9d213
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/5ace431c0dd5
status-b2g18:
--- → fixed
status-b2g18-v1.0.1:
--- → fixed
Assignee | ||
Updated•12 years ago
|
Comment 6•12 years ago
|
||
Andrew, can you comment on how QA can verify this patch?
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Tony Chung [:tchung] from comment #6)
> Andrew, can you comment on how QA can verify this patch?
I wouldn't specifically test this patch, but you want to make sure you have a moztrap case that covers creating an account that's not one of the domain names in here: https://github.com/mozilla-b2g/gaia/tree/master/apps/email/autoconfig
Without the patch, that will fail *when running with the back-end on worker-thread from bug 814257*.
You need to log in
before you can comment on or make changes to this bug.
Description
•