Update error handling and add tests

apollographql · Mar 16, 2021 · 9d4f358 · 9d4f358
1 parent 636fc1f
commit 9d4f358
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 20 deletions.
diff --git a/gateway-js/src/__tests__/integration/networkRequests.test.ts b/gateway-js/src/__tests__/integration/networkRequests.test.ts
@@ -165,9 +165,7 @@ describe('CSDL update failures', () => {
 
     await expect(
       gateway.load(mockApolloConfig),
-    ).rejects.toThrowErrorMatchingInlineSnapshot(
-      `"401: Unexpected failure while fetching updated CSDL"`,
-    );
+    ).rejects.toThrowErrorMatchingInlineSnapshot(`"401: Unauthorized"`);
 
     await expect(gateway.stop()).rejects.toThrowErrorMatchingInlineSnapshot(
       `"ApolloGateway.stop does not need to be called before ApolloGateway.load is called successfully"`,
@@ -197,9 +195,7 @@ describe('CSDL update failures', () => {
     await gateway.load(mockApolloConfig);
     await errorLoggedPromise;
 
-    expect(logger.error).toHaveBeenCalledWith(
-      '500: Unexpected failure while fetching updated CSDL',
-    );
+    expect(logger.error).toHaveBeenCalledWith('500: Internal Server Error');
   });
 
   it('Handles GraphQL errors', async () => {

diff --git a/gateway-js/src/__tests__/loadCsdlFromStorage.test.ts b/gateway-js/src/__tests__/loadCsdlFromStorage.test.ts
@@ -6,6 +6,7 @@ import {
   graphVariant,
   apiKey,
   mockCloudConfigUrl,
+  mockCsdlRequest,
 } from './integration/nockMocks';
 
 describe('loadCsdlFromStorage', () => {
@@ -293,4 +294,61 @@ describe('loadCsdlFromStorage', () => {
       }
     `);
   });
+
+  describe('errors', () => {
+    it('throws on a malformed response', async () => {
+      mockCsdlRequest().reply(200, 'Invalid JSON');
+
+      const fetcher = getDefaultFetcher();
+      await expect(
+        loadCsdlFromStorage({
+          graphId,
+          graphVariant,
+          apiKey,
+          endpoint: mockCloudConfigUrl,
+          fetcher,
+        }),
+      ).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"200: invalid json response body at https://example.cloud-config-url.com/cloudconfig/ reason: Unexpected token I in JSON at position 0"`,
+      );
+    });
+
+    it('throws errors from JSON on 400', async () => {
+      const message = 'Query syntax error';
+      mockCsdlRequest().reply(
+        400,
+        JSON.stringify({
+          errors: [{ message }],
+        }),
+      );
+
+      const fetcher = getDefaultFetcher();
+      await expect(
+        loadCsdlFromStorage({
+          graphId,
+          graphVariant,
+          apiKey,
+          endpoint: mockCloudConfigUrl,
+          fetcher,
+        }),
+      ).rejects.toThrowError(message);
+    });
+
+    it("throws on non-OK status codes when `errors` isn't present in a JSON response", async () => {
+      mockCsdlRequest().reply(500);
+
+      const fetcher = getDefaultFetcher();
+      await expect(
+        loadCsdlFromStorage({
+          graphId,
+          graphVariant,
+          apiKey,
+          endpoint: mockCloudConfigUrl,
+          fetcher,
+        }),
+      ).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"500: Internal Server Error"`,
+      );
+    });
+  });
 });
diff --git a/gateway-js/src/loadCsdlFromStorage.ts b/gateway-js/src/loadCsdlFromStorage.ts
@@ -63,21 +63,21 @@ export async function loadCsdlFromStorage({
 
   let response: CsdlQueryResult;
 
-  try {
-    response = await result.json();
-  } catch (e) {
-    // JSON parse error, bad response
-    throw new Error(result.status + ': Unexpected failure while fetching updated CSDL');
-  }
-
-  // This happens before the 200 check below because the server returns a 400
-  // in the case of GraphQL errors (i.e. query validation)
-  if ('errors' in response) {
-    throw new Error(response.errors.map((error) => error.message).join('\n'));
-  }
+  if (result.ok || result.status === 400) {
+    try {
+      response = await result.json();
+    } catch (e) {
+      // Bad response
+      throw new Error(result.status + ': ' + e.message ?? e);
+    }
 
-  if (!result.ok) {
-    throw new Error('Unexpected failure while fetching updated CSDL');
+    // This happens before the 200 check below because the server returns a 400
+    // in the case of GraphQL errors (i.e. query validation)
+    if ('errors' in response) {
+      throw new Error(response.errors.map((error) => error.message).join('\n'));
+    }
+  } else {
+    throw new Error(result.status + ': ' + result.statusText);
   }
 
   const { routerConfig } = response.data;