From 692e0461dde2b70c9930d9a083ab4c6dbc78d63e Mon Sep 17 00:00:00 2001
From: Trevor Scheer <trevor.scheer@gmail.com>
Date: Wed, 3 Mar 2021 15:08:56 -0700
Subject: [PATCH] Small error improvement for health checks

* Don't log and rethrow an error which is already handled further up
* Include original error message and originating service in health check error message
---
 .../integration/networkRequests.test.ts       | 29 ++++++++++---------
 gateway-js/src/index.ts                       | 15 ++++++----
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/gateway-js/src/__tests__/integration/networkRequests.test.ts b/gateway-js/src/__tests__/integration/networkRequests.test.ts
index 71e4dac8c..a0f85605b 100644
--- a/gateway-js/src/__tests__/integration/networkRequests.test.ts
+++ b/gateway-js/src/__tests__/integration/networkRequests.test.ts
@@ -259,9 +259,10 @@ describe('Downstream service health checks', () => {
       });
 
       // TODO: smell that we should be awaiting something else
-      await expect(gateway.load()).rejects.toThrowErrorMatchingInlineSnapshot(
-        `"500: Internal Server Error"`,
-      );
+      await expect(gateway.load()).rejects.toThrowErrorMatchingInlineSnapshot(`
+              "The gateway did not update its schema due to failed service health checks. The gateway will continue to operate with the previous schema and reattempt updates. The following error occurred during the health check:
+              [accounts]: 500: Internal Server Error"
+            `);
     });
   });
 
@@ -291,11 +292,11 @@ describe('Downstream service health checks', () => {
 
       gateway = new ApolloGateway({ serviceHealthCheck: true, logger });
 
-      await expect(
-        gateway.load(mockApolloConfig),
-      ).rejects.toThrowErrorMatchingInlineSnapshot(
-        `"500: Internal Server Error"`,
-      );
+      await expect(gateway.load(mockApolloConfig)).rejects
+        .toThrowErrorMatchingInlineSnapshot(`
+              "The gateway did not update its schema due to failed service health checks. The gateway will continue to operate with the previous schema and reattempt updates. The following error occurred during the health check:
+              [accounts]: 500: Internal Server Error"
+            `);
     });
 
     // This test has been flaky for a long time, and fails consistently after changes
@@ -382,18 +383,18 @@ describe('Downstream service health checks', () => {
         .mockImplementationOnce(async () => {
           // mock the first poll and handle the error which would otherwise be caught
           // and logged from within the `pollServices` class method
-          await expect(
-            original.apply(gateway),
-          ).rejects.toThrowErrorMatchingInlineSnapshot(
-            `"500: Internal Server Error"`,
-          );
+          await expect(original.apply(gateway)).rejects
+            .toThrowErrorMatchingInlineSnapshot(`
+                  "The gateway did not update its schema due to failed service health checks. The gateway will continue to operate with the previous schema and reattempt updates. The following error occurred during the health check:
+                  [accounts]: 500: Internal Server Error"
+                `);
           // finally resolve the promise which drives this test
           resolve();
         });
 
       // @ts-ignore for testing purposes, replace the `updateSchema`
       // function on the gateway with our mock
-      gateway.updateSchema= mockUpdateSchema;
+      gateway.updateSchema = mockUpdateSchema;
 
       // load the gateway as usual
       await gateway.load(mockApolloConfig);
diff --git a/gateway-js/src/index.ts b/gateway-js/src/index.ts
index 81c3711b6..f0cd934cc 100644
--- a/gateway-js/src/index.ts
+++ b/gateway-js/src/index.ts
@@ -527,12 +527,12 @@ export class ApolloGateway implements GraphQLService {
       try {
         await this.serviceHealthCheck(serviceMap);
       } catch (e) {
-        this.logger.error(
-          'The gateway did not update its schema due to failed service health checks.  ' +
-            'The gateway will continue to operate with the previous schema and reattempt updates.' +
-            e,
+        throw new Error(
+          'The gateway did not update its schema due to failed service health checks. ' +
+            'The gateway will continue to operate with the previous schema and reattempt updates. ' +
+            'The following error occurred during the health check:\n' +
+            e.message,
         );
-        throw e;
       }
     }
   }
@@ -557,7 +557,10 @@ export class ApolloGateway implements GraphQLService {
       Object.entries(serviceMap).map(([name, { dataSource }]) =>
         dataSource
           .process({ request: { query: HEALTH_CHECK_QUERY }, context: {} })
-          .then((response) => ({ name, response })),
+          .then((response) => ({ name, response }))
+          .catch((e) => {
+            throw new Error(`[${name}]: ${e.message}`);
+          }),
       ),
     );
   }