From 991f78b4aa86a9f1a78eeeed53e7602fad5d6d6e Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Thu, 15 Nov 2018 23:55:29 -0500 Subject: [PATCH 1/2] Swallows errors on ml lookups that we know can fail --- .../__test__/get_anomaly_aggs.test.ts | 16 ++++++++++++++++ .../get_anomaly_aggs.js | 7 +++---- .../get_avg_response_time_anomalies.js | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/__test__/get_anomaly_aggs.test.ts diff --git a/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/__test__/get_anomaly_aggs.test.ts b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/__test__/get_anomaly_aggs.test.ts new file mode 100644 index 0000000000000..de1519f8f3cb3 --- /dev/null +++ b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/__test__/get_anomaly_aggs.test.ts @@ -0,0 +1,16 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +// @ts-ignore +import { getAnomalyAggs } from '../get_anomaly_aggs'; + +test('getAnomalyAggs should swallow errors', () => { + const failClient = jest.fn(() => + Promise.reject(new Error('anomaly lookup failed')) + ); + + expect(getAnomalyAggs({ client: failClient })).resolves.toEqual(null); +}); diff --git a/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_anomaly_aggs.js b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_anomaly_aggs.js index 86fb84fd88871..589780e5e59c0 100644 --- a/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_anomaly_aggs.js +++ b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_anomaly_aggs.js @@ -63,9 +63,8 @@ export async function getAnomalyAggs({ const resp = await client('search', params); return resp.aggregations; } catch (e) { - if (e.statusCode === 404) { - return null; - } - throw e; + // swallow error because there are lots of reasons + // the ml index lookup may fail + return null; } } diff --git a/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_avg_response_time_anomalies.js b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_avg_response_time_anomalies.js index c6d411eb91884..3e950d919b6cc 100644 --- a/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_avg_response_time_anomalies.js +++ b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_avg_response_time_anomalies.js @@ -34,7 +34,7 @@ export async function getAvgResponseTimeAnomalies({ if (!aggs) { return { - message: 'ml index does not exist' + message: 'Error reading machine learning index' }; } From 760d566e43ed7050272b71faacff6870ed4f57f6 Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Fri, 16 Nov 2018 12:51:06 -0500 Subject: [PATCH 2/2] Adjusts when we swallow ml lookup errors, fixes async test --- .../__test__/get_anomaly_aggs.test.ts | 21 ++++++++++++++----- .../get_anomaly_aggs.js | 12 +++++++---- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/__test__/get_anomaly_aggs.test.ts b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/__test__/get_anomaly_aggs.test.ts index de1519f8f3cb3..7ded075a7ae65 100644 --- a/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/__test__/get_anomaly_aggs.test.ts +++ b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/__test__/get_anomaly_aggs.test.ts @@ -7,10 +7,21 @@ // @ts-ignore import { getAnomalyAggs } from '../get_anomaly_aggs'; -test('getAnomalyAggs should swallow errors', () => { - const failClient = jest.fn(() => - Promise.reject(new Error('anomaly lookup failed')) - ); +test('getAnomalyAggs should swallow HTTP errors', () => { + const httpError = new Error('anomaly lookup failed') as any; + httpError.statusCode = 418; + const failClient = jest.fn(() => Promise.reject(httpError)); - expect(getAnomalyAggs({ client: failClient })).resolves.toEqual(null); + return expect(getAnomalyAggs({ client: failClient })).resolves.toEqual(null); +}); + +test('getAnomalyAggs should throw other errors', () => { + const otherError = new Error('anomaly lookup ASPLODED') as any; + const failClient = jest.fn(() => Promise.reject(otherError)); + + return expect( + getAnomalyAggs({ + client: failClient + }) + ).rejects.toThrow(otherError); }); diff --git a/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_anomaly_aggs.js b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_anomaly_aggs.js index 589780e5e59c0..cae78df06f725 100644 --- a/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_anomaly_aggs.js +++ b/x-pack/plugins/apm/server/lib/transactions/charts/get_avg_response_time_anomalies/get_anomaly_aggs.js @@ -62,9 +62,13 @@ export async function getAnomalyAggs({ try { const resp = await client('search', params); return resp.aggregations; - } catch (e) { - // swallow error because there are lots of reasons - // the ml index lookup may fail - return null; + } catch (err) { + if ('statusCode' in err) { + // swallow HTTP errors because there are lots of reasons + // the ml index lookup may fail, and we're ok with that + return null; + } + + throw err; } }