Http class now rejects 4xx and 5xx responses

This patch makes sure that successful HTTP requests, which result
in an error code, are passed to the catch() or reject handler, rather
than the success handler.

This results in two different cases in the claim() handler, that of
an internally thrown exception (like a TypeError), and that of a
Response() instance. In most cases, developers should not write
code that throws exceptions - and even in those, the ES6 Promise
API silently swallows most uncaught throws.

Change-Id: I736bcaccf6324a3d66191692e5f2ce922a727dea

src/util/http.js

@@ -88,6 +88,7 @@ export default class Http {
    * @returns {Promise} A promise which will resolve with the processed request response.
    */
   httpRequest (method, url, headers = {}, body) {
+
     // Sanitize the headers...
     headers = Object.assign({}, headers, this.defaultHeaders);
 
@@ -100,34 +101,50 @@ export default class Http {
     }
     const request = new Request(url, init);
 
-    let promise = Promise.resolve(request);
+    // Build the wrapper promise.
+    return new Promise((resolve, reject) => {
 
-    // Loop through the request interceptors, constructing a promise chain.
+      let promise = Promise.resolve(request);
-    for (let interceptor of this.requestInterceptors) {
-      promise = promise.then(interceptor);
-    }
 
-    // Make the actual request...
+      // Loop through the request interceptors, constructing a promise chain.
-    promise = promise
+      for (let interceptor of this.requestInterceptors) {
-      .then((request) => {
+        promise = promise.then(interceptor);
-        // Deconstruct the request, since fetch-mock doesn't actually support fetch(Request);
+      }
-        const init = {
+
-          method: request.method,
+      // Make the actual request...
-          headers: request.headers
+      promise = promise
-        };
+        .then((request) => {
-        if (['GET', 'HEAD'].indexOf(request.method) === -1 && request.body) {
+          // Deconstruct the request, since fetch-mock doesn't actually support fetch(Request);
-          init.body = request.body;
+          const init = {
+            method: request.method,
+            headers: request.headers
+          };
+          if (['GET', 'HEAD'].indexOf(request.method) === -1 && request.body) {
+            init.body = request.body;
+          }
+
+          return fetch(request.url, init);
+        });
+
+      // Fetch will treat all http responses (2xx, 3xx, 4xx, 5xx, etc) as successful responses.
+      // This will catch all 4xx and 5xx responses and return them to the catch() handler. Note
+      // that it's up to the downstream developer to determine whether what they received is an
+      // error or a failed response.
+      promise.then((response) => {
+        if (response.status >= 400) {
+          return reject(response);
+        } else {
+          return response;
         }
-
-        return fetch(request.url, init);
       });
 
-    // Pass the response content through the response interceptors...
+      // Pass the response content through the response interceptors...
-    for (let interceptor of this.responseInterceptors) {
+      for (let interceptor of this.responseInterceptors) {
-      promise = promise.then(interceptor);
+        promise = promise.then(interceptor);
-    }
+      }
 
-    return promise;
+      promise.then((response) => resolve(response), (error) => reject(error));
+    });
   }
 
   /**

test/unit/util/httpTest.js

@@ -189,21 +189,17 @@ describe('Http', () => {
       });
   });
 
-  it("should pass failed requests back to the invoker", (done) => {
+  it("should pass failed requests to the catch block.", (done) => {
     fetchMock.get(testUrl, {status: 500, body: testResponse});
 
     http.httpGet(testUrl)
       .then((response) => {
         // The HTTP request 'succeeded' with a failing state.
-        expect(response.status).toEqual(500);
+        expect(response).toBeNull();
-        return response.json();
-      })
-      .then((body) => {
-        expect(body).toEqual(testResponse);
         done();
       })
-      .catch((error) => {
+      .catch((response) => {
-        expect(error).toBeNull();
+        expect(response.status).toBe(500);
         done();
       });
   });