Bug 1121682 - fetch() should reject with TypeError
authorNikhil Marathe <nsm.nikhil@gmail.com>
Wed, 14 Jan 2015 13:43:27 -0800
changeset 252608 5656dc5587ade16b5346bd39ddc66c921b58c1ba
parent 252607 fc16e4cd8fbb6efd597c30c4b759d13b44d17116
child 252609 a1791566f721f1a14f515e7aec5f45890fe22a8f
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1121682
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1121682 - fetch() should reject with TypeError
dom/bindings/Errors.msg
dom/bindings/Exceptions.cpp
dom/bindings/Exceptions.h
dom/bindings/ToJSValue.cpp
dom/fetch/Fetch.cpp
dom/tests/mochitest/fetch/worker_test_fetch_basic.js
dom/tests/mochitest/fetch/worker_test_fetch_basic_http.js
dom/tests/mochitest/fetch/worker_test_fetch_cors.js
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@@ -58,9 +58,10 @@ MSG_DEF(MSG_HEADERS_IMMUTABLE, 0, "Heade
 MSG_DEF(MSG_INVALID_HEADER_NAME, 1, "{0} is an invalid header name.")
 MSG_DEF(MSG_INVALID_HEADER_VALUE, 1, "{0} is an invalid header value.")
 MSG_DEF(MSG_INVALID_HEADER_SEQUENCE, 0, "Headers require name/value tuples when being initialized by a sequence.")
 MSG_DEF(MSG_PERMISSION_DENIED_TO_PASS_ARG, 1, "Permission denied to pass cross-origin object as {0}.")
 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")
--- a/dom/bindings/Exceptions.cpp
+++ b/dom/bindings/Exceptions.cpp
@@ -181,16 +181,28 @@ GetCurrentJSStack()
   if (!stack) {
     return nullptr;
   }
 
   // Note that CreateStack only returns JS frames, so we're done here.
   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
 {
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(StackFrame)
   NS_DECL_NSISTACKFRAME
--- a/dom/bindings/Exceptions.h
+++ b/dom/bindings/Exceptions.h
@@ -39,16 +39,29 @@ ThrowExceptionObject(JSContext* aCx, nsI
 // Create an exception object for the given nsresult and message but
 // don't set it pending on aCx.  This never returns null.
 already_AddRefed<Exception>
 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 {
 
 // aMaxDepth can be used to define a maximal depth for the stack trace. If the
 // value is -1, a default maximal depth will be selected.
 already_AddRefed<nsIStackFrame>
 CreateStack(JSContext* aCx, int32_t aMaxDepth = -1);
 
--- a/dom/bindings/ToJSValue.cpp
+++ b/dom/bindings/ToJSValue.cpp
@@ -61,16 +61,17 @@ ToJSValue(JSContext* aCx,
 }
 
 bool
 ToJSValue(JSContext* aCx,
           ErrorResult& aArgument,
           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);
   MOZ_ASSERT(getPendingResult);
   JS_ClearPendingException(aCx);
   return true;
 }
 
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -261,19 +261,25 @@ MainThreadFetchResolver::MainThreadFetch
 }
 
 void
 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()
 {
   NS_ASSERT_OWNINGTHREAD(MainThreadFetchResolver);
 }
 
 class WorkerFetchResponseRunnable MOZ_FINAL : public WorkerRunnable
@@ -291,21 +297,28 @@ public:
 
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) MOZ_OVERRIDE
   {
     MOZ_ASSERT(aWorkerPrivate);
     aWorkerPrivate->AssertIsOnWorkerThread();
     MOZ_ASSERT(aWorkerPrivate == mResolver->GetWorkerPrivate());
 
-    nsRefPtr<nsIGlobalObject> global = aWorkerPrivate->GlobalScope();
-    mResolver->mResponse = new Response(global, mInternalResponse);
+    nsRefPtr<Promise> promise = mResolver->mFetchPromise.forget();
+
+    if (mInternalResponse->Type() != ResponseType::Error) {
+      nsRefPtr<nsIGlobalObject> global = aWorkerPrivate->GlobalScope();
+      mResolver->mResponse = new Response(global, mInternalResponse);
 
-    nsRefPtr<Promise> promise = mResolver->mFetchPromise.forget();
-    promise->MaybeResolve(mResolver->mResponse);
+      promise->MaybeResolve(mResolver->mResponse);
+    } else {
+      ErrorResult result;
+      result.ThrowTypeError(MSG_FETCH_FAILED);
+      promise->MaybeReject(result);
+    }
 
     return true;
   }
 };
 
 class WorkerFetchResponseEndRunnable MOZ_FINAL : public WorkerRunnable
 {
   nsRefPtr<WorkerFetchResolver> mResolver;
@@ -317,17 +330,16 @@ public:
   }
 
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) MOZ_OVERRIDE
   {
     MOZ_ASSERT(aWorkerPrivate);
     aWorkerPrivate->AssertIsOnWorkerThread();
     MOZ_ASSERT(aWorkerPrivate == mResolver->GetWorkerPrivate());
-    MOZ_ASSERT(mResolver->mResponse);
 
     mResolver->CleanUp(aCx);
     return true;
   }
 };
 
 void
 WorkerFetchResolver::OnResponseAvailable(InternalResponse* aResponse)
--- a/dom/tests/mochitest/fetch/worker_test_fetch_basic.js
+++ b/dom/tests/mochitest/fetch/worker_test_fetch_basic.js
@@ -18,17 +18,19 @@ function testAboutURL() {
     is(res.headers.get('content-type'), 'text/html;charset=utf-8',
        "about:blank content-type should be text/html;charset=utf-8");
     return res.text().then(function(v) {
       is(v, "", "about:blank body should be empty");
     });
   });
 
   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]);
 }
 
 function testDataURL() {
   return fetch("data:text/plain;charset=UTF-8,Hello").then(function(res) {
     ok(true, "Data URL fetch should resolve");
--- a/dom/tests/mochitest/fetch/worker_test_fetch_basic_http.js
+++ b/dom/tests/mochitest/fetch/worker_test_fetch_basic_http.js
@@ -38,18 +38,19 @@ function testURL() {
 }
 
 var failFiles = [['ftp://localhost' + path + 'file_XHR_pass1.xml', 'GET']];
 
 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);
   });
 
   return Promise.all(promises);
 }
 
 function testRequestGET() {
--- a/dom/tests/mochitest/fetch/worker_test_fetch_cors.js
+++ b/dom/tests/mochitest/fetch/worker_test_fetch_cors.js
@@ -19,17 +19,19 @@ function isNetworkError(response) {
 function isOpaqueResponse(response) {
   return response.type == "opaque" && response.status === 0 && response.statusText === "";
 }
 
 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.");
   });
 }
 
 function testNoCorsCtor() {
   // Request constructor Step 19.1
   var simpleMethods = ["GET", "HEAD", "POST"];
   for (var i = 0; i < simpleMethods.length; ++i) {
     var r = new Request("http://example.com", { method: simpleMethods[i], mode: "no-cors" });
@@ -655,72 +657,53 @@ function testModeCors() {
       req.url += "&exposeHeaders=" + escape(test.exposeHeaders);
     if (test.preflightBody)
       req.url += "&preflightBody=" + escape(test.preflightBody);
 
     var request = new Request(req.url, { method: req.method, mode: "cors",
                                          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());
+        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 {
+          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(v, "",
-                 "wrong responseText in HEAD test 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.headers.get(header), test.responseHeaders[header],
+                 "|Headers.get()|wrong response header (" + header + ") in test for " +
                  test.toSource());
             }
           }
+        }
 
-          return res.text().then(function(v) {
+        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 test for " + test.toSource());
-          });
-        }
+               "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));
   }
 
   return Promise.all(fetches);
 }
 
 function testCredentials() {
@@ -831,47 +814,44 @@ function testCredentials() {
       req.url += "&allowMethods=" + escape(test.allowMethods);
 
     return new Request(req.url, { method: req.method,
                                   headers: req.headers,
                                   credentials: req.withCred ? "include" : "omit" });
   }
 
   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) {
     var test = tests[i];
     var request = makeRequest(test);
     fetch(request).then(function(res) {
       testResponse(res, test);
       if (i < tests.length-1) {
         runATest(i+1);
       } 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);
   return finalPromise;
 }
 
 function testRedirects() {
   var origin = "http://mochi.test:8888";
@@ -1148,37 +1128,29 @@ function testRedirects() {
         req.url += "&body=" + escape(test.body);
     }
 
     var request = new Request(req.url, { method: req.method,
                                          headers: req.headers,
                                          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));
   }
 
   return Promise.all(fetches);
 }
 
 function runTest() {