Skip to content

Commit

Permalink
Small error improvement for health checks
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
trevor-scheer committed Mar 3, 2021
1 parent 544ddaf commit 692e046
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 20 deletions.
29 changes: 15 additions & 14 deletions gateway-js/src/__tests__/integration/networkRequests.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
`);
});
});

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
15 changes: 9 additions & 6 deletions gateway-js/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Expand All @@ -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}`);
}),
),
);
}
Expand Down

0 comments on commit 692e046

Please sign in to comment.