Instead of resolving with a network error Response.

Fetch spec section 5.6, step 3.1.

Comment on attachment 8549163 [details] [diff] [review] fetch() should reject with TypeError I'm sorry, but I don't feel comfortable enough with jsapi stuff to review this. Redirect to Andrea.

For reference, Bug 1107777 introduced the JSContext option I'm using.

So I guess I'm curious if you're saying that bug 1091091 was implemented incorrectly? Can you or some explain why Promise::MaybeReject(ErrorResult) doesn't work here?

(In reply to Ben Kelly [:bkelly] from comment #4) > So I guess I'm curious if you're saying that bug 1091091 was implemented > incorrectly? Can you or some explain why Promise::MaybeReject(ErrorResult) > doesn't work here? It only doesn't work when ErrorResult has failed with a TypeError. ToJSValue calls https://dxr.mozilla.org/mozilla-central/source/dom/bindings/BindingUtils.h#105 which leads to JS_ReportErrorNumberUCArray being called, which goes via the "Let's report instead of setting an exception on the cx, unless this flag is set or JS is running". If we always want to capture the error in ToJSValue, we should set the JSContext flag inside it.

Can we push down the cx option munging into the ToJSValue overload for ErrorResult, instead, please? I can't wait for the SpiderMonkey folks to fix their exception story....

Boris for JSAPI bits, Ben for making sure I'm following the fetch spec. Boris, I've put the class in Exceptions.h instead of ToJSValue.h since I also want to use it in Bug 1109742 in the dom/fetch/Fetch.cpp file. Is that ok?

Comment on attachment 8551932 [details] [diff] [review] fetch() should reject with TypeError >+++ b/dom/bindings/Exceptions.h >+// Throwing a TypeError on an ErrorResult may result in Spidermonkey using its Nix the extra space before "SpiderMonkey" (and note the casing). >+++ b/dom/bindings/ToJSValue.cpp >- > } // namespace tojsvalue_detail Please leave that blank line. >+ AutoForceSetExceptionOnContext forceExn(aCx); Just out of curiosity, did the MOZ_ASSERT that JS_GetPendingException returns true fail without this patch? I really hope it did... >+++ b/dom/fetch/Fetch.cpp >+ AutoSafeJSContext cx; This is unused, right? Please ditch it. r=me. Thank you for fixing this!

(In reply to Boris Zbarsky [:bz] from comment #8) > Comment on attachment 8551932 [details] [diff] [review] > fetch() should reject with TypeError > > >+++ b/dom/bindings/Exceptions.h > >+// Throwing a TypeError on an ErrorResult may result in Spidermonkey using its > > Nix the extra space before "SpiderMonkey" (and note the casing). > > >+++ b/dom/bindings/ToJSValue.cpp > >- > > } // namespace tojsvalue_detail > > Please leave that blank line. > > >+ AutoForceSetExceptionOnContext forceExn(aCx); > > Just out of curiosity, did the MOZ_ASSERT that JS_GetPendingException > returns true fail without this patch? I really hope it did... Yes it did.