From 1ec644ef9e9b3e810951dc6b2bc8ee9573b90155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20Kamy=C5=9Fev?= Date: Wed, 23 Aug 2023 16:47:12 +0700 Subject: [PATCH] `supressIntermidiateErrors` to `retry` (#340) * Add param supressIntermidiateErrors to retry operator * Add comment * Improve test and show bug * Next steps of solution * Solved * Simplify solution * Add option `supressIntermediateErrors` to `retry` operator * INcrease size-limit * Fix typo * Allow to use otherwise and supressIntermediateErrors * Improve docs --- .changeset/spotty-poems-play.md | 5 ++ apps/website/docs/api/operators/retry.md | 1 + apps/website/docs/releases/0-9.md | 16 ++++- apps/website/docs/tutorial/retries.md | 16 +++++ packages/core/project.json | 2 +- .../create_remote_operation.ts | 40 ++++++----- .../src/retry/__tests__/retry.query.test.ts | 40 +++++++++++ packages/core/src/retry/retry.ts | 67 +++++++++++++++++-- 8 files changed, 165 insertions(+), 22 deletions(-) create mode 100644 .changeset/spotty-poems-play.md diff --git a/.changeset/spotty-poems-play.md b/.changeset/spotty-poems-play.md new file mode 100644 index 000000000..00acf862e --- /dev/null +++ b/.changeset/spotty-poems-play.md @@ -0,0 +1,5 @@ +--- +'@farfetched/core': minor +--- + +Add option `supressIntermediateErrors` to `retry` operator diff --git a/apps/website/docs/api/operators/retry.md b/apps/website/docs/api/operators/retry.md index 2b9243b0e..7936f6282 100644 --- a/apps/website/docs/api/operators/retry.md +++ b/apps/website/docs/api/operators/retry.md @@ -17,6 +17,7 @@ Config fields: - `(params, { attempt }) => mapped` - `{ source: Store, fn: (params, { attempt }, source) => mapped }` - `otherwise?`: [_Event_](https://effector.dev/docs/api/effector/event) or [_Effect_](https://effector.dev/docs/api/effector/effect), that will be called after the last attempt if the [_Query_](/api/primitives/query) is still failed +- `supressIntermediateErrors?`: _boolean_ whether to suppress intermediate errors or not, defaults to `false`. If `true`, then the [_Query_](/api/primitives/query) will not be marked as failed until the last attempt is failed. ## Build-in delays diff --git a/apps/website/docs/releases/0-9.md b/apps/website/docs/releases/0-9.md index 9441bbe1b..8e7c8342a 100644 --- a/apps/website/docs/releases/0-9.md +++ b/apps/website/docs/releases/0-9.md @@ -16,6 +16,20 @@ Since v0.9 is technical release with no new significant features, there are a fe ### Separate [_Event_](https://effector.dev/docs/api/effector/event) for [_Query_](/api/primitives/query) cancelation -Cancelled [_Queries_](/api/primitives/query) were treated as failed before this release. We have added a separate [_Event_](https://effector.dev/docs/api/effector/event) `.aborted` to [_Query_](/api/primitives/query) to distinguish between cancelation and failure. It is recommended to use `.aborted` instead of `.finished.failure` to handle cancelation. In the next release v0.10 cancelation will not be treated as failure anymore, so you will have to handle it explicitly. +Cancelled [_Queries_](/api/primitives/query) were treated as failed before this release. We have added a separate [_Event_](https://effector.dev/docs/api/effector/event) `.aborted` to [_Query_](/api/primitives/query) to distinguish between cancelation and failure. It is recommended to use `.aborted` instead of `.finished.failure` to handle cancelation. + +:::warning +In the next release v0.10 cancelation will not be treated as failure anymore, so you will have to handle it explicitly. +::: + +### `supressIntermediateErrors` in `retry` operator + +Before this release, [`retry`](/api/operators/retry) operator was marking [_Query_](/api/primitives/query) as failed on every failed attempt. Since v0.9 it accepts options `supressIntermediateErrors` to overwrite this behavior. If `true`, then the [_Query_](/api/primitives/query) will not be marked as failed until the last attempt is failed. + +:::warning + +In the next release v0.10 `supressIntermediateErrors` will be true `true` by default. To restore the previous behavior, you will have to set it to `false` explicitly. + +::: diff --git a/apps/website/docs/tutorial/retries.md b/apps/website/docs/tutorial/retries.md index 9e5c19b99..8ac1c1f08 100644 --- a/apps/website/docs/tutorial/retries.md +++ b/apps/website/docs/tutorial/retries.md @@ -145,6 +145,22 @@ retry(characterQuery, { `mapParams` accepts a function that takes current parameters, occurred error and attempt number and returns new parameters. In this example, we just add `attempt` parameter to the current parameters. +## Intermediate errors + +By default, `retry` does not supress errors, so if the operation failed, it will fail as well. But sometimes, we want to suppress errors and check for status only after all retries are failed. To do this, we can use the `supressIntermediateErrors` option of the `retry` operator. + +```ts +retry(characterQuery, { + times: 5, + delay: 500, + supressIntermediateErrors: true, +}); +``` + +::: tip +In the next release v0.10 `supressIntermediateErrors` will be true `true` by default. To restore the previous behavior, you will have to set it to `false` explicitly. If you want to be ready for this change, you can set it to `false` already. +::: + ## API reference You can find the full API reference for the `retry` operator in the [API reference](/api/operators/retry). diff --git a/packages/core/project.json b/packages/core/project.json index 8c2e10187..8b8525fa7 100644 --- a/packages/core/project.json +++ b/packages/core/project.json @@ -71,7 +71,7 @@ "size": { "executor": "./tools/executors/size-limit:size-limit", "options": { - "limit": "18.9 kB", + "limit": "20 kB", "outputPath": "dist/packages/core" }, "dependsOn": [ diff --git a/packages/core/src/remote_operation/create_remote_operation.ts b/packages/core/src/remote_operation/create_remote_operation.ts index 0345bfa8b..6164657b4 100644 --- a/packages/core/src/remote_operation/create_remote_operation.ts +++ b/packages/core/src/remote_operation/create_remote_operation.ts @@ -92,7 +92,6 @@ export function createRemoteOperation< unknown >(async ({ params }) => { const result = await executeFx(params); - return { result, stale: false }; }), }; @@ -229,12 +228,13 @@ export function createRemoteOperation< sample({ clock: retrieveDataFx.fail, - fn: ({ error, params }) => ({ - error: error, + source: $enabled, + filter: (enabled, { error }) => enabled && !error.stopErrorPropagation, + fn: (_, { error, params }) => ({ + error: error.error as any, params: params.params, - meta: { stopErrorPropagation: false, stale: false }, + meta: { stopErrorPropagation: error.stopErrorPropagation, stale: false }, }), - filter: $enabled, target: finished.failure, }); @@ -374,22 +374,32 @@ function createDataSourceHandlers(dataSources: DataSource[]) { meta: ExecutionMeta; }, { result: unknown; stale: boolean }, - any + { stopErrorPropagation: boolean; error: unknown } >({ handler: async ({ params, skipStale }) => { for (const dataSource of dataSources) { - const fromSource = await dataSource.get({ params }); - - if (skipStale && fromSource?.stale) { - continue; - } - - if (fromSource) { - return fromSource; + try { + const fromSource = await dataSource.get({ params }); + + if (skipStale && fromSource?.stale) { + continue; + } + + if (fromSource) { + return fromSource; + } + } catch (error) { + throw { + stopErrorPropagation: false, + error, + }; } } - throw new Error('No data source returned data'); + throw { + stopErrorPropagation: false, + error: new Error('No data source returned data'), + }; }, }); diff --git a/packages/core/src/retry/__tests__/retry.query.test.ts b/packages/core/src/retry/__tests__/retry.query.test.ts index a16d90af4..bc3033e42 100644 --- a/packages/core/src/retry/__tests__/retry.query.test.ts +++ b/packages/core/src/retry/__tests__/retry.query.test.ts @@ -338,4 +338,44 @@ describe('retry with query', () => { // 1 for retry expect(listeners.onFailure).toBeCalledTimes(2); }); + + test('throw error in case of retry with supressIntermediateErrors', async () => { + const query = createQuery({ + handler: vi.fn().mockImplementation(({ attempt }) => { + throw new Error(`Sorry, attempt ${attempt}`); + }), + }); + + retry(query, { + times: 1, + mapParams({ meta }) { + return { attempt: meta.attempt }; + }, + delay: 0, + supressIntermediateErrors: true, + }); + + const scope = fork(); + + const { listeners } = watchRemoteOperation(query, scope); + + await allSettled(query.start, { scope, params: { attempt: 0 } }); + + // 1 for retry + expect(listeners.onFailure).toBeCalledTimes(1); + expect(listeners.onFailure.mock.calls[0]).toMatchInlineSnapshot(` + [ + { + "error": [Error: Sorry, attempt 1], + "meta": { + "stale": false, + "stopErrorPropagation": false, + }, + "params": { + "attempt": 1, + }, + }, + ] + `); + }); }); diff --git a/packages/core/src/retry/retry.ts b/packages/core/src/retry/retry.ts index 748c3c3b2..aa56fe00f 100644 --- a/packages/core/src/retry/retry.ts +++ b/packages/core/src/retry/retry.ts @@ -1,10 +1,16 @@ import { combine, + createEffect, createEvent, createStore, sample, split, + attach, + scopeBind, type Event, + type EffectError, + type EffectParams, + type EffectResult, } from 'effector'; import { @@ -44,6 +50,7 @@ type RetryConfig< MapParamsSource >; otherwise?: Event>; + supressIntermediateErrors?: boolean; }; export function retry< @@ -58,9 +65,11 @@ export function retry< delay: timeout, filter, mapParams, - otherwise, + ...params }: RetryConfig ): void { + const supressIntermediateErrors = params.supressIntermediateErrors ?? false; + const $maxAttempts = normalizeStaticOrReactive(times); const $attempt = createStore(1, { serialize: 'ignore', @@ -70,18 +79,30 @@ export function retry< attempt: $attempt, }); + const $supressError = combine( + $attempt, + $maxAttempts, + (attempt, maxAttempts) => + supressIntermediateErrors && attempt <= maxAttempts + ); + + const failed = createEvent<{ + params: RemoteOperationParams; + error: RemoteOperationError; + }>(); + const newAttempt = createEvent(); const { planNextAttempt, __: retriesAreOver } = split( sample({ - clock: operation.finished.failure, + clock: failed, source: { maxAttempts: $maxAttempts, attempt: $attempt, }, filter: normalizeSourced({ field: (filter ?? true) as any, - clock: operation.finished.failure, + clock: failed, }), fn: ({ attempt, maxAttempts }, { params, error }) => ({ params, @@ -117,7 +138,43 @@ export function retry< .on(newAttempt, (attempt) => attempt + 1) .reset([operation.finished.success, operation.start]); - if (otherwise) { - sample({ clock: retriesAreOver, target: otherwise }); + if (params.otherwise) { + sample({ clock: retriesAreOver, target: params.otherwise }); } + + if (supressIntermediateErrors) { + const originalFx = + operation.__.lowLevelAPI.dataSourceRetrieverFx.use.getCurrent(); + + operation.__.lowLevelAPI.dataSourceRetrieverFx.use( + attach({ + source: $supressError, + mapParams: (opts, supressError) => ({ ...opts, supressError }), + effect: createEffect< + EffectParams< + typeof operation.__.lowLevelAPI.dataSourceRetrieverFx + > & { supressError: boolean }, + EffectResult, + EffectError + >(async ({ supressError, ...opts }) => { + const boundFailed = scopeBind(failed, { safe: true }); + try { + const result = await originalFx(opts); + + return result; + } catch (error: any) { + if (supressError) { + boundFailed({ params: opts.params, error: error.error }); + + throw { error: error.error, stopErrorPropagation: true }; + } else { + throw error; + } + } + }), + }) + ); + } + + sample({ clock: operation.finished.failure, target: failed }); }