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

RangeError: Maximum call stack size exceeded when using persistedExchange in front of ssrExchange #3441

Closed
3 tasks done
Warxcell opened this issue Dec 1, 2023 · 7 comments · Fixed by #3442
Closed
3 tasks done
Labels
documentation 📖 This needs to be documented but won't need code changes

Comments

@Warxcell
Copy link

Warxcell commented Dec 1, 2023

Describe the bug

We have following setup of exchanges

persistedExchange(),
ssrExchange,
fetchExchange,

Which leads to

file:/node_modules/wonka/dist/wonka.mjs:246
    i(start((e => {
             ^

RangeError: Maximum call stack size exceeded
    at file:///node_modules/wonka/dist/wonka.mjs:246:14
    at file:///node_modules/wonka/dist/wonka.mjs:442:9
    at Array.<anonymous> (file://node_modules/wonka/dist/wonka.mjs:179:9)
    at file://node_modules/wonka/dist/wonka.mjs:426:17
    at Array.<anonymous> (file://node_modules/wonka/dist/wonka.mjs:235:15)
    at file://node_modules/wonka/dist/wonka.mjs:426:17
    at next (file:///node_modules/wonka/dist/wonka.mjs:974:11)
    at Object.next (file://node_modules/wonka/dist/wonka.mjs:1004:9)
    at file:///node_modules/@urql/exchange-persisted/dist/urql-exchange-persisted.mjs:90:13
    at Array.<anonymous> (file:///node_modules/wonka/dist/wonka.mjs:192:14)

if non-persisted query is executed.

if I exchange ssr in front of persisted Query like that

ssrExchange,
persistedExchange(),
fetchExchange,

it works - but it also re-executes queries after hydration on client-side.

Reproduction

https://github.com/Warxcell/urql-ssr-persisted-exchange-demo

Urql version

"@urql/core@>=4.1.0", "@urql/core@^4.0.0":
version "4.2.0"
"@urql/vue@1.1.2":
version "1.1.2"
"@urql/exchange-persisted@^4.1.0":
version "4.1.0"

Validations

  • I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • Read the docs.
  • Follow our Code of Conduct
@JoviDeCroock
Copy link
Collaborator

JoviDeCroock commented Dec 1, 2023

That feels correct, the fetchExchange is an async exchange while the ssr is meant to be synchronous so the order is [...synchronous, ...async]. In the current order there would also never be an ssr-result saved to the ssrExchange. Why do you need two fetchExchanges?

@Warxcell
Copy link
Author

Warxcell commented Dec 1, 2023

That feels correct, the fetchExchange is an async exchange while the ssr is meant to be synchronous so the order is [...synchronous, ...async]. In the current order there would also never be an ssr-result saved to the ssrExchange. Why do you need two fetchExchanges?

according to docs, persistedExchange DOES NOT replaces fetchExchange - so no, there are NOT 2 fetch exchanges.

@kitten
Copy link
Member

kitten commented Dec 1, 2023

The reproduction here doesn't reproduce the issue.

However, to explain what's going on. For this to become a problem a couple of things need to be the case. Mainly, the exchange(s) after the persistedExchange must be:

  • replying synchronously; and
  • must always respond with PersistedQueryNotFound
    If both of these are true then you'll get a RangeError as the persistedExchange will never terminate. We can add a protection for that, but just pointing out that this is likely usage error.

Unrelatedly, some notes on the reproduction, which seems to behave fine in its current state:

  • there's no cache, so I can't guarantee much in terms of client-side non-duplicate requests. Please note that the ssrExchange IS NOT a cache. While uncached usage of urql should work there are some caveats, so I'd consider this an advanced use-case.
  • the example hydrates the string 'urql' into initial state, so do take care that this doesn't actually get misused or misparsed.
  • it doesn't actually hook into Nuxt's lifecycles, uses the SSR state, or uses await useQuery as far as I can tell. We have some threads on setup for this in issues/discussions, I believe, but overall, I'm not well versed in Nuxt so I can't check against that. So, just a note that this looks off

Overall, while it shouldn't matter whether you place persistedExchange in front of or after ssrExchange, since they're both synchronous. The reversal doesn't matter and neither will lead to duplicate requests per se.

I suspect what you might have seen is an Automatic Persisted Query miss, hence the request repeating? As long as the operation.key matches, the ssrExchange will be used.
However, the ssrExchange will only make its results available to the Client once on the client-side, since it's not a cache but just a "replay exchange" for prior results from the server-side. So, if for any reason you repeat the query (e.g. client navigation / updates, or ssrExchange hydrating a PersistedQueryNotFound error) then the ssrExchange will not respond for the given operation.key either way.

I hope that clarifies this, but for now I'll mark this issue as “needs more info” ✌️
However, I don't currently see anything that looks off, except for potentially no specific error message being available for when the persistedExchange might receive PersistedQueryNotFound continuously 🤔


Edit: I've just seen that you meant the server-side log contains the mentioned error. My bad! They were hidden from me, so I didn't check or notice them, since the client served just fine.

I just realised what's going on here,
I've mentioned it above.

I suspect what you might have seen is an Automatic Persisted Query miss, hence the request repeating? As long as the operation.key matches, the ssrExchange will be used.
However, the ssrExchange will only make its results available to the Client once on the client-side, since it's not a cache but just a "replay exchange" for prior results from the server-side. So, if for any reason you repeat the query (e.g. client navigation / updates, or ssrExchange hydrating a PersistedQueryNotFound error) then the ssrExchange will not respond for the given operation.key either way.

The ssrExchange will still replay results on the server-side. Hence the PersistedQueryNotFound error is replayed.

The client still renders likely since the rest of the setup isn't actually handling SSR for the client-side, so basically, it's as if you're rendering HTML in Nuxt SSR mode and then discarding the result and building a completely independent result on the client-side.

Long story short, the stacktraced error in the Node process (i.e. server-side) can be completely explained by the above, the other issues you're describing are basically expected behaviour given the setup (re. the prior list I provided)
So, I'll close this out and will look into whether we can at least make this clearer and issue a warning when a loop occurs instead

@kitten kitten added the needs more info ✋ A question or report that needs more info to be addressable label Dec 1, 2023
@Warxcell
Copy link
Author

Warxcell commented Dec 1, 2023

isn't the idea of ssr exchange - when client hydrates - to NOT execute queries again, because they already have data sent from server?

If I remove persisted exchange and leave ssrExchange - client does NOT executes queries.

@kitten
Copy link
Member

kitten commented Dec 1, 2023

See edit; I missed what's going on on the server-side, and that explains the error there, while the rest can be explained by missing setup code, as long as the reproduction is basically what's being run in your case too.

isn't the idea of ssr exchange - when client hydrates - to NOT execute queries again, because they already have data sent from server?

Not necessarily, no. The ssrExchange has a different priority. Its priority is to make sure that, as long as it's hydrated on the client-side correctly, the render/hydration that the client-side performs uses the same data that the server used.

This is a subtly different goal. After hydration, we don't expect it to act like a cache and that's why it only returns a result once. It basically replaces the fetchExchange for results that the server has already used/seen once.

The problem with persistedExchange is that, as long as your persistedExchange is used in front of the ssrExchange the ssrExchange “sees” your persisted errors. Meaning, it will treat them as the canonical result. This then means that we allow this result to be rendered server-side.

The ssrExchange is basically a single-time “snapshot” of what results your server uses to render the SSR result.

Edit 2: You're likely aware of this, but just so I address this too, since I forgot. If you add random data to the query, its key changes on every render, i.e. it'll also be different between client-side and server-side and a match can never occur between the two

@kitten kitten added documentation 📖 This needs to be documented but won't need code changes and removed needs more info ✋ A question or report that needs more info to be addressable labels Dec 1, 2023
@Warxcell
Copy link
Author

Warxcell commented Dec 1, 2023

Edit 2: You're likely aware of this, but just so I address this too, since I forgot. If you add random data to the query, its key changes on every render, i.e. it'll also be different between client-side and server-side and a match can never occur between the two

Yeah, that was only to reproduce nodejs error.

I've tested now and indeed it re-executes query on client-side if there is only ssrExchange, however it does not when you have ssrExchange and then persistedExchange. If I remove persistedExchange - it always re-execute query, even if there is normalized cache. I always though that ssrExchange will prevent client re-execution.

In that case - what's the best way to prevent client-side to execute queries that SSR already executed ?

@kitten
Copy link
Member

kitten commented Dec 1, 2023

I'm adding a dev-time warning to this, since it's a very specific case that's clearly just confusing and hard to grasp given the error.

If I remove persistedExchange - it always re-execute query, even if there is normalized cache. I always though that ssrExchange will prevent client re-execution.

There's a short explanation here: https://formidable.com/open-source/urql/docs/advanced/persistence-and-uploads/#automatic-persisted-queries

tl;dr: Run this just as an SPA without SSR to see more clearly what's going on. You may have to check closely what the request looks like. Basically, this is because you're using automatic persisted queries. A second request will be sent when the hash that's sent is unknown to the server.

The longer explanation is in the docs ✌️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation 📖 This needs to be documented but won't need code changes
Projects
None yet
3 participants