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

Race condition for similar queries, where the merge isn't reached #10359

Closed
tim-rellify opened this issue Dec 12, 2022 · 11 comments
Closed

Race condition for similar queries, where the merge isn't reached #10359

tim-rellify opened this issue Dec 12, 2022 · 11 comments

Comments

@tim-rellify
Copy link

Intended outcome:
I'm working on a react application with several components, that execute different queries for the same type but with different fields in parallel (on load of the whole react application).
To merge the different fields of the different query responses for the same type, "merge: true" is used as type policy for the affected type when initializing the apollo in memory cache.

A simple example:

type Article {
id
name
url
content
}

And I want to implement a NameComponent, UrlComponent and ContentComponent in react. Each of these components requests the corresponding field in a separate query with the help of a watch query hook.

After all components are loaded and all hooks return a data value, the Apollo cache should contain the article with all requested fields.

{
id: 123
name: 'My article'
url: 'github.com'
content: 'Cool article about github'
}

Actual outcome:
Generally, this runs fine. But sometimes, the response of one of the queries is missing in the cache.

How to reproduce the issue:
I couldn't reproduce the above described issue in very simple use cases. In my use case, I run about 5 different queries for the same type along with other queries for other types. But I observed the following:

  • In the browser's network tab, you can see, that all queries were fired and return a response back
  • You can even log / debug the response in a custom Apollo link. So the response is finding its way to the Apollo client
  • In a custom merge function as a type policy, you can log / debug, that this merge function is only called for queries, that also end up later in the cache. If the issue doesn't occur, all query results end up there and also correctly in the cache. So generally it seems to be working, but sometimes not
  • So the issue must be somewhere between handling the query data inside the Apollo client and the merge function

I also thought, that this might be related to #9502, but that didn't have any effect after I applied the changes there.

Versions
System:
OS: Windows 10 10.0.22000
Binaries:
Node: 18.12.1 - C:\Program Files\nodejs\node.EXE
Yarn: 3.3.0 - ~\Documents\GitHub\myProject\node_modules.bin\yarn.CMD
npm: 8.19.2 - C:\Program Files\nodejs\npm.CMD
Browsers:
Edge: Spartan (44.22000.120.0), Chromium (108.0.1462.42)
npmPackages:
@apollo/client: ^3.7.2 => 3.7.2

@bignimbus
Copy link
Contributor

Hi @tim-rellify 👋🏻 thanks so much for opening this issue! As you can imagine, verifying this particular behavior may be quite time-consuming for the team so it's not likely we'll be able to take a look at this in the near future. If we had a runnable reproduction, though, it may help expedite a resolution.

I had one question:

In a custom merge function as a type policy, you can log / debug, that this merge function is only called for queries, that also end up later in the cache. If the issue doesn't occur, all query results end up there and also correctly in the cache. So generally it seems to be working, but sometimes not

Can you elaborate on that, preferably with the merge function code that you're using?

@tim-rellify
Copy link
Author

tim-rellify commented Dec 12, 2022

Hi @tim-rellify 👋🏻 thanks so much for opening this issue! As you can imagine, verifying this particular behavior may be quite time-consuming for the team so it's not likely we'll be able to take a look at this in the near future. If we had a runnable reproduction, though, it may help expedite a resolution.

I had one question:

In a custom merge function as a type policy, you can log / debug, that this merge function is only called for queries, that also end up later in the cache. If the issue doesn't occur, all query results end up there and also correctly in the cache. So generally it seems to be working, but sometimes not

Can you elaborate on that, preferably with the merge function code that you're using?

Sure. And thanks for the response! :-)
But I just added a custom merge function just to log the cases, where a merge doesn't happen.
Is there any direction, where I could have a closer look on? In the Apollo client code. So that I could probably find the issue.
Is there something like a queue or a single point, where the responses come together?

@tim-rellify
Copy link
Author

I could workaround the issue by calling the "reobserve" function of the watch query in a useEffect, if data is undefined and the watch query was called and loaded.

After debugging the apollo client itself, I could observe, that the internal merge functionality seems to be processed correctly.

@bignimbus bignimbus added 🏓 awaiting-team-response requires input from the apollo team and removed 🏓 awaiting-contributor-response requires input from a contributor labels Dec 14, 2022
@hakonxxx
Copy link

any updates? found same issue in same condition: multi different queries inflight which with different DocumentNode but same schema, this schema have individual cache(type policy), sometimes the cache not update after all queries completed.
for example a schema:
definition:

type Test  = {
  a: Record<string, string> | null
  b: Record<string, string> | null
  c: Record<string, string> | null
}

const documentA = gql`
query test() {
  a
  b
}
`
const documentB = gql`
query test() {
  a
  c
}
`

query results:

// query a
{
  a: {}
  b: {}
}
// query b
{
  a: {}
  c: {}
}

origin cache:

{
  a: null
  b: {}
}

cache after queries complete:

// nothing changed
{
  a: null
  b: {}
}

@tim-rellify
Copy link
Author

any updates? found same issue in same condition: multi different queries inflight which with different DocumentNode but same schema, this schema have individual cache(type policy), sometimes the cache not update after all queries completed. for example a schema: definition:

type Test  = {
  a: Record<string, string> | null
  b: Record<string, string> | null
  c: Record<string, string> | null
}

const documentA = gql`
query test() {
  a
  b
}
`
const documentB = gql`
query test() {
  a
  c
}
`

query results:

// query a
{
  a: {}
  b: {}
}
// query b
{
  a: {}
  c: {}
}

origin cache:

{
  a: null
  b: {}
}

cache after queries complete:

// nothing changed
{
  a: null
  b: {}
}

Were these the only requests for your type "Test"? If no, did you set { merge: true } for the type policies for your type "Test" when initializing the InMemoryCache?
E.g.:

const cache = new InMemoryCache({
  typePolicies: {
    Test: {
      merge: true
    }
  }
});

Because otherwise, another request would overwrite the response of another. By default, these aren't merged!

@hakonxxx
Copy link

any updates? found same issue in same condition: multi different queries inflight which with different DocumentNode but same schema, this schema have individual cache(type policy), sometimes the cache not update after all queries completed. for example a schema: definition:

type Test  = {
  a: Record<string, string> | null
  b: Record<string, string> | null
  c: Record<string, string> | null
}

const documentA = gql`
query test() {
  a
  b
}
`
const documentB = gql`
query test() {
  a
  c
}
`

query results:

// query a
{
  a: {}
  b: {}
}
// query b
{
  a: {}
  c: {}
}

origin cache:

{
  a: null
  b: {}
}

cache after queries complete:

// nothing changed
{
  a: null
  b: {}
}

Were these the only requests for your type "Test"? If no, did you set { merge: true } for the type policies for your type "Test" when initializing the InMemoryCache? E.g.:

const cache = new InMemoryCache({
  typePolicies: {
    Test: {
      merge: true
    }
  }
});

Because otherwise, another request would overwrite the response of another. By default, these aren't merged!

thanks for reply~~ first question is yes, for the type policy, i have tried two types

  1. no merge func, the updated cache should be query a or b result, but it`s the old cache.
  2. default mergeObjects with console.log, there is no merge function logs when the issue happen
merge(existing, incoming, opts) {
          // this is business logic to distinguish two queries(just like query a/b modify same cache) by variables
          console.log(
            (opts.variables?.filter ? 'activity' : 'collection') +
              ' merge-----------------'
          );
          console.log(existing, incoming, '-----------------');
          return opts.mergeObjects(existing, incoming);
}

@hakonxxx
Copy link

OMG!!!!! i have two queries, very simple:

const Page = () => {
  useA(); // same __typename, different documentNode
  useB(); // same __typename, different documentNode
  return null
}

and console the merge function call, there are three behaviors when first loaded:

  1. only a merged
  2. only b merged
  3. both a and b merged

maybe is the cache system bug(the __typename schema had a individual type policy)

@hakonxxx
Copy link

see more in #10587 , found the root cause

@tim-rellify
Copy link
Author

@phryneas This ticket can be closed with #10587. I looked into the replay there and it looks like what I observed here.

@tim-rellify
Copy link
Author

After some minor tests and removing the workaround, this seems to be solved with #10587 / version 3.7.11. Thanks to everyone, involved here!

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants