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

ApolloQueryResult returning partial data in @apollo/client@^3.6.9 with nextFetchPolicy: 'cache-only' #9923

Open
KeithGillette opened this issue Jul 20, 2022 · 8 comments

Comments

@KeithGillette
Copy link
Contributor

KeithGillette commented Jul 20, 2022

Intended outcome:
The behavior of ApolloQueryResult and nextFetchPolicy: 'cache-only' changed in @apollo/client@3.6.9. Prior to 3.6.9, the data returned in the ObservableQuery.result() Promise resolution or Apollo Angular's QueryRef.valueChanges Observable would conform to the shape of the underlying query and we would not see results with partial: true.

Actual outcome:
Upon updating to @apollo/client@3.6.9, we now are getting data with missing fields and partial: true with no changes to our codebase.

How to reproduce the issue:
Compare resolution of ApolloQueryResult between 3.6.8 and 3.6.9 when multiple queries with different shapes but the same variables are triggered.

Versions
System:
OS: macOS 12.4
Binaries:
Node: 14.19.1 - ~/.nvm/versions/node/v14.19.1/bin/node
npm: 6.14.16 - ~/.nvm/versions/node/v14.19.1/bin/npm
npmPackages:
@apollo/client: 3.6.9 => 3.6.9
apollo-angular: 3.0.1 => 3.0.1
apollo-server-core: 3.10.0 => 3.10.0
apollo-server-express: 3.10.0 => 3.10.0

@hwillson
Copy link
Member

Thanks for this @KeithGillette - any chance you'd be able to provide a small runnable reproduction that shows this?

@KeithGillette
Copy link
Contributor Author

Thanks for your attention to this bug report, @hwillson. I've looked at the Apollo Client error template projects but I'm unfamiliar with React so a bit stymied on where to start with a simple reproduction, as the issue came out of our rather complex Apollo-Angular project when we noticed a behavior change in ObservableQuery.result() with only an update from @apollo/client@3.6.8 to @apollo/client@3.6.9. @kamilkisiela, is there an Apollo Angular error template or super simple sample project I could try to create a reproduction from?

@KeithGillette KeithGillette changed the title ObservableQuery.result() returning partial data in @apollo/client@3.6.9 ObservableQuery.result() returning partial data in @apollo/client@3.6.9 with nextFetchPolicy: 'cache-only' Oct 19, 2022
@KeithGillette KeithGillette changed the title ObservableQuery.result() returning partial data in @apollo/client@3.6.9 with nextFetchPolicy: 'cache-only' ApolloQueryResult returning partial data in @apollo/client@3.6.9 with nextFetchPolicy: 'cache-only' Oct 19, 2022
@KeithGillette
Copy link
Contributor Author

KeithGillette commented Oct 19, 2022

I figured out how to create a simple Apollo-Angular StackBlitz with local schema and ApolloLink resolvers but have thus far been able to reproduce the issue in a shareable project.

I did discover, however, that the return of partial data starting in @apollo/client@3.6.9 occurs specifically with nextFetchPolicy: 'cache-only', which we have set in our project to prevent refetching queries on cache.evict() while still getting cache updates from other requests/explicit refetches. In addition, I found partial data in normal ObservableQuery (Apollo Angular QueryRef.valueChanges Observable ApolloQueryResult) emissions, not just those from ObservableQuery.result() which I had first noted. I updated the issue title and description to reflect these two new findings.

The problem seems to be restricted to queries on types which have field parameters on multiple array fields similar to the simplified example below. Our application has multiple queries on the same parent document, some of which return only the non-Array properties, some only the _id and one array property, some only the _id and another array property. These queries are being sent over the network and the data returned includes the correct fields but the local ObservableQueries are sometimes emitting partial results to their respective consumers, specifically just objects with _id and __typename fields. Perhaps we have multiple queries in flight simultaneously with requests for the same Parent but different fields and the local ObservableQueries are being fulfilled from the result of a differently shaped query on the same Parent _id? I'm not sure why that would happen with the default partial: false setting but that's as close as I've been able to narrow the problematic new behavior in 3.6.9 down to so far.

type Parent {
  _id: ID!
  name: String!
  childList1 (someParameter: String!: [Child!]
  childList2 (someParameter: String!): [Child!]
}

query ParentRead($id: ID!, $list1Filter: String) {
  Parent(id: $id) {
    id
    name
  }
} 

query ParentReadList1($id: ID!, $list1Filter: String) {
  Parent(id: $id) {
    id
    name
    childList1(filter: $list1Filter) {
      id
      name
    }
  }
}

query ParentReadList2($id: ID!, $list1Filter: String) {
  Parent(id: $id) {
    id
    name
    childList2(filter: $list1Filter) {
      id
      name
    }
  }
}

Any pointers on troubleshooting to isolate further appreciated.

@KeithGillette
Copy link
Contributor Author

KeithGillette commented Dec 30, 2022

This issue persists through @apollo/client@3.7.4.

@KeithGillette KeithGillette changed the title ApolloQueryResult returning partial data in @apollo/client@3.6.9 with nextFetchPolicy: 'cache-only' ApolloQueryResult returning partial data in @apollo/client@^3.6.9 with nextFetchPolicy: 'cache-only' Jan 6, 2023
@KeithGillette
Copy link
Contributor Author

KeithGillette commented Jan 14, 2023

I have isolated the source of this issue to the changes in release 3.6.9 in QueryManager.fetchQueryObservable starting on line 1125. Restoring fromVariables to return this.fetchQueryByPolicy<TData, TVars>(queryInfo, normalized, networkStatus); eliminates the issue of returning partial data in the queries of the form specified in my previous post (updated with example queries on the type). I verified stepping through calls to QueryManager that the new code queryInfo.observableQuery["applyNextFetchPolicy"]("after-fetch", options); is being called for the queries in question but am unsure why this may result in incorrectly returning partial data. Any suggestions for further troubleshooting appreciated, @hwillson.

@KeithGillette
Copy link
Contributor Author

Despite including some fixes for incorrectly returning partial data, this issue persists in @apollo/client@3.7.5. Again, any help in further troubleshooting appreciated, @hwillson. I have isolated the code change that generated this issue but am not conversant enough in the logic understand why it returns partial data in our project or to propose a viable fix.

@jerelmiller
Copy link
Member

Hey @KeithGillette 👋 !

Thanks for your patience and thanks for all the detail! This looks very similar to #10222 and I'm willing to bet both issues suffer from the same root cause. I did a lot of exploration through this part of the code base when trying to fix that issue (#10239). Unfortunately I ran into a ton of challenges with my initial solution.

One of the behaviors introduced in 3.6.9 was that nextFetchPolicy is now set synchronously when a fetch is performed. Why does this matter? Its because ObservableQuery.getCurrentResult() depends on the fetchPolicy to determine what code path to take when building up the result retuned from that function. If nextFetchPolicy is a policy that reads from the cache, and ObservableQuery.getCurrentResult() is called right after a query is kicked off, its going to try and read a result from the cache. This is important to note because this function is used in places like useQuery to get the result returned from the hook. In fact, I think this statement of yours strengthens the idea that this synchronous update is causing issues:

I verified stepping through calls to QueryManager that the new code queryInfo.observableQuery["applyNextFetchPolicy"]("after-fetch", options); is being called for the queries in question but am unsure why this may result in incorrectly returning partial data.

This is definitely a very tricky part of the code base because there are so many moving parts all working together at the same time. I'd like to revisit that PR soon and come up with an alternative approach. I'm hoping that by fixing #10222, I can also fix this issue as well. I'll make sure to write an additional test for this specific case to make sure this is checked.

Thanks again for the help trying to narrow this down!

@KeithGillette
Copy link
Contributor Author

While the underlying issue is still present as evidenced by console errors, reliability of data availability for components does appear improved in release 3.7.11. Any updates on your pending fix, @jerelmiller?

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

No branches or pull requests

4 participants