Repository: igorkamyshev/farfetched
Pull request #451 (2024-03-11 08:31:56)

Invoke `.finished.failure` in case of disabled `retry` applied to operation

github-actions[bot]: # Farfetched Canary

Latest canary version is "0.12.1"

[More information about canary versions](https://farfetched.pages.dev/releases/canary.html)

---

Used changeset: "[]"
cloudflare-pages[bot]: ## Deploying with &nbsp;<a href="https://pages.dev"><img alt="Cloudflare Pages" src="https://user-images.githubusercontent.com/23264/106598434-9e719e00-654f-11eb-9e59-6167043cfa01.png" width="16"></a> &nbsp;Cloudflare Pages

<table><tr><td><strong>Latest commit:</strong> </td><td>
<code>0f72546</code>
</td></tr>
<tr><td><strong>Status:</strong></td><td>&nbsp;✅&nbsp; Deploy successful!</td></tr>
<tr><td><strong>Preview URL:</strong></td><td>
<a href='https://9738fc1d.farfetched.pages.dev'>https://9738fc1d.farfetched.pages.dev</a>
</td></tr>
<tr><td><strong>Branch Preview URL:</strong></td><td>
<a href='https://retry-bugfix.farfetched.pages.dev'>https://retry-bugfix.farfetched.pages.dev</a>
</td></tr>
</table>

[View logs](https://dash.cloudflare.com/?to=/:account/pages/view/farfetched/9738fc1d-c738-426c-bc46-8528b846b1c8)


Patch:
diff --git a/.changeset/young-drinks-warn.md b/.changeset/young-drinks-warn.md
new file mode 100644
index 0000000..812b908
--- /dev/null
+++ b/.changeset/young-drinks-warn.md
@@ -0,0 +1,5 @@
+---
+'@farfetched/core': patch
+---
+
+Invoke `.finished.failure` in case of disabled `retry` applied to operation
diff --git a/packages/core/package.json b/packages/core/package.json
index 36c3d03..d7faf2c 100644
--- a/packages/core/package.json
+++ b/packages/core/package.json
@@ -41,7 +41,7 @@
   "size-limit": [
     {
       "path": "./dist/core.js",
-      "limit": "15.95 kB"
+      "limit": "16.1 kB"
     }
   ]
 }
diff --git a/packages/core/src/retry/retry.ts b/packages/core/src/retry/retry.ts
index ac53dd6..9a95ba4 100644
--- a/packages/core/src/retry/retry.ts
+++ b/packages/core/src/retry/retry.ts
@@ -98,15 +98,17 @@ export function retry<
 
   const newAttempt = createEvent();
 
+  const $partialFilter = normalizeSourced({
+    field: filter ?? true,
+  });
+
   const { planNextAttempt, __: retriesAreOver } = split(
     sample({
       clock: failed,
       source: {
         maxAttempts: $maxAttempts,
         attempt: $attempt,
-        partialFilter: normalizeSourced({
-          field: filter ?? true,
-        }),
+        partialFilter: $partialFilter,
       },
       filter: ({ partialFilter }, clock) => partialFilter(clock),
       fn: ({ attempt, maxAttempts }, { params, error, meta }) => ({
@@ -160,27 +162,46 @@ export function retry<
 
     operation.__.lowLevelAPI.dataSourceRetrieverFx.use(
       attach({
-        source: $supressError,
-        mapParams: (opts, supressError) => ({ ...opts, supressError }),
+        source: { supressError: $supressError, partialFilter: $partialFilter },
+        mapParams: (opts, { supressError, partialFilter }) => ({
+          ...opts,
+          supressError,
+          partialFilter,
+        }),
         effect: createEffect<
           EffectParams<
             typeof operation.__.lowLevelAPI.dataSourceRetrieverFx
-          > & { supressError: boolean },
+          > & {
+            supressError: boolean;
+            partialFilter: (params: FailInfo<Q>) => boolean;
+          },
           EffectResult<typeof operation.__.lowLevelAPI.dataSourceRetrieverFx>,
           EffectError<typeof operation.__.lowLevelAPI.dataSourceRetrieverFx>
-        >(async ({ supressError, ...opts }) => {
+        >(async ({ supressError, partialFilter, ...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,
-                meta: opts.meta,
-              });
+            const failInfo = {
+              params: opts.params,
+              error: error.error,
+              meta: opts.meta,
+            };
+
+            if (
+              /*
+               * If filter returns false, this fail is not supposed to be retried
+               * so we should not suppress this error in any case.
+               *
+               * If filter returns is true, we should suppress this error only if
+               * supressError is true.
+               */
+              partialFilter(failInfo) &&
+              supressError
+            ) {
+              boundFailed(failInfo);
 
               throw { error: error.error, stopErrorPropagation: true };
             } else {


Test patch:
diff --git a/packages/core/src/retry/__tests__/retry.test.ts b/packages/core/src/retry/__tests__/retry.test.ts
new file mode 100644
index 0000000..ea49537
--- /dev/null
+++ b/packages/core/src/retry/__tests__/retry.test.ts
@@ -0,0 +1,51 @@
+import { test, describe, vi, expect } from 'vitest';
+import { allSettled, createWatch, fork } from 'effector';
+
+import { unknownContract } from '../../contract/unknown_contract';
+import { createJsonMutation } from '../../mutation/create_json_mutation';
+import { httpError } from '../../errors/create_error';
+import { retry } from '../retry';
+
+describe('retry', () => {
+  test('does not suppress finished.failure event after all retries in case of filter', async () => {
+    const m = createJsonMutation({
+      request: {
+        url: 'api.salo.com',
+        method: 'POST',
+      },
+      response: {
+        contract: unknownContract,
+      },
+    });
+
+    retry(m, {
+      times: 3,
+      delay: 100,
+      /* Always return false, basically disabled retry */
+      filter: () => false,
+    });
+
+    const onFailure = vi.fn();
+
+    const scope = fork({
+      handlers: [
+        [
+          m.__.executeFx,
+          () => {
+            throw httpError({
+              status: 400,
+              statusText: 'Sorry',
+              response: 'Sorry',
+            });
+          },
+        ],
+      ],
+    });
+
+    createWatch({ unit: m.finished.failure, scope, fn: onFailure });
+
+    await allSettled(m.start, { scope });
+
+    expect(onFailure).toBeCalledTimes(1);
+  });
+});