From 9ac2d2f6620a84cb991c355a42abfdd5ef415075 Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Fri, 3 Sep 2021 16:51:21 +0200 Subject: [PATCH] catch errors from providers (#111093) (#111121) --- .../public/services/search_service.test.ts | 64 +++++++++++++++++++ .../public/services/search_service.ts | 8 +-- .../server/services/search_service.test.ts | 38 +++++++++++ .../server/services/search_service.ts | 5 +- 4 files changed, 109 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/global_search/public/services/search_service.test.ts b/x-pack/plugins/global_search/public/services/search_service.test.ts index 4b3c06f03dcc8..b0e6a72290438 100644 --- a/x-pack/plugins/global_search/public/services/search_service.test.ts +++ b/x-pack/plugins/global_search/public/services/search_service.test.ts @@ -272,6 +272,43 @@ describe('SearchService', () => { }); }); + it('catches errors from providers', async () => { + const { registerResultProvider } = service.setup({ + config: createConfig(), + }); + + getTestScheduler().run(({ expectObservable, hot }) => { + registerResultProvider( + createProvider('A', { + source: hot('a---c-|', { + a: [providerResult('A1'), providerResult('A2')], + c: [providerResult('A3')], + }), + }) + ); + registerResultProvider( + createProvider('B', { + source: hot( + '-b-# ', + { + b: [providerResult('B1')], + }, + new Error('something went bad') + ), + }) + ); + + const { find } = service.start(startDeps()); + const results = find({ term: 'foobar' }, {}); + + expectObservable(results).toBe('ab--c-|', { + a: expectedBatch('A1', 'A2'), + b: expectedBatch('B1'), + c: expectedBatch('A3'), + }); + }); + }); + it('return mixed server/client providers results', async () => { const { registerResultProvider } = service.setup({ config: createConfig(), @@ -304,6 +341,33 @@ describe('SearchService', () => { }); }); + it('catches errors from the server', async () => { + const { registerResultProvider } = service.setup({ + config: createConfig(), + }); + + getTestScheduler().run(({ expectObservable, hot }) => { + fetchServerResultsMock.mockReturnValue(hot('#', {}, new Error('fetch error'))); + + registerResultProvider( + createProvider('A', { + source: hot('a-b-|', { + a: [providerResult('P1')], + b: [providerResult('P2')], + }), + }) + ); + + const { find } = service.start(startDeps()); + const results = find({ term: 'foobar' }, {}); + + expectObservable(results).toBe('a-b-|', { + a: expectedBatch('P1'), + b: expectedBatch('P2'), + }); + }); + }); + it('handles the `aborted$` option', async () => { const { registerResultProvider } = service.setup({ config: createConfig(), diff --git a/x-pack/plugins/global_search/public/services/search_service.ts b/x-pack/plugins/global_search/public/services/search_service.ts index bf06aa04061ed..85f4d4143a609 100644 --- a/x-pack/plugins/global_search/public/services/search_service.ts +++ b/x-pack/plugins/global_search/public/services/search_service.ts @@ -5,8 +5,8 @@ * 2.0. */ -import { merge, Observable, timer, throwError } from 'rxjs'; -import { map, takeUntil } from 'rxjs/operators'; +import { merge, Observable, timer, throwError, EMPTY } from 'rxjs'; +import { map, takeUntil, catchError } from 'rxjs/operators'; import { uniq } from 'lodash'; import { duration } from 'moment'; import { i18n } from '@kbn/i18n'; @@ -177,16 +177,16 @@ export class SearchService { const serverResults$ = fetchServerResults(this.http!, params, { preference, aborted$, - }); + }).pipe(catchError(() => EMPTY)); const providersResults$ = [...this.providers.values()].map((provider) => provider.find(params, providerOptions).pipe( + catchError(() => EMPTY), takeInArray(this.maxProviderResults), takeUntil(aborted$), map((results) => results.map((r) => processResult(r))) ) ); - return merge(...providersResults$, serverResults$).pipe( map((results) => ({ results, diff --git a/x-pack/plugins/global_search/server/services/search_service.test.ts b/x-pack/plugins/global_search/server/services/search_service.test.ts index 246fbd675aba2..45824fde26afe 100644 --- a/x-pack/plugins/global_search/server/services/search_service.test.ts +++ b/x-pack/plugins/global_search/server/services/search_service.test.ts @@ -178,6 +178,44 @@ describe('SearchService', () => { }); }); + it('catches errors from providers', async () => { + const { registerResultProvider } = service.setup({ + config: createConfig(), + basePath, + }); + + getTestScheduler().run(({ expectObservable, hot }) => { + registerResultProvider( + createProvider('A', { + source: hot('a---c-|', { + a: [result('A1'), result('A2')], + c: [result('A3')], + }), + }) + ); + registerResultProvider( + createProvider('B', { + source: hot( + '-b-# ', + { + b: [result('B1')], + }, + new Error('something went bad') + ), + }) + ); + + const { find } = service.start({ core: coreStart, licenseChecker }); + const results = find({ term: 'foobar' }, {}, request); + + expectObservable(results).toBe('ab--c-|', { + a: expectedBatch('A1', 'A2'), + b: expectedBatch('B1'), + c: expectedBatch('A3'), + }); + }); + }); + it('handles the `aborted$` option', async () => { const { registerResultProvider } = service.setup({ config: createConfig(), diff --git a/x-pack/plugins/global_search/server/services/search_service.ts b/x-pack/plugins/global_search/server/services/search_service.ts index a6c2a7ee234d6..22bac036544ab 100644 --- a/x-pack/plugins/global_search/server/services/search_service.ts +++ b/x-pack/plugins/global_search/server/services/search_service.ts @@ -5,8 +5,8 @@ * 2.0. */ -import { Observable, timer, merge, throwError } from 'rxjs'; -import { map, takeUntil } from 'rxjs/operators'; +import { Observable, timer, merge, throwError, EMPTY } from 'rxjs'; +import { map, takeUntil, catchError } from 'rxjs/operators'; import { uniq } from 'lodash'; import { i18n } from '@kbn/i18n'; import { KibanaRequest, CoreStart, IBasePath } from 'src/core/server'; @@ -174,6 +174,7 @@ export class SearchService { const providersResults$ = [...this.providers.values()].map((provider) => provider.find(params, findOptions, context).pipe( + catchError(() => EMPTY), takeInArray(this.maxProviderResults), takeUntil(aborted$), map((results) => results.map((r) => processResult(r)))