Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expire errored payload with suspense: true #160

Closed
bbenezech opened this issue Nov 28, 2019 · 7 comments · Fixed by #2658
Closed

Expire errored payload with suspense: true #160

bbenezech opened this issue Nov 28, 2019 · 7 comments · Fixed by #2658
Assignees
Labels
area: suspense bug Something isn't working

Comments

@bbenezech
Copy link

bbenezech commented Nov 28, 2019

I have an error boundary with a retry button that updates its own key to rerender the whole subtree.
I want to retry the failed request on rerender (I use shouldRetryOnError: false).

I tried to rethrow the error around useSWR. I get the logs, but on next render, the xhr request isn't refired, but SWR reraises the same error as before :(

  try {
    console.log("requesting", { key });
    result = useSWR(key, fetcher, { suspense: true });
  } catch (err) {
    console.log("error", { key });
    trigger(key);
    throw err;
  }

  const { data, isValidating, revalidate } = result;

Any pointer?

Thx a lot!

@shuding shuding added bug Something isn't working and removed bug Something isn't working labels Dec 1, 2019
@shuding shuding self-assigned this Dec 4, 2019
@shuding shuding added the bug Something isn't working label Dec 4, 2019
@nulladdict
Copy link
Contributor

It seems like trigger doesn't work because here we don't get any updaters.
The reason we don't have any updaters is because they're set up in useLayoutEffect hook and in suspense mode we don't get to render and setup them if first request errors out

I think this particular issue could possibly be fixed if we set up errorKey's cache entry expiration before re-throwing it

@nulladdict
Copy link
Contributor

Hi, since #231 got merged you can now clear up the cached error value on your own.
For your example it might look something like this:

import { cache } from 'swr';
// ...
try {
  console.log("requesting", { key });
  result = useSWR(key, fetcher, { suspense: true });
} catch (err) {
  console.log("error", { key });
  const [, , errorKey] = cache.serializeKey(key);
  cache.delete(errorKey, false);
  throw err;
}
// ...
const { data, isValidating, revalidate } = result;

@bbenezech
Copy link
Author

Thanks a lot @nulladdict, you really helped me.

This works for me:

let result;
try {
  result = useSWR(key, fetcher, { suspense: true });
} catch (err) {
  const [, , errorKey] = cache.serializeKey(key);
  window.setTimeout(() => cache.delete(errorKey, false));
  throw err;
}
const { data, isValidating, revalidate } = result;

As you can see I have to delete the cache entry after rethrowing, otherwise it loops indefinitely.
The error promise is never received by my error boundary, useSWR justs goes nuts and short cuts the whole rerendering. Same with cache.__cache.delete.

Do you have any idea why deleting the entry from the cache could be somehow affecting Suspense's logic?

@nulladdict
Copy link
Contributor

nulladdict commented Mar 4, 2020

Hi @bbenezech, I think the better approach is to clear out cached error in designated error boundary component
Here's a codesandbox example of approach I've been using

Couple points to mention:

  • if we caught a promise we're just need to re-throw it since it's a suspender
  • if we caught an actual error we save info about what key caused it, so we can clear cache entry later
  • in ErrorGuard component we clear cached entry when user clicks the button, but we could also just clear it in setState callback inside componentDidCatch method instead
  • we can also clear out all cached errors if we iterate over cached keys and check if they're error or not, but that seems kinda dangerous to me

@bbenezech
Copy link
Author

Awesome! I love this, much better. Works for me.
I was confused with the suspenders in the logs, thank you for the detailed explanation.

I think we should close this?

@ivoberger
Copy link

I ran into the same problem and found this issue. Thing is, in SWR 1.x the cache has changed such that serializeKey is no longer accessible and the exported unstable_serialize only returns the regular key, not the error key. Any suggestion what to do instead?
For now I'm searching the cache keys with the request key used in useSWR hook and then deleting everything that has it in there. That's quite hacky though so a better solution would be appreciated.

@dgioulakis
Copy link

Is there any update on this? How should this be handled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: suspense bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants