fix: Broken error handling (#302)

* fix: Broken error handling * fix: Ensure expectations are checked regardless of throw
goabstract · Jul 31, 2020 · c48321c · c48321c
1 parent 5860f5f
commit c48321c
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 26 deletions.
diff --git a/src/endpoints/Endpoint.js b/src/endpoints/Endpoint.js
@@ -90,10 +90,11 @@ export default class Endpoint {
         }
       }
 
-      if (
-        transportMode.length > 1 &&
-        Object.keys(errors).length === transportMode.length
-      ) {
+      const totalErrors = Object.keys(errors).length;
+      if (transportMode.length === totalErrors) {
+        if (totalErrors === 1) {
+          throw Object.values(errors)[0];
+        }
         throw new MultiError(errors);
       }
 

diff --git a/tests/Client.test.js b/tests/Client.test.js
@@ -19,26 +19,50 @@ describe("Client", () => {
   test("no transports specified", async () => {
     expect.assertions(1);
 
+    let error;
     try {
       await API_CLIENT.organizations.list({
         transportMode: []
       });
-    } catch (error) {
-      expect(error).toBeInstanceOf(EndpointUndefinedError);
+    } catch (e) {
+      error = e;
     }
+    expect(error).toBeInstanceOf(EndpointUndefinedError);
   });
 
   test("endpoint undefined in all transports", async () => {
     expect.assertions(2);
 
+    let error;
     try {
       await API_CLIENT.organizations.list({
         transportMode: ["api", "cli"]
       });
-    } catch (error) {
-      expect(error).toBeInstanceOf(MultiError);
-      expect(error.errors.cli).toBeInstanceOf(EndpointUndefinedError);
+    } catch (e) {
+      error = e;
     }
+    expect(error).toBeInstanceOf(MultiError);
+    expect(error && error.errors.cli).toBeInstanceOf(EndpointUndefinedError);
+  });
+
+  test("individual transport throws", async () => {
+    mockAPI("/projects/project-id/branches/?", { ok: false }, 404);
+
+    let error;
+    try {
+      await API_CLIENT.branches.list(
+        {
+          projectId: "project-id"
+        },
+        {
+          transportMode: ["api"]
+        }
+      );
+    } catch (e) {
+      error = e;
+    }
+
+    expect(error).toBeInstanceOf(NotFoundError);
   });
 
   test("all transports throw", async () => {
@@ -48,6 +72,7 @@ describe("Client", () => {
       code: "not_found"
     });
 
+    let error;
     try {
       await API_CLIENT.branches.list(
         {
@@ -57,11 +82,13 @@ describe("Client", () => {
           transportMode: ["api", "cli"]
         }
       );
-    } catch (error) {
-      expect(error).toBeInstanceOf(MultiError);
-      expect(error.errors.api).toBeInstanceOf(NotFoundError);
-      expect(error.errors.cli).toBeInstanceOf(NotFoundError);
+    } catch (e) {
+      error = e;
     }
+
+    expect(error).toBeInstanceOf(MultiError);
+    expect(error && error.errors.api).toBeInstanceOf(NotFoundError);
+    expect(error && error.errors.cli).toBeInstanceOf(NotFoundError);
   });
 
   test("attempts all transports in order", async () => {

diff --git a/tests/endpoints/Branches.test.js b/tests/endpoints/Branches.test.js
@@ -206,11 +206,13 @@ describe("branches", () => {
     });
 
     test("cli - no descriptor", async () => {
+      let error;
       try {
         await CLI_CLIENT.branches.list(undefined);
-      } catch (error) {
-        expect(error.errors.cli).toBeInstanceOf(BranchSearchCLIError);
+      } catch (e) {
+        error = e;
       }
+      expect(error).toBeInstanceOf(BranchSearchCLIError);
     });
   });
 

diff --git a/tests/endpoints/Files.test.js b/tests/endpoints/Files.test.js
@@ -40,16 +40,18 @@ describe("files", () => {
         ]
       });
 
+      let error;
       try {
         await API_CLIENT.files.info({
           branchId: "branch-id",
           fileId: "file-id",
           projectId: "project-id",
           sha: "sha"
         });
-      } catch (error) {
-        expect(error.errors.api).toBeInstanceOf(NotFoundError);
+      } catch (e) {
+        error = e;
       }
+      expect(error).toBeInstanceOf(NotFoundError);
     });
 
     test("cli", async () => {
@@ -213,16 +215,18 @@ describe("files", () => {
         );
       });
 
+      let error;
       try {
         await API_CLIENT.files.raw({
           branchId: "branch-id",
           fileId: "file-id",
           projectId: "project-id",
           sha: "sha"
         });
-      } catch (error) {
-        expect(error.errors.api).toBeInstanceOf(FileExportError);
+      } catch (e) {
+        error = e;
       }
+      expect(error).toBeInstanceOf(FileExportError);
     });
 
     test("cli - exports file", async () => {

diff --git a/tests/endpoints/Pages.test.js b/tests/endpoints/Pages.test.js
@@ -49,7 +49,7 @@ describe("pages", () => {
           sha: "sha"
         });
       } catch (error) {
-        expect(error.errors.api).toBeInstanceOf(NotFoundError);
+        expect(error).toBeInstanceOf(NotFoundError);
       }
     });
 
@@ -84,6 +84,7 @@ describe("pages", () => {
         ]
       });
 
+      let error;
       try {
         await CLI_CLIENT.pages.info({
           branchId: "branch-id",
@@ -92,9 +93,10 @@ describe("pages", () => {
           projectId: "project-id",
           sha: "sha"
         });
-      } catch (error) {
-        expect(error.errors.cli).toBeInstanceOf(NotFoundError);
+      } catch (e) {
+        error = e;
       }
+      expect(error).toBeInstanceOf(NotFoundError);
     });
   });
 

diff --git a/tests/errors.test.js b/tests/errors.test.js
@@ -122,7 +122,7 @@ describe("errors", () => {
       try {
         await CLI_CLIENT.projects.list({ organizationId: "org-id" });
       } catch (error) {
-        expect(error.errors.cli).toBeInstanceOf(EndpointUndefinedError);
+        expect(error).toBeInstanceOf(EndpointUndefinedError);
       }
     });
 

diff --git a/tests/util/helpers.test.js b/tests/util/helpers.test.js
@@ -17,12 +17,14 @@ describe("helpers", () => {
   });
 
   test("inferShareId throws", () => {
+    let error;
     try {
       utils.inferShareId(({}: any));
-    } catch (error) {
-      expect(error).toBeInstanceOf(Error);
-      expect(error.message).toContain("Could not infer share id");
+    } catch (e) {
+      error = e;
     }
+    expect(error).toBeInstanceOf(Error);
+    expect(error && error.message).toContain("Could not infer share id");
   });
 
   test("isNodeEnvironment", () => {