From 990826024072c42e658a0351a96d6faab064b5b7 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 20 Jul 2018 15:46:40 -0700 Subject: [PATCH 1/2] Clarify fatal result of Courier request errors. --- .../courier/fetch/__tests__/call_client.js | 32 ++++++++++--------- src/ui/public/courier/fetch/call_client.js | 7 ++-- src/ui/public/courier/fetch/fetch_now.js | 5 ++- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/ui/public/courier/fetch/__tests__/call_client.js b/src/ui/public/courier/fetch/__tests__/call_client.js index 565af41f23ebf..e0acdf8432077 100644 --- a/src/ui/public/courier/fetch/__tests__/call_client.js +++ b/src/ui/public/courier/fetch/__tests__/call_client.js @@ -121,6 +121,23 @@ describe('callClient', () => { }); }); + describe('errors', () => { + it(`cause searchRequest.handleFailure() to be called with the ES error that's thrown`, async () => { + esShouldError = true; + const searchRequest = createSearchRequest(1); + + const handleFailureSpy = sinon.spy(); + searchRequest.handleFailure = handleFailureSpy; + + searchRequests = [ searchRequest ]; + try { + await callClient(searchRequests); + } catch(e) { + sinon.assert.calledWith(handleFailureSpy, 'fake es error'); + } + }); + }); + describe('implementation', () => { it('calls es.msearch() once, regardless of number of searchRequests', () => { expect(fakeSearch.callCount).to.be(0); @@ -141,21 +158,6 @@ describe('callClient', () => { await callClient(searchRequests); expect(whenAbortedSpy.callCount).to.be(1); }); - - it(`calls searchRequest.handleFailure() with the ES error that's thrown`, async () => { - esShouldError = true; - const searchRequest = createSearchRequest(1); - - const handleFailureSpy = sinon.spy(); - searchRequest.handleFailure = handleFailureSpy; - - searchRequests = [ searchRequest ]; - try { - await callClient(searchRequests); - } catch(e) { - sinon.assert.calledWith(handleFailureSpy, 'fake es error'); - } - }); }); describe('aborting at different points in the request lifecycle:', () => { diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index fd8266eb6e122..a5887e227493e 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -165,6 +165,7 @@ export function CallClientProvider(Private, Promise, es) { // order than the original searchRequests. So we'll put them back in order so that we can // use the order to associate each response with the original request. const responsesInOriginalRequestOrder = new Array(searchRequestsAndStatuses.length); + segregatedResponses.forEach((responses, strategyIndex) => { responses.forEach((response, responseIndex) => { const searchRequest = searchStrategiesWithRequests[strategyIndex].searchRequests[responseIndex]; @@ -185,9 +186,9 @@ export function CallClientProvider(Private, Promise, es) { // If there are any errors, notify the searchRequests of them. defer.promise.catch((err) => { - searchRequests.forEach((searchRequest, index) => { - if (searchRequestsAndStatuses[index] !== ABORTED) { - searchRequest.handleFailure(err); + searchRequestsAndStatuses.forEach((searchRequestOrStatus) => { + if (searchRequestOrStatus !== ABORTED) { + searchRequestOrStatus.handleFailure(err); } }); }); diff --git a/src/ui/public/courier/fetch/fetch_now.js b/src/ui/public/courier/fetch/fetch_now.js index 70aa20a26619c..74a9ed94d0753 100644 --- a/src/ui/public/courier/fetch/fetch_now.js +++ b/src/ui/public/courier/fetch/fetch_now.js @@ -52,7 +52,10 @@ export function FetchNowProvider(Private, Promise) { return searchRequest.retry(); })) - .catch(error => fatalError(error, 'Courier fetch')); + .catch(error => { + // If any of the searchRequests resolve with an error, kill Kibana. + fatalError(error, 'Courier fetch'); + }); } function fetchSearchResults(searchRequests) { From 69b03c4bf1afcbeacaf30fdbfa08add70e5c81e1 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 20 Jul 2018 16:16:12 -0700 Subject: [PATCH 2/2] Prevent search request errors from resulting in a fatal error. --- src/ui/public/courier/fetch/fetch_now.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ui/public/courier/fetch/fetch_now.js b/src/ui/public/courier/fetch/fetch_now.js index 74a9ed94d0753..7bb3890fbf95e 100644 --- a/src/ui/public/courier/fetch/fetch_now.js +++ b/src/ui/public/courier/fetch/fetch_now.js @@ -53,7 +53,7 @@ export function FetchNowProvider(Private, Promise) { return searchRequest.retry(); })) .catch(error => { - // If any of the searchRequests resolve with an error, kill Kibana. + // If any errors occur after the search requests have resolved, then we kill Kibana. fatalError(error, 'Courier fetch'); }); } @@ -73,7 +73,11 @@ export function FetchNowProvider(Private, Promise) { return startRequests(searchRequests) .then(function () { replaceAbortedRequests(); - return callClient(searchRequests); + return callClient(searchRequests) + .catch(() => { + // Silently swallow errors that result from search requests so the consumer can surface + // them as notifications instead of courier forcing fatal errors. + }); }) .then(function (responses) { replaceAbortedRequests();