Allow handling some errors locally while using error boundaries

[dantman]

This adds a shouldThrowError option that can be used when useErrorBoundary/suspense to decide whether an error should be thrown or handled locally on the error state.

It's nice to have errors thrown to error boundaries. However sometimes you have some errors you do want to handle within the component instead of having them thrown to external boundaries.

e.g. In a product list page you may want to handle an "invalid product filter" from the server with an in-list error instead of it being thrown out to an error boundary. But you still may want to use an error boundary so 500 errors from the server and network errors still get thrown out to a shared error boundary.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tannerlinsley/react-query/AEXSu8vr5hqMeCUqgRsokV4n1p5K
✅ Preview: https://react-query-git-fork-dantman-should-throw-tannerlinsley.vercel.app

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3b2da89:

Sandbox	Source
tannerlinsley/react-query: basic	Configuration
tannerlinsley/react-query: basic-typescript	Configuration

[TkDodo]

I like the idea, but not really the api: an extra boolean flag that only works if another flag is set as well :/

How about extending the useErrorBoundary flag instead to: boolean | (error) => boolean, would that also work? It would also be in line with what e.g. initialData supports (retry also works like that), and it would also be backwards compatible

[TkDodo]

Also, I think if you use suspense, you have to throw all errors - it’s how suspense works

[dantman]

How about extending the useErrorBoundary flag instead to: boolean | (error) => boolean, would that also work? It would also be in line with what e.g. initialData supports (retry also works like that), and it would also be backwards compatible

I thought about that. It also applies to suspense so seemed a bit odd. And the implementation a bit messier, having to handle it being boolean or function. But if you think so, I could refactor it to work that way.

Also, I think if you use suspense, you have to throw all errors - it’s how suspense works

I'm not so sure about that. While the error boundary is integral with suspense, I don't believe there's anything in suspense that says you can't decide an error state is part of state instead.

[TkDodo]

I don't believe there's anything in suspense that says you can't decide an error state is part of state instead

It’s mentioned here: https://reactjs.org/docs/concurrent-mode-suspense.html#handling-errors

since react renders before the promise resolves, I think it’s mandatory.

But if you think so, I could refactor it to work that way.

I think the boolean or function api would be more in line with how other flags handle these situations. If I’m right about suspense, the useErrorBoundary doesn’t do anything here, so if you set it to false (or a function), it should have no effect. We can also loop in @tannerlinsley

[dantman]

It’s mentioned here: https://reactjs.org/docs/concurrent-mode-suspense.html#handling-errors

since react renders before the promise resolves, I think it’s mandatory.

That only documents how React uses an error boundary to catch errors thrown in components instead of catch. Not an assertion that all suspense uses cannot render instead of throwing.

Suspense works by throwing a promise and then re-rendering when that is fulfilled, at which point you render the full component. I will concede that it's possible that when a promise is rejected React could consider that thrown, instead of rendering the component and expecting the throw result.error in useBaseQuery to throw the error.

However that doesn't actually matter because the suspense implementation in useBaseQuery actually catches any error and does not re-throw it.
https://github.com/tannerlinsley/react-query/blob/21f7b89e916852129e996b0141abcc2e1af554d4/src/react/useBaseQuery.ts#L111-L121

As a result we're guaranteed that in the suspense case, the thing that the throw result.error is what actually throws the error to the error boundary in BOTH the useErrorBoundary and suspense cases.
https://github.com/tannerlinsley/react-query/blob/21f7b89e916852129e996b0141abcc2e1af554d4/src/react/useBaseQuery.ts#L125-L131

So it's entirely possible to chose not to throw an error, even when using suspense.

That said handling { suspense: true, useErrorBoundary: false } the same as { suspense: true, useErrorBoundary: () => false } would probably be a breaking change.

[TkDodo]

thank you, this is great work - my comments are mostly stylistic changes 😅 , but please see the last comment 👍

[tannerlinsley]

I think the chances are rare that it will be a breaking change, and if it is, it can easily be fixed. This is one of those confusing moments in semver… I personally do not think this warrants a major version. If anyone does, then they’re going to be waiting a very long time to get the major release.

Push it.

[dantman]

I've updated shouldThrowError so a useErrorBoundary boolean will override suspense's error throwing behaviour.

[TkDodo]

Looks really good already, thank you 🚀

[TkDodo]

awesome, thank you 🙌

[tannerlinsley]

🎉 This PR is included in version 3.23.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀