swr icon indicating copy to clipboard operation
swr copied to clipboard

Retry on error with suspense

Open micha149 opened this issue 2 years ago • 4 comments

Bug report

I use swr with graphql-request and I am struggling with the automatic retry on an api error. Only a single request is made until the error is raised to my ErrorBoundary. If I set { suspense: false } two requests are made and the data shows up as expected.

Expected Behavior

I would expect that swr handles the retry also in suspense mode. From the outside any retry should behave as a very long running request.

const useSomeHook = (key: string, enableSuspense: boolean): string | undefined => {
    const calls = useRef<number>(0);

    const fetcher = (): string => {
        calls.current += 1;

        if (calls.current < 2) {
            throw new Error('Something\'s worng');
        }

        return 'yay';
    };

    const { data } = useSwr(key, fetcher, { suspense: enableSuspense, errorRetryInterval: 10 });

    return data;
};

describe('Retry on failing fetcher', () => {
    it('renders correctly without suspense', async () => {
        const { result, waitFor } = renderHook(() => useSomeHook('myKey', false));

        await waitFor(() => result.current !== undefined);

        expect(result.current).toEqual('yay'); // OK
    });

    it('renders correctly with suspense', async () => {
        const { result, waitForNextUpdate } = renderHook(() => useSomeHook('myOtherKey', true));

        await waitForNextUpdate();

        expect(result.current).toEqual('yay'); // Fails with `Something's worng`
    });
});

I created a codesandbox to hopefully make the problem a bit more understandable: https://codesandbox.io/s/swr-suspesnse-retry-bug-sqpjyn?file=/src/App.js

Additional Context

micha149 avatar Apr 06 '22 08:04 micha149

this is a useful option if there is an error and you want to retry requests. if you get the data it will not request again.

Reference Options

useSWR(key, fetcher, {
  revalidateIfStale: false,
  revalidateOnFocus: false,
  revalidateOnReconnect: false,
  errorRetryInterval:0,
  shouldRetryOnError:true,
})

Babailan avatar Apr 07 '22 02:04 Babailan

@Babailan sorry for the delay. I was on vacation as you replied and missed to answer.

I don't understand what you mean. I am aware of the usefulness of the retry feature. My report is, that it seems not to work when enabling suspense.

micha149 avatar Apr 29 '22 06:04 micha149

@micha149 I think you didn't provide the Suspense Component. and i thought your problem is 2 request has been made everytime you are requesting.

suspense: https://17.reactjs.org/docs/concurrent-mode-suspense.html

Babailan avatar Apr 30 '22 13:04 Babailan

@Babailan I would like to say that I know what I am doing. Concepts like suspense are familiar to me.

I created a codesandbox to demonstrate my issue. In this box a fake fetcher will fail for the first request, for a retry it returns some data so the greeting message can be displayed. Set the variable SUSPENSE_ENABLED to true and it will not work anymore.

https://codesandbox.io/s/swr-suspesnse-retry-bug-sqpjyn?file=/src/App.js

micha149 avatar May 03 '22 13:05 micha149

Is there any progress on this issue? I'm having same problem.

Siihyun avatar Nov 11 '22 06:11 Siihyun

I've encountered this also. I can see why not refetching on render makes sense for non-suspense usage (errors in non-suspense don't suspend rendering).

For suspense, I think this is unnecessary - a thrown error will suspend rendering. When the error guard retries rendering the failed path, IMHO, it should try the request again (I think this is what is done in urql). Is there openness to a PR to add this functionality?

Edit: Tried to nuke the cache as a temporary workaround but that isn't possible due to the context not being exported.

andyrichardson avatar Dec 14 '22 16:12 andyrichardson

I got around this issue by writing a middleware which calls my custom onErrorRetry and emulates what SWR should do: retry until success, which shows the Suspense fallback in the meantime (in my case a loading screen).

const retrySuspenseSWRMiddleware: Middleware = (useSWRNext: SWRHook) => {
  return (key, fetcher, options) => {
    if (options.suspense) {
      const suspenseFetcher: typeof fetcher = async (fetcherOptions) =>
        new Promise(async (resolve, reject) => {
          let retryCount = 0;

          async function retry() {
            try {
              const data = await fetcher(fetcherOptions);
              resolve(data);
              return data;
            } catch (error) {
              retryCount += 1;
              onErrorRetry(error, retryCount, retry);
            }
          }

          try {
            await retry();
          } catch (error) {
            reject(error);
          }
        });
      return useSWRNext(key, suspenseFetcher, options);
    }
    return useSWRNext(key, fetcher, options);
  };
};

Then I passed it in my SWRConfig using use: [retrySuspenseSWRMiddleware], but you can also pass it to useSWR instead.

The setTimeout etc are happening inside onErrorRetry, so this piece of code doesn't actually retry: it catches the exception instead of throwing it and triggers the onErrorRetry callback. ~If you want to reuse the default onErrorRetry, I'm afraid you have to copy it from here: https://github.com/vercel/swr/blob/e81d22f4121743c75b6b0998cc0bbdbe659889c1/_internal/utils/config.ts#L16-L38~

EDIT: Turns out you can call it using options.onErrorRetry, even if you don't specify a custom onErrorRetry anywhere.

BertrandBordage avatar Apr 19 '23 10:04 BertrandBordage

Generic workaround that should work for everybody without any other change:

const retrySuspenseSWRMiddleware: Middleware = (useSWRNext: SWRHook) => {
  return (key: string, fetcher, options) => {
    if (options.suspense) {
      const suspenseFetcher: typeof fetcher = async (fetcherOptions) =>
        new Promise(async (resolve, reject) => {
          async function revalidate(
            revalidateOpts?: RevalidatorOptions
          ): Promise<boolean> {
            const { retryCount = 0, dedupe = true } = revalidateOpts ?? {};
            try {
              const data = await fetcher(fetcherOptions);
              resolve(data);
              return true;
            } catch (error) {
              options.onErrorRetry(error, key, options, revalidate, {
                retryCount: retryCount + 1,
                dedupe,
              });
            }
          }

          try {
            await revalidate();
          } catch (error) {
            reject(error);
          }
        });
      return useSWRNext(key, suspenseFetcher, options);
    }
    return useSWRNext(key, fetcher, options);
  };
};

and then use it with use: [retrySuspenseSWRMiddleware] in SWRConfig or useSWR.

BertrandBordage avatar Apr 19 '23 10:04 BertrandBordage