Merge pull request #131 from Esri/fix/response-ok

check for errors before trying to parse response
Esri · Mar 1, 2018 · e2b05b2 · e2b05b2
2 parents 232b863 + c86b07d
commit e2b05b2
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 0 deletions.
diff --git a/packages/arcgis-rest-request/src/request.ts b/packages/arcgis-rest-request/src/request.ts
@@ -5,6 +5,7 @@ import { checkForErrors } from "./utils/check-for-errors";
 import { encodeFormData } from "./utils/encode-form-data";
 import { encodeQueryString } from "./utils/encode-query-string";
 import { requiresFormData } from "./utils/process-params";
+import { ArcGISRequestError } from "./utils/ArcGISRequestError";
 
 export interface IAuthenticationManager {
   portal: string;
@@ -193,6 +194,17 @@ export function request(
       return options.fetch(url, fetchOptions);
     })
     .then(response => {
+      if (!response.ok) {
+        // server responded w/ an actual error (404, 500, etc)
+        const { status, statusText } = response;
+        throw new ArcGISRequestError(
+          statusText,
+          `HTTP ${status}`,
+          response,
+          url,
+          options
+        );
+      }
       switch (params.f) {
         case "json":
           return response.json();

diff --git a/packages/arcgis-rest-request/test/request.test.ts b/packages/arcgis-rest-request/test/request.test.ts
@@ -167,6 +167,26 @@ describe("request()", () => {
       });
   });
 
+  it("should re-throw HTTP errors (404, 500, etc)", done => {
+    fetchMock.once("*", 404);
+
+    request(
+      "https://www.arcgis.com/sharing/rest/content/items/43a8e51789044d9480a20089a84129ad/data"
+    ).catch(error => {
+      expect(error.name).toBe(ErrorTypes.ArcGISRequestError);
+      expect(error.message).toBe("HTTP 404: Not Found");
+      expect(error instanceof Error).toBeTruthy();
+      expect(error.url).toBe(
+        "https://www.arcgis.com/sharing/rest/content/items/43a8e51789044d9480a20089a84129ad/data"
+      );
+      expect(error.options.params).toEqual({ f: "json" });
+      expect(error.options.httpMethod).toEqual("POST");
+      expect(typeof error.options.fetch).toEqual("function");
+      expect(error.options.fetch.length).toEqual(2);
+      done();
+    });
+  });
+
   it("should throw errors with information about the request", done => {
     fetchMock.once("*", ArcGISOnlineError);
 
@@ -189,6 +209,7 @@ describe("request()", () => {
 
   it("should allow you to use custom implementations of `fetch`", done => {
     const MockFetchResponse = {
+      ok: true,
       json() {
         return Promise.resolve(SharingRestInfo);
       },
@@ -244,6 +265,7 @@ describe("request()", () => {
       FormData = oldFormData;
 
       const MockFetchResponse = {
+        ok: true,
         json() {
           return Promise.resolve(SharingRestInfo);
         },