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

(v3) next.js serverless build run into timeout #1432

Closed
amuttsch opened this issue Dec 15, 2020 · 15 comments
Closed

(v3) next.js serverless build run into timeout #1432

amuttsch opened this issue Dec 15, 2020 · 15 comments

Comments

@amuttsch
Copy link
Contributor

Describe the bug
When building a next.js application with a serverless target and deploying it on netlify using next-on-netlify results into the page not rendering and running into a timeout. This problem seems to be related to v3, using v2 the app works as intended.

To Reproduce

Demo: https://react-query-v3-timeout.netlify.app/
Code: https://github.com/frigus02/test-react-query-prefetch-ssr

Steps to reproduce the behavior:

  1. yarn install
  2. yarn run build
  3. yarn run next-on-netlify
  4. yarn run netlify deploy -s <your_site_id> -d out_publish -f out_functions --prod

Expected behavior

It should render the page correctly without running into a timeout.

Additional context

The timeout issue usually results from a promise or some other long running task not finishing. The serverless functions are deployed on a lambda and only return data if the function returns.

@frigus02
Copy link

We investigated a bit further. You can now test this without next-on-netlify.

  1. git clone https://github.com/frigus02/test-react-query-prefetch-ssr
  2. cd test-react-query-prefetch-ssr
  3. yarn install
  4. yarn build
  5. yarn test:lambda

You can see it render the page pretty quickly. But the process never terminates.

In React Query v2 this was working fine. Try using:

  1. git clone https://github.com/frigus02/test-react-query-prefetch-ssr
  2. cd test-react-query-prefetch-ssr
  3. git checkout v2
  4. yarn install
  5. yarn build
  6. yarn test:lambda

You can see it render the page and then terminate immediately.

@frigus02
Copy link

We traced this down to the GC timeout: https://github.com/tannerlinsley/react-query/blob/ac342e237ae9fab6759f2d7be616662da6c16225/src/core/query.ts#L169

First we added queryClient.clear() after every useQuery or fetchQuery. This solved the problem.

We then realized this might be related to the GC. So we set cacheTime very low (100ms) in all queries, which also solved the issue. The process terminates after rendering the page.

@boschni
Copy link
Collaborator

boschni commented Dec 15, 2020

So basically the script does not terminate because the GC timeout is still running in the background? I can imagine this would be pretty hard to track down for most people. Not sure what a good default behavior would be though, as some users also want to use the caching functionality on the server as singleton. Either we need to change this behavior or make it explicit in the docs.

@frigus02
Copy link

Yeah. For us a workaround could be to set the cacheTime to 0 or -1 on the server. E.g.

const queryClient = new QueryClient({
  defaultOptions: {
    queries: {
      cacheTime: typeof window === "undefined" ? -1 : 5 * 60 * 1000,
    },
  },
});

I agree with you, though. It would be nice to make this explicit somehow.

@amuttsch
Copy link
Contributor Author

How was this implemented previously? IIRC the cacheTime was available previously. Solutions I could think of are providing an option to disable GC or use timestamps on cache. If the query is triggered again it is evaluated on the fly if it is still valid or not.

@boschni
Copy link
Collaborator

boschni commented Dec 15, 2020

Previously GC was disabled when running on a server (which means you also couldn't use the functionality). Would running queryClient.clear() after rendering the page be an option? Or indeed a flag to disable the GC timers.

@frigus02
Copy link

frigus02 commented Dec 15, 2020

I don't know at which point I would have to call queryClient.clear() with a Next.js app on a serverless platform. I couldn't find the right place to do that, because as far as I know we don't have access to the server. But I'm also still quite new to Next.js. So I might just haven't come across this, yet.

If you have control over the server, that could be a good solution.

@frigus02
Copy link

frigus02 commented Dec 15, 2020

Update: setting cacheTime to -1 on the server leads to another problem. For some reason queryClient.fetchQuery always rejects with a CancelledError { revert: undefined, silent: undefined }.

Reproducable on branch cancelled-error on https://github.com/frigus02/test-react-query-prefetch-ssr/tree/cancelled-error

Thinking about this, would you prefer a separate issue for the CancelledError?

@frigus02
Copy link

We played with different options:

  • Set cacheTime on server to -1.

    👉 Running queryClient.fetchQuery on server rejects with CancelledError.

  • Call queryClient.clear() at the end of getServerSideProps/getInitialProps.

    👉 Doesn't really help on it's own because the rendering happens afterwards, where components call useQuery.

  • Combine those two options: set cacheTime on server to -1 only for the QueryClient used by useQuery and call queryClient.clear() at the end of getServerSideProps/getInitialProps for the server side prefetching of data.

    👉 The options used for prefetching take priority here. Hydration always uses the cacheTime from the dehydrated state: https://github.com/tannerlinsley/react-query/blob/ac342e237ae9fab6759f2d7be616662da6c16225/src/hydration/hydration.ts#L170

It seems like running queryClient.clear() after rendering is the only possible solution. It's unclear to me how to do that with Next.js and the serverless target, though. Or a flag to disable caching on the server.

@boschni
Copy link
Collaborator

boschni commented Dec 16, 2020

Created a fix for the cancelled queries when using low cache times and also removed the cache time hydration. In the meanwhile, you should be able to disable the GC with cacheTime: Infinity.

@frigus02
Copy link

Thanks for fixing this so quickly. We just tested with version 3.3.0. It works for us with the following setup:

  • cacheTime: Infinity for the temporary QueryClient inside getInitialProps/getServerSideProps
  • cacheTime: typeof window === "undefined" ? 0 : 5 * 60 * 1000 for the QueryClient used with the <QueryClientProvider/>

Is this worth mentioning in the SSR docs?

@tannerlinsley
Copy link
Collaborator

I really would like to get this to a better place that feels more automatic so nothing extra is necessary. @boschni Do you think that's feasible?

@boschni
Copy link
Collaborator

boschni commented Dec 16, 2020

Yeah I guess it's a bit difficult because in some SSR scenarios you want to have GC running, but in case of Lambda deployments you don't. A flag to disable GC could be added though, then at least you don't need to juggle with the cache times?

@tannerlinsley
Copy link
Collaborator

That could be nice. And easily documented. “If you are in a long running server, do nothing. If you are in a short lived server, do this.”

@vishalvijay
Copy link

Any update on this, are planning to add the flag?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants