From 156c338510afcf3f9a5ef3812ced86ec1954f85c Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Thu, 12 Dec 2024 16:40:24 +0100 Subject: [PATCH] fix(error): directly emit error from events (#6472) * fix(error): directly emit error from events In `@algolia/events`, errors are properly rethrown if they are thrown with `.emit('error', new Error("msg"))`, not if they are thrown with `.emit('error', { error: new Error('msg') })`. Therefore in InstantSearch and the helper, all errors are now properly thrown directly in the emit. [FX-3187] BREAKING CHANGE: if you have any `on('error'` handler that reads `event.error` from the callback, change that to just `error` * !fixup --- packages/algoliasearch-helper/index.d.ts | 4 ++-- .../src/algoliasearch.helper.js | 16 ++++------------ .../integration-spec/helper.distinct.facet.js | 4 ++-- .../test/integration-spec/helper.filters.js | 4 ++-- .../test/integration-spec/helper.numerics.js | 8 ++++---- .../test/integration-spec/helper.searchOnce.js | 4 ++-- .../test/integration-spec/helper.tags.js | 4 ++-- .../test/spec/algoliasearch.helper/events.js | 8 ++------ .../__tests__/instantsearch-integration.test.ts | 5 +---- .../src/__tests__/instantsearch.test.tsx | 4 ++-- packages/instantsearch-core/src/instantsearch.ts | 16 +++++----------- 11 files changed, 28 insertions(+), 49 deletions(-) diff --git a/packages/algoliasearch-helper/index.d.ts b/packages/algoliasearch-helper/index.d.ts index 67cda4d5f6..3dc6603c51 100644 --- a/packages/algoliasearch-helper/index.d.ts +++ b/packages/algoliasearch-helper/index.d.ts @@ -74,7 +74,7 @@ declare namespace algoliasearchHelper { event: 'result', cb: (res: { results: SearchResults; state: SearchParameters }) => void ): this; - on(event: 'error', cb: (res: { error: Error }) => void): this; + on(event: 'error', cb: (res: Error) => void): this; on(event: 'searchQueueEmpty', cb: () => void): this; /** @@ -338,7 +338,7 @@ declare namespace algoliasearchHelper { }; }) => void ): this; - on(event: 'error', cb: (res: { error: Error }) => void): this; + on(event: 'error', cb: (res: Error) => void): this; lastResults: SearchResults | null; lastRecommendResults: RecommendResults | null; diff --git a/packages/algoliasearch-helper/src/algoliasearch.helper.js b/packages/algoliasearch-helper/src/algoliasearch.helper.js index a782ec57ff..593aa6647e 100644 --- a/packages/algoliasearch-helper/src/algoliasearch.helper.js +++ b/packages/algoliasearch-helper/src/algoliasearch.helper.js @@ -1329,9 +1329,7 @@ AlgoliaSearchHelper.prototype._search = function (options) { .catch(this._dispatchAlgoliaError.bind(this, queryId)); } catch (error) { // If we reach this part, we're in an internal error state - this.emit('error', { - error: error, - }); + this.emit('error', error); } return undefined; @@ -1420,9 +1418,7 @@ AlgoliaSearchHelper.prototype._recommend = function () { .catch(this._dispatchRecommendError.bind(this, queryId)); } catch (error) { // If we reach this part, we're in an internal error state - this.emit('error', { - error: error, - }); + this.emit('error', error); } return; @@ -1579,9 +1575,7 @@ AlgoliaSearchHelper.prototype._dispatchAlgoliaError = function ( this._currentNbQueries -= queryId - this._lastQueryIdReceived; this._lastQueryIdReceived = queryId; - this.emit('error', { - error: error, - }); + this.emit('error', error); if (this._currentNbQueries === 0) this.emit('searchQueueEmpty'); }; @@ -1599,9 +1593,7 @@ AlgoliaSearchHelper.prototype._dispatchRecommendError = function ( queryId - this._lastRecommendQueryIdReceived; this._lastRecommendQueryIdReceived = queryId; - this.emit('error', { - error: error, - }); + this.emit('error', error); if (this._currentNbRecommendQueries === 0) this.emit('recommendQueueEmpty'); }; diff --git a/packages/algoliasearch-helper/test/integration-spec/helper.distinct.facet.js b/packages/algoliasearch-helper/test/integration-spec/helper.distinct.facet.js index dd1f1723aa..f586079c14 100644 --- a/packages/algoliasearch-helper/test/integration-spec/helper.distinct.facet.js +++ b/packages/algoliasearch-helper/test/integration-spec/helper.distinct.facet.js @@ -31,8 +31,8 @@ test('[INT][FILTERS] Using distinct should let me retrieve all facet without dis facets: ['colors'], }); - helper.on('error', function (event) { - done.fail(event.error); + helper.on('error', function (error) { + done.fail(error); }); helper.on('result', function (event) { diff --git a/packages/algoliasearch-helper/test/integration-spec/helper.filters.js b/packages/algoliasearch-helper/test/integration-spec/helper.filters.js index 9d85f1cc4f..393b1287b2 100644 --- a/packages/algoliasearch-helper/test/integration-spec/helper.filters.js +++ b/packages/algoliasearch-helper/test/integration-spec/helper.filters.js @@ -32,8 +32,8 @@ test('[INT][FILTERS] Should retrieve different values for multi facetted records var calls = 0; - helper.on('error', function (event) { - done.fail(event.error); + helper.on('error', function (error) { + done.fail(error); }); helper.on('result', function (event) { diff --git a/packages/algoliasearch-helper/test/integration-spec/helper.numerics.js b/packages/algoliasearch-helper/test/integration-spec/helper.numerics.js index fdb92f90b4..96d4be696c 100644 --- a/packages/algoliasearch-helper/test/integration-spec/helper.numerics.js +++ b/packages/algoliasearch-helper/test/integration-spec/helper.numerics.js @@ -32,8 +32,8 @@ test('[INT][NUMERICS][RAW-API]Test numeric operations on the helper and their re var calls = 0; - helper.on('error', function (event) { - done.fail(event.error); + helper.on('error', function (error) { + done.fail(error); }); helper.on('result', function (event) { @@ -84,8 +84,8 @@ test('[INT][NUMERICS][MANAGED-API]Test numeric operations on the helper and thei var calls = 0; - helper.on('error', function (event) { - done.fail(event.error); + helper.on('error', function (error) { + done.fail(error); }); helper.on('result', function (event) { diff --git a/packages/algoliasearch-helper/test/integration-spec/helper.searchOnce.js b/packages/algoliasearch-helper/test/integration-spec/helper.searchOnce.js index 85cf869e53..f292f56497 100644 --- a/packages/algoliasearch-helper/test/integration-spec/helper.searchOnce.js +++ b/packages/algoliasearch-helper/test/integration-spec/helper.searchOnce.js @@ -30,8 +30,8 @@ test('[INT][SEARCHONCE] Should be able to search once with custom parameters wit var state0 = helper.state; var calls = 1; - helper.on('error', function (event) { - done.fail(event.error); + helper.on('error', function (error) { + done.fail(error); }); helper.on('result', function (event) { diff --git a/packages/algoliasearch-helper/test/integration-spec/helper.tags.js b/packages/algoliasearch-helper/test/integration-spec/helper.tags.js index 62bc63ddb9..3337f415db 100644 --- a/packages/algoliasearch-helper/test/integration-spec/helper.tags.js +++ b/packages/algoliasearch-helper/test/integration-spec/helper.tags.js @@ -32,8 +32,8 @@ test('[INT][TAGS]Test tags operations on the helper and their results on the alg var calls = 0; - helper.on('error', function (event) { - done.fail(event.error); + helper.on('error', function (error) { + done.fail(error); }); helper.on('result', function (event) { diff --git a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/events.js b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/events.js index 8712e0ae56..6a93fb2ee6 100644 --- a/packages/algoliasearch-helper/test/spec/algoliasearch.helper/events.js +++ b/packages/algoliasearch-helper/test/spec/algoliasearch.helper/events.js @@ -356,9 +356,7 @@ test('error event should be emitted once the request is complete with errors', f return runAllMicroTasks().then(function () { expect(errored).toHaveBeenCalledTimes(1); - expect(errored).toHaveBeenLastCalledWith({ - error: expect.any(Error), - }); + expect(errored).toHaveBeenLastCalledWith(expect.any(Error)); }); }); @@ -381,7 +379,5 @@ test('error event should be emitted if an error happens at request time', functi helper.search(); expect(errored).toHaveBeenCalledTimes(1); - expect(errored).toHaveBeenLastCalledWith({ - error: expect.any(Error), - }); + expect(errored).toHaveBeenLastCalledWith(expect.any(Error)); }); diff --git a/packages/instantsearch-core/src/__tests__/instantsearch-integration.test.ts b/packages/instantsearch-core/src/__tests__/instantsearch-integration.test.ts index 27ccea5507..d32cc8b082 100644 --- a/packages/instantsearch-core/src/__tests__/instantsearch-integration.test.ts +++ b/packages/instantsearch-core/src/__tests__/instantsearch-integration.test.ts @@ -132,14 +132,11 @@ describe('errors', () => { search.addWidgets([connectSearchBox(() => {})({})]); - expect.assertions(4); + expect.assertions(2); search.on('error', (error) => { expect(error).toBeInstanceOf(Error); expect(error.message).toBe('test!'); - - expect(error.error).toBeInstanceOf(Error); - expect(error.error.message).toBe('test!'); }); search.start(); diff --git a/packages/instantsearch-core/src/__tests__/instantsearch.test.tsx b/packages/instantsearch-core/src/__tests__/instantsearch.test.tsx index e7681db7c9..8d10700904 100644 --- a/packages/instantsearch-core/src/__tests__/instantsearch.test.tsx +++ b/packages/instantsearch-core/src/__tests__/instantsearch.test.tsx @@ -1108,9 +1108,9 @@ describe('start', () => { search.addWidgets([virtualSearchBox({})]); search.start(); - search.on('error', (event) => { + search.on('error', (error) => { expect(searchClient.search).toHaveBeenCalledTimes(1); - expect(event.error).toEqual(new Error('SERVER_ERROR')); + expect(error).toEqual(new Error('SERVER_ERROR')); done(); }); }); diff --git a/packages/instantsearch-core/src/instantsearch.ts b/packages/instantsearch-core/src/instantsearch.ts index a0f2a14c64..bf08fdc793 100644 --- a/packages/instantsearch-core/src/instantsearch.ts +++ b/packages/instantsearch-core/src/instantsearch.ts @@ -402,28 +402,22 @@ See documentation: ${createDocumentationLink({ // Only the "main" Helper emits the `error` event vs the one for `search` // and `results` that are also emitted on the derived one. - mainHelper.on('error', ({ error }) => { + mainHelper.on('error', (error) => { if (!(error instanceof Error)) { // typescript lies here, error is in some cases { name: string, message: string } const err = error as Record; - error = Object.keys(err).reduce((acc, key) => { + this.error = Object.keys(err).reduce((acc, key) => { (acc as any)[key] = err[key]; return acc; }, new Error(err.message)); + } else { + this.error = error; } - // If an error is emitted, it is re-thrown by events. In previous versions - // we emitted {error}, which is thrown as: - // "Uncaught, unspecified \"error\" event. ([object Object])" - // To avoid breaking changes, we make the error available in both - // `error` and `error.error` - // @MAJOR emit only error - (error as any).error = error; - this.error = error; this.status = 'error'; this.scheduleRender(false); // This needs to execute last because it throws the error. - this.emit('error', error); + this.emit('error', this.error); }); this.mainHelper = mainHelper;