blitz-js / legacy-framework

MIT License
2 stars 2 forks source link

Query inside a custom ErrorBoundary is executed multiple times, before the fallback is rendered #30

Open lksnmnn opened 3 years ago

lksnmnn commented 3 years ago

What is the problem?

A ChildComponent executes a query, which throws a 404 NotFoundError. The ChildComponent lives inside an ErrorBoundary with its own fallback. The query is being retried at least 3 times before the boundary finally renders the fallback and calls its onError callback.

We chatted about this in a discordia help channel: https://discord.com/channels/802917734999523368/814628031322128394/844583897223135303

Expectation: No retries on 404.

Paste all your error logs here:


workbox Router is responding to: / ./workbox-32092201.js:47:24
workbox Network request for '/' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/' ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/static/chunks/webpack.js?ts=1621520427922 ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/static/chunks/main.js?ts=1621520427922 ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/static/chunks/pages/_app.js?ts=1621520427922 ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/static/chunks/pages/index.js?ts=1621520427922 ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/static/chunks/react-refresh.js?ts=1621520427922 ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/static/development/_buildManifest.js?ts=1621520427922 ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/static/development/_ssgManifest.js?ts=1621520427922 ./workbox-32092201.js:47:24
workbox Network request for '/_next/static/chunks/webpack.js?ts=1621520427922' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/static/chunks/webpack.js?ts=1621520427922' ./workbox-32092201.js:47:24
workbox Network request for '/_next/static/chunks/main.js?ts=1621520427922' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/static/chunks/main.js?ts=1621520427922' ./workbox-32092201.js:47:24
workbox Network request for '/_next/static/chunks/pages/_app.js?ts=1621520427922' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/static/chunks/pages/_app.js?ts=1621520427922' ./workbox-32092201.js:47:24
workbox Network request for '/_next/static/chunks/react-refresh.js?ts=1621520427922' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/static/chunks/react-refresh.js?ts=1621520427922' ./workbox-32092201.js:47:24
workbox Network request for '/_next/static/chunks/pages/index.js?ts=1621520427922' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/static/chunks/pages/index.js?ts=1621520427922' ./workbox-32092201.js:47:24
workbox Network request for '/_next/static/development/_buildManifest.js?ts=1621520427922' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/static/development/_buildManifest.js?ts=1621520427922' ./workbox-32092201.js:47:24
workbox Network request for '/_next/static/development/_ssgManifest.js?ts=1621520427922' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/static/development/_ssgManifest.js?ts=1621520427922' ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/static/chunks/0.js ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/webpack-hmr?page=/ ./workbox-32092201.js:47:24
workbox Router is responding to: /logo.png ./workbox-32092201.js:47:24
workbox No route found for: /api/users/queries/myFailingQuery ./workbox-32092201.js:47:24
workbox No route found for: /api/users/queries/getCurrentUser ./workbox-32092201.js:47:24
workbox Router is responding to: https://fonts.googleapis.com/css2?family=Libre+Franklin:wght@300;700&display=swap ./workbox-32092201.js:47:24
workbox Network request for '/_next/static/chunks/0.js' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/static/chunks/0.js' ./workbox-32092201.js:47:24
workbox Router is responding to: /favicon.ico ./workbox-32092201.js:47:24
workbox Network request for '/_next/webpack-hmr?page=/' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/webpack-hmr?page=/' ./workbox-32092201.js:47:24
workbox Network request for '/logo.png' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/logo.png' ./workbox-32092201.js:47:24
workbox Network request for 'https://fonts.googleapis.com/css2?family=Libre+Franklin:wght@300;700&display=swap' returned a response with status '0'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to 'https://fonts.googleapis.com/css2?family=Libre+Franklin:wght@300;700&display=swap' ./workbox-32092201.js:47:24
workbox Network request for '/favicon.ico' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/favicon.ico' ./workbox-32092201.js:47:24
workbox Router is responding to: https://fonts.gstatic.com/s/librefranklin/v7/jizDREVItHgc8qDIbSTKq4XkRiUf2zcZiVbJ.woff2 ./workbox-32092201.js:47:24
Object { name: "NotFoundError", message: "This could not be found", code: undefined, statusCode: 404, meta: undefined, stack: "" }
query.js:317:19
Uncaught 
Object { name: "NotFoundError", message: "This could not be found", code: undefined, statusCode: 404, meta: undefined, stack: "" }
useBaseQuery.js:91
The above error occurred in the <Sadge404> component:

Sadge404@webpack-internal:///./pages/index.tsx:167:81
ErrorBoundary@webpack-internal:///../../node_modules/react-error-boundary/dist/react-error-boundary.umd.js:71:37
main
div
Home@webpack-internal:///./pages/index.tsx:190:111
WithSuperJSON@webpack-internal:///../../node_modules/babel-plugin-superjson-next/dist/tools.js:110:26
BlitzInnerRoot@webpack-internal:///../../node_modules/@blitzjs/core/dist/blitzjs-core.esm.js:697:19
Layout@webpack-internal:///./app/core/layouts/Layout.tsx:13:15
ErrorBoundary@webpack-internal:///../../node_modules/react-error-boundary/dist/react-error-boundary.umd.js:71:37
Suspense
App@webpack-internal:///./pages/_app.tsx:25:19
Hydrate@webpack-internal:///../../node_modules/react-query/es/hydration/react.js:26:18
QueryClientProvider@webpack-internal:///../../node_modules/react-query/es/react/QueryClientProvider.js:37:16
BlitzProvider@webpack-internal:///../../node_modules/@blitzjs/core/dist/blitzjs-core.esm.js:476:16
BlitzOuterRoot@webpack-internal:///../../node_modules/@blitzjs/core/dist/blitzjs-core.esm.js:728:28
ErrorBoundary@webpack-internal:///../../node_modules/@next/react-dev-overlay/lib/internal/ErrorBoundary.js:23:47
ReactDevOverlay@webpack-internal:///../../node_modules/@next/react-dev-overlay/lib/internal/ReactDevOverlay.js:73:20
Container@webpack-internal:///../../node_modules/next/dist/client/index.js:154:20
AppContainer@webpack-internal:///../../node_modules/next/dist/client/index.js:643:18
Root@webpack-internal:///../../node_modules/next/dist/client/index.js:779:19

React will try to recreate this component tree from scratch using the error boundary you provided, ErrorBoundary. react-dom.development.js:21953
The above error occurred in the <Sadge404> component:

Sadge404@webpack-internal:///./pages/index.tsx:167:81
ErrorBoundary@webpack-internal:///../../node_modules/react-error-boundary/dist/react-error-boundary.umd.js:71:37
main
div
Home@webpack-internal:///./pages/index.tsx:190:111
WithSuperJSON@webpack-internal:///../../node_modules/babel-plugin-superjson-next/dist/tools.js:110:26
BlitzInnerRoot@webpack-internal:///../../node_modules/@blitzjs/core/dist/blitzjs-core.esm.js:697:19
Layout@webpack-internal:///./app/core/layouts/Layout.tsx:13:15
ErrorBoundary@webpack-internal:///../../node_modules/react-error-boundary/dist/react-error-boundary.umd.js:71:37
Suspense
App@webpack-internal:///./pages/_app.tsx:25:19
Hydrate@webpack-internal:///../../node_modules/react-query/es/hydration/react.js:26:18
QueryClientProvider@webpack-internal:///../../node_modules/react-query/es/react/QueryClientProvider.js:37:16
BlitzProvider@webpack-internal:///../../node_modules/@blitzjs/core/dist/blitzjs-core.esm.js:476:16
BlitzOuterRoot@webpack-internal:///../../node_modules/@blitzjs/core/dist/blitzjs-core.esm.js:728:28
ErrorBoundary@webpack-internal:///../../node_modules/@next/react-dev-overlay/lib/internal/ErrorBoundary.js:23:47
ReactDevOverlay@webpack-internal:///../../node_modules/@next/react-dev-overlay/lib/internal/ReactDevOverlay.js:73:20
Container@webpack-internal:///../../node_modules/next/dist/client/index.js:154:20
AppContainer@webpack-internal:///../../node_modules/next/dist/client/index.js:643:18
Root@webpack-internal:///../../node_modules/next/dist/client/index.js:779:19

React will try to recreate this component tree from scratch using the error boundary you provided, ErrorBoundary. react-dom.development.js:21953
    React 17
    NextJS 3
    <anonym> React
    NextJS 3
    React 2
    NextJS 3
    <anonym> React
    NextJS 3
    <anonym> index.tsx:4
    <anonym> index.js:895
    NextJS 3
    <anonym> next-dev.js:2
    <anonym> next-dev.js:153
    NextJS 5
Uncaught 
Object { name: "NotFoundError", message: "This could not be found", code: undefined, statusCode: 404, meta: undefined, stack: "" }
useBaseQuery.js:91
workbox Network request for 'https://fonts.gstatic.com/s/librefranklin/v7/jizDREVItHgc8qDIbSTKq4XkRiUf2zcZiVbJ.woff2' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to 'https://fonts.gstatic.com/s/librefranklin/v7/jizDREVItHgc8qDIbSTKq4XkRiUf2zcZiVbJ.woff2' ./workbox-32092201.js:47:24
workbox Router is responding to: https://fonts.gstatic.com/s/librefranklin/v7/jizDREVItHgc8qDIbSTKq4XkRiUf2zcZiVbJ.woff2 ./workbox-32092201.js:47:24
workbox Router is responding to: /_next/static/development/_devPagesManifest.json ./workbox-32092201.js:47:24
workbox Network request for '/_next/static/development/_devPagesManifest.json' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/_next/static/development/_devPagesManifest.json' ./workbox-32092201.js:47:24
workbox Network request for 'https://fonts.gstatic.com/s/librefranklin/v7/jizDREVItHgc8qDIbSTKq4XkRiUf2zcZiVbJ.woff2' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to 'https://fonts.gstatic.com/s/librefranklin/v7/jizDREVItHgc8qDIbSTKq4XkRiUf2zcZiVbJ.woff2' ./workbox-32092201.js:47:24
workbox Router is responding to: /favicon.ico ./workbox-32092201.js:47:24
workbox Network request for '/favicon.ico' returned a response with status '200'. ./workbox-32092201.js:47:24
workbox Using NetworkOnly to respond to '/favicon.ico' ./workbox-32092201.js:47:24

Paste all relevant code snippets here:

index.tsx

const Sadge404 = () => {
  const [data] = useQuery(myFailingQuery, null)

  return <p>My nice template</p>
}

const Home: BlitzPage = () => {
  const { reset } = useQueryErrorResetBoundary()
  return (
    <div className="container">
      <main>
        <ErrorBoundary fallback={<p>My dank fallback</p>} onReset={reset}>
          <Sadge404 />
        </ErrorBoundary>
      </main>
    </div>
  )
}
//... etc

Query:

import { Ctx, NotFoundError } from "blitz"

export default async function myFailingQuery(_ = null, { session }: Ctx) {
  throw new NotFoundError()
  return "I never can reach my full potential"
}

What are detailed steps to reproduce this?

Here is repo (blitz new) with the above query and components added to index.tsx, which reproduces the issue. Just install and run blitz dev

https://github.com/lksnmnn/blitz-404-error-boundary-issue

Run blitz -v and paste the output here:

macOS Catalina | darwin-x64 | Node: v12.18.0

blitz: 0.35.0 (global)
blitz: 0.35.0 (local)

  Package manager: npm 
  System:
    OS: macOS 10.15.7
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 24.12 GB / 64.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 12.18.0 - ~/.nvm/versions/node/v12.18.0/bin/node
    Yarn: Not Found
    npm: 6.14.11 - ~/.nvm/versions/node/v12.18.0/bin/npm
    Watchman: Not Found
  npmPackages:
    @prisma/client: ~2.20 => 2.20.1 
    blitz: 0.35.0 => 0.35.0 
    prisma: ~2.20 => 2.20.1 
    react: 0.0.0-experimental-6a589ad71 => 0.0.0-experimental-6a589ad71 
    react-dom: 0.0.0-experiment

This does also occure with node 14 and npm 7.

Please include below any other applicable logs and screenshots that show your problem:

No response

flybayer commented 3 years ago

Thanks!

For whoever wants to look into this, this function (https://github.com/blitz-js/blitz/blob/canary/packages/core/src/utils/react-query-utils.ts#L23-L30) should be defining the query retry behavior. But it seems that maybe that function isn't even being used? Idk, but that's the place to start.

lksnmnn commented 3 years ago

~Yes, I added a console log and this retry callback is actually not being called in this case.~ This seems to be an issue in react-query then? I've just skimmed through some issues and it seems that there are more subtle issues with react-query that explain some of the bugs we are currently seeing in blitz.

Edit: when I set suspense to false, it works as expected and runs the shouldRetry callback

Edit 2: I had to re-run yarn dev and now I can see that the retry function is being used and returns false as expected

lksnmnn commented 3 years ago

Hm. The more I look at the code the weirder it gets. I dont think that it actually does retry the fetch request (network tab shows only a single request) but it runs through the useQuery -> useBaseQuery() code multiple times during render and then just throws the error multiple times, everything else seems to work correctly ?!

If suspense is false, then it won't even render the error fallback, if I set useErrorBoundary: true it behaves just like as suspense is true.

flybayer commented 3 years ago

If suspense is false, then it won't even render the error fallback, if I set useErrorBoundary: true it behaves just like as suspense is true.

This is correct behavior

Hm. The more I look at the code the weirder it gets. I don't think that it actually does retry the fetch request (network tab shows only a single request) but it runs through the useQuery -> useBaseQuery() code multiple times during render and then just throws the error multiple times, everything else seems to work correctly ?!

This does seem suspicious 🤔 . If it's a RQ issue, then next step is to make a failing RQ test and open an issue over there with it

cj commented 3 years ago

I have just run into the same/a similar issue. I have a query exactly like this example https://blitzjs.com/docs/use-query but the ErrorBoundary is never triggered and infinitely retries the query.

cj commented 3 years ago

It's throwing an error, but it's just never caught by ErrorBoundary:

[
  {
    code: 'EPC-1400',
    description: 'Invalid or expired Partner Access Token',
    type: 'system',
    resourceId: 'ORIGIN'
  }
]
14:16:29.323 ERROR fetchLoan()
 EpcError
details:
{
  name: 'EpcError',
  code: 'EPC-1400'
}
error stack:
• blitzjs-core-server.cjs.dev.js:356 <anonymous>
    node_modules/@blitzjs/core/server/dist/blitzjs-core-server.cjs.dev.js:356:20

• blitzjs-core-server.cjs.dev.js:1606 <anonymous>
    node_modules/@blitzjs/core/server/dist/blitzjs-core-server.cjs.dev.js:1606:22

• blitzjs-core-server.cjs.dev.js:81 handleRequestWithMiddleware
    node_modules/@blitzjs/core/server/dist/blitzjs-core-server.cjs.dev.js:81:5

• api-utils.ts:84 apiResolver
    node_modules/next/next-server/server/api-utils.ts:84:5

• next-server.ts:1160 handleApiRequest
    node_modules/next/next-server/server/next-server.ts:1160:5

• next-server.ts:1013 fn
    node_modules/next/next-server/server/next-server.ts:1013:27

• router.ts:346 execute
    node_modules/next/next-server/server/router.ts:346:24

• next-server.ts:1258 run
    node_modules/next/next-server/server/next-server.ts:1258:23

• next-server.ts:568 handleRequest
    node_modules/next/next-server/server/next-server.ts:568:14
flybayer commented 3 years ago

@cj can you try this and let me know if it works or not? Add this code to _app.tsx

import {EpcError} from 'where-ever-this-comes-from'
import SuperJson from "superjson"

SuperJson.registerClass(EpcError, {
  identifier: "EpcError",
  allowProps: ["name", "message", "code", "statusCode", "meta"],
})
cj commented 3 years ago

@flybayer let me give that a try. Does SuperJson need to be in _app? I currently have https://gist.github.com/cj/073baeb3eb07257e7f27a6df0c90419b

flybayer commented 3 years ago

Ah yeah, because SuperJson.registerClass must be executed on both client and server. If it's in the resolver, it only executes server side but not client side