Invoke .finished.failure in case of disabled retry applied to ope…

…ration (#451)

* Add test-case for bug with retry

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

* Fix

* Size

.changeset/young-drinks-warn.md

@@ -0,0 +1,5 @@
+---
+'@farfetched/core': patch
+---
+
+Invoke `.finished.failure` in case of disabled `retry` applied to operation

packages/core/package.json

@@ -41,7 +41,7 @@
   "size-limit": [
     {
       "path": "./dist/core.js",
-      "limit": "15.95 kB"
+      "limit": "16.1 kB"
     }
   ]
 }

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);
+  });
+});

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 {