Bug 1121682 - fetch() should reject with TypeError

--HG--
extra : rebase_source : 9e8d5e193695b856280c769dccc128c4ce4797d3

dom/bindings/Errors.msg

@@ -63,4 +63,5 @@ MSG_DEF(MSG_MISSING_REQUIRED_DICTIONARY_MEMBER, 1, "Missing required {0}.")
 MSG_DEF(MSG_INVALID_REQUEST_METHOD, 1, "Invalid request method {0}.")
 MSG_DEF(MSG_REQUEST_BODY_CONSUMED_ERROR, 0, "Request body has already been consumed.")
 MSG_DEF(MSG_RESPONSE_INVALID_STATUSTEXT_ERROR, 0, "Response statusText may not contain newline or carriage return.")
+MSG_DEF(MSG_FETCH_FAILED, 0, "NetworkError when attempting to fetch resource.")
 MSG_DEF(MSG_DEFINE_NON_CONFIGURABLE_PROP_ON_WINDOW, 0, "Not allowed to define a non-configurable property on the WindowProxy object")

dom/bindings/Exceptions.cpp

@@ -186,6 +186,18 @@ GetCurrentJSStack()
   return stack.forget();
 }
 
+AutoForceSetExceptionOnContext::AutoForceSetExceptionOnContext(JSContext* aCx)
+  : mCx(aCx)
+{
+  mOldValue = JS::ContextOptionsRef(mCx).autoJSAPIOwnsErrorReporting();
+  JS::ContextOptionsRef(mCx).setAutoJSAPIOwnsErrorReporting(true);
+}
+
+AutoForceSetExceptionOnContext::~AutoForceSetExceptionOnContext()
+{
+  JS::ContextOptionsRef(mCx).setAutoJSAPIOwnsErrorReporting(mOldValue);
+}
+
 namespace exceptions {
 
 class StackFrame : public nsIStackFrame

dom/bindings/Exceptions.h

@@ -44,6 +44,19 @@ CreateException(JSContext* aCx, nsresult aRv, const char* aMessage = nullptr);
 already_AddRefed<nsIStackFrame>
 GetCurrentJSStack();
 
+// Throwing a TypeError on an ErrorResult may result in SpiderMonkey using its
+// own error reporting mechanism instead of just setting the exception on the
+// context.  This happens if no script is running. Bug 1107777 adds a flag that
+// forcibly turns this behaviour off. This is a stack helper to set the flag.
+class MOZ_STACK_CLASS AutoForceSetExceptionOnContext {
+private:
+  JSContext* mCx;
+  bool mOldValue;
+public:
+  explicit AutoForceSetExceptionOnContext(JSContext* aCx);
+  ~AutoForceSetExceptionOnContext();
+};
+
 // Internal stuff not intended to be widely used.
 namespace exceptions {
 

dom/bindings/ToJSValue.cpp

@@ -66,6 +66,7 @@ ToJSValue(JSContext* aCx,
           JS::MutableHandle<JS::Value> aValue)
 {
   MOZ_ASSERT(aArgument.Failed());
+  AutoForceSetExceptionOnContext forceExn(aCx);
   DebugOnly<bool> throwResult = ThrowMethodFailedWithDetails(aCx, aArgument, "", "");
   MOZ_ASSERT(!throwResult);
   DebugOnly<bool> getPendingResult = JS_GetPendingException(aCx, aValue);

dom/fetch/Fetch.cpp

@@ -266,9 +266,15 @@ MainThreadFetchResolver::OnResponseAvailable(InternalResponse* aResponse)
   NS_ASSERT_OWNINGTHREAD(MainThreadFetchResolver);
   AssertIsOnMainThread();
 
-  nsCOMPtr<nsIGlobalObject> go = mPromise->GetParentObject();
-  mResponse = new Response(go, aResponse);
-  mPromise->MaybeResolve(mResponse);
+  if (aResponse->Type() != ResponseType::Error) {
+    nsCOMPtr<nsIGlobalObject> go = mPromise->GetParentObject();
+    mResponse = new Response(go, aResponse);
+    mPromise->MaybeResolve(mResponse);
+  } else {
+    ErrorResult result;
+    result.ThrowTypeError(MSG_FETCH_FAILED);
+    mPromise->MaybeReject(result);
+  }
 }
 
 MainThreadFetchResolver::~MainThreadFetchResolver()
@@ -296,11 +302,18 @@ public:
     aWorkerPrivate->AssertIsOnWorkerThread();
     MOZ_ASSERT(aWorkerPrivate == mResolver->GetWorkerPrivate());
 
-    nsRefPtr<nsIGlobalObject> global = aWorkerPrivate->GlobalScope();
-    mResolver->mResponse = new Response(global, mInternalResponse);
-
     nsRefPtr<Promise> promise = mResolver->mFetchPromise.forget();
-    promise->MaybeResolve(mResolver->mResponse);
+
+    if (mInternalResponse->Type() != ResponseType::Error) {
+      nsRefPtr<nsIGlobalObject> global = aWorkerPrivate->GlobalScope();
+      mResolver->mResponse = new Response(global, mInternalResponse);
+
+      promise->MaybeResolve(mResolver->mResponse);
+    } else {
+      ErrorResult result;
+      result.ThrowTypeError(MSG_FETCH_FAILED);
+      promise->MaybeReject(result);
+    }
 
     return true;
   }
@@ -322,7 +335,6 @@ public:
     MOZ_ASSERT(aWorkerPrivate);
     aWorkerPrivate->AssertIsOnWorkerThread();
     MOZ_ASSERT(aWorkerPrivate == mResolver->GetWorkerPrivate());
-    MOZ_ASSERT(mResolver->mResponse);
 
     mResolver->CleanUp(aCx);
     return true;

dom/tests/mochitest/fetch/worker_test_fetch_basic.js

@@ -23,7 +23,9 @@ function testAboutURL() {
   });
 
   var p2 = fetch('about:config').then(function(res) {
-    is(res.type, "error", "about:config should fail");
+    ok(false, "about:config should fail");
+  }, function(e) {
+    ok(e instanceof TypeError, "about:config should fail");
   });
 
   return Promise.all([p1, p2]);

dom/tests/mochitest/fetch/worker_test_fetch_basic_http.js

@@ -43,8 +43,9 @@ function testURLFail() {
   var promises = [];
   failFiles.forEach(function(entry) {
     var p = fetch(entry[0]).then(function(res) {
-      ok(res.type === "error", "Response should be an error for " + entry[0]);
-      is(res.status, 0, "Response status should be 0 for " + entry[0]);
+      ok(false, "Response should be an error for " + entry[0]);
+    }, function(e) {
+      ok(e instanceof TypeError, "Response should be an error for " + entry[0]);
     });
     promises.push(p);
   });

dom/tests/mochitest/fetch/worker_test_fetch_cors.js

@@ -24,7 +24,9 @@ function testModeSameOrigin() {
   // Fetch spec Section 4, step 4, "request's mode is same-origin".
   var req = new Request("http://example.com", { mode: "same-origin" });
   return fetch(req).then(function(res) {
-    ok(isNetworkError(res), "Attempting to fetch a resource from a different origin with mode same-origin should fail.");
+    ok(false, "Attempting to fetch a resource from a different origin with mode same-origin should fail.");
+  }, function(e) {
+    ok(e instanceof TypeError, "Attempting to fetch a resource from a different origin with mode same-origin should fail.");
   });
 }
 
@@ -660,62 +662,43 @@ function testModeCors() {
                                          headers: req.headers, body: req.body });
     fetches.push((function(test, request) {
       return fetch(request).then(function(res) {
-      dump("Response for " + request.url + "\n");
-        if (test.pass) {
-          ok(!isNetworkError(res),
-            "shouldn't have failed in test for " + test.toSource());
-          if (test.status) {
-            is(res.status, test.status, "wrong status in test for " + test.toSource());
-            is(res.statusText, test.statusMessage, "wrong status text for " + test.toSource());
-          }
-          else {
-            is(res.status, 200, "wrong status in test for " + test.toSource());
-            is(res.statusText, "OK", "wrong status text for " + test.toSource());
-          }
-          if (test.responseHeaders) {
-            for (header in test.responseHeaders) {
-              if (test.expectedResponseHeaders.indexOf(header) == -1) {
-                is(res.headers.has(header), false,
-                   "|Headers.has()|wrong response header (" + header + ") in test for " +
-                   test.toSource());
-              }
-              else {
-                is(res.headers.get(header), test.responseHeaders[header],
-                   "|Headers.get()|wrong response header (" + header + ") in test for " +
-                   test.toSource());
-              }
-            }
-          }
-
-          return res.text().then(function(v) {
-            if (test.method !== "HEAD") {
-              is(v, "<res>hello pass</res>\n",
-                 "wrong responseText in test for " + test.toSource());
-            }
-            else {
-              is(v, "",
-                 "wrong responseText in HEAD test for " + test.toSource());
-            }
-          });
+        ok(test.pass, "Expected test to pass for " + test.toSource());
+        if (test.status) {
+          is(res.status, test.status, "wrong status in test for " + test.toSource());
+          is(res.statusText, test.statusMessage, "wrong status text for " + test.toSource());
         }
         else {
-          ok(isNetworkError(res),
-            "should have failed in test for " + test.toSource());
-          is(res.status, 0, "wrong status in test for " + test.toSource());
-          is(res.statusText, "", "wrong status text for " + test.toSource());
-          if (test.responseHeaders) {
-            for (header in test.responseHeaders) {
-              is(res.headers.get(header), null,
-                 "wrong response header (" + header + ") in test for " +
+          is(res.status, 200, "wrong status in test for " + test.toSource());
+          is(res.statusText, "OK", "wrong status text for " + test.toSource());
+        }
+        if (test.responseHeaders) {
+          for (header in test.responseHeaders) {
+            if (test.expectedResponseHeaders.indexOf(header) == -1) {
+              is(res.headers.has(header), false,
+                 "|Headers.has()|wrong response header (" + header + ") in test for " +
+                 test.toSource());
+            }
+            else {
+              is(res.headers.get(header), test.responseHeaders[header],
+                 "|Headers.get()|wrong response header (" + header + ") in test for " +
                  test.toSource());
             }
           }
-
-          return res.text().then(function(v) {
-            is(v, "",
-               "wrong responseText in test for " + test.toSource());
-          });
         }
+
+        return res.text().then(function(v) {
+          if (test.method !== "HEAD") {
+            is(v, "<res>hello pass</res>\n",
+               "wrong responseText in test for " + test.toSource());
+          }
+          else {
+            is(v, "",
+               "wrong responseText in HEAD test for " + test.toSource());
+          }
+        });
+      }, function(e) {
+          ok(!test.pass, "Expected test failure for " + test.toSource());
+          ok(e instanceof TypeError, "Exception should be TypeError for " + test.toSource());
       });
     })(test, request));
   }
@@ -836,24 +819,13 @@ function testCredentials() {
   }
 
   function testResponse(res, test) {
-    if (test.pass) {
-      is(isNetworkError(res), false,
-        "shouldn't have failed in test for " + test.toSource());
-      is(res.status, 200, "wrong status in test for " + test.toSource());
-      is(res.statusText, "OK", "wrong status text for " + test.toSource());
-      return res.text().then(function(v) {
-        is(v, "<res>hello pass</res>\n",
-         "wrong text in test for " + test.toSource());
-      });
-    }
-    else {
-      is(isNetworkError(res), true,
-        "should have failed in test for " + test.toSource());
-      return res.text().then(function(v) {
-        is(v, "",
-         "wrong text in test for " + test.toSource());
-      });
-    }
+    ok(test.pass, "Expected test to pass for " + test.toSource());
+    is(res.status, 200, "wrong status in test for " + test.toSource());
+    is(res.statusText, "OK", "wrong status text for " + test.toSource());
+    return res.text().then(function(v) {
+      is(v, "<res>hello pass</res>\n",
+       "wrong text in test for " + test.toSource());
+    });
   }
 
   function runATest(i) {
@@ -866,7 +838,15 @@ function testCredentials() {
       } else {
         finalPromiseResolve();
       }
-    }, finalPromiseReject);
+    }, function(e) {
+      ok(!test.pass, "Expected test failure for " + test.toSource());
+      ok(e instanceof TypeError, "Exception should be TypeError for " + test.toSource());
+      if (i < tests.length-1) {
+        runATest(i+1);
+      } else {
+        finalPromiseResolve();
+      }
+    });
   }
 
   runATest(0);
@@ -1153,27 +1133,19 @@ function testRedirects() {
                                          body: req.body });
     fetches.push((function(request, test) {
       return fetch(request).then(function(res) {
-        if (test.pass) {
-          is(isNetworkError(res), false,
-            "shouldn't have failed in test for " + test.toSource());
-          is(res.status, 200, "wrong status in test for " + test.toSource());
-          is(res.statusText, "OK", "wrong status text for " + test.toSource());
-          is((new URL(res.url)).host, (new URL(test.hops[test.hops.length-1].server)).host, "Response URL should be redirected URL");
-          return res.text().then(function(v) {
-            is(v, "<res>hello pass</res>\n",
-               "wrong responseText in test for " + test.toSource());
-          });
-        }
-        else {
-          is(isNetworkError(res), true,
-            "should have failed in test for " + test.toSource());
-          is(res.status, 0, "wrong status in test for " + test.toSource());
-          is(res.statusText, "", "wrong status text for " + test.toSource());
-          return res.text().then(function(v) {
-            is(v, "",
-               "wrong responseText in test for " + test.toSource());
-          });
-        }
+        ok(test.pass, "Expected test to pass for " + test.toSource());
+        is(isNetworkError(res), false,
+          "shouldn't have failed in test for " + test.toSource());
+        is(res.status, 200, "wrong status in test for " + test.toSource());
+        is(res.statusText, "OK", "wrong status text for " + test.toSource());
+        is((new URL(res.url)).host, (new URL(test.hops[test.hops.length-1].server)).host, "Response URL should be redirected URL");
+        return res.text().then(function(v) {
+          is(v, "<res>hello pass</res>\n",
+             "wrong responseText in test for " + test.toSource());
+        });
+      }, function(e) {
+        ok(!test.pass, "Expected test failure for " + test.toSource());
+        ok(e instanceof TypeError, "Exception should be TypeError for " + test.toSource());
       });
     })(request, test));
   }