Regression in 3.9.5 with cache updates and error responses

[jerelmiller]

From #11579 (comment)

Unfortunately, I was unable to achieve an error regarding the missing field using the codesandbox example, so I reproduced a simpler case with a field merge error in "typePolicies" (index.js:22).

I'm attaching two identical codesandboxes, the only difference being the versions of @apollo/client.

@apollo/client v.3.9.5
https://codesandbox.io/p/sandbox/issue-apollo-client-v-3-9-5-notes-app-vk8jlf

@apollo/client v.3.9.4
https://codesandbox.io/p/sandbox/issue-apollo-client-v-3-9-5-notes-app-forked-d3923z

Steps to reproduce:

1. optional, restart the backend codesandbox if it is needed
2. Visit the sandbox.
3. Add a Todo.
4. Update the added Todo.

Actual behavior (in version 3.9.5 - broken):
After updating the Todo, the label of the Todo is NOT updated.

Expected behavior (in version 3.9.4 - works):
After updating the Todo, the label of the Todo is updated.

P.S. At first glance, it may seem strange why I presented a code example where I intentionally broke cache management functionality. There are two main concerns on my part:

1. Applications running smoothly but likely having inaccuracies in cache configuration (typePolicies) up to version 3.9.4 inclusively worked seamlessly.
2. Given that there was an error in the configuration of field merge settings in "typePolicies", it remains impossible to easily detect the error. Neither the developer console nor the application provides any information about the problem whatsoever.

[iamdylancurran]

Just want to follow up on this, from 3.9.4 to 3.9.5+ we are experiencing updates to the cache failing to apply and it's breaking many of our unit tests.

This only happens if we upgrade from 3.9.4 forward, and the issues seems to match the example you gave in the initial comment. There's no error or explanation given so we have no idea what we're actually meant to fix here, and we can't upgrade as it stands.

[jerelmiller]

Thanks for letting us know @iamdylancurran! I can almost certainly guarantee its related to #11579. This fixed a bug where a partial cache update could be reported to a query that didn't accept partial updates, but seems it may have caused other side effects. No guarantees on a timeline, but we'll see if we can look at this soon.

[jonnyleeharris]

Just want to follow up on this, from 3.9.4 to 3.9.5+ we are experiencing updates to the cache failing to apply and it's breaking many of our unit tests.

This only happens if we upgrade from 3.9.4 forward, and the issues seems to match the example you gave in the initial comment. There's no error or explanation given so we have no idea what we're actually meant to fix here, and we can't upgrade as it stands.

I am having the exact same issue. Some e2e tests started failing - pinned it down to a cache update not occurring.

We do not use returnPartialData or notifyOnNetworkStatus.

We do make heavy use of fragments though, and are using @graphql-codegen to generate our type policies/fragment matchers.

I will report back if i find anything useful :)

[jerelmiller]

Hey @av-k 👋

I've been digging into this a bit more to understand the change that happened. From what I'm seeing, it looks like 3.9.4 works because the partial cache write from one query/mutation causes a network request for the other query which will then fulfill the cache and re-render with the full data set. The change I introduced in 3.9.5 skips this step, so it explains why this broke for you. My learning here is that some partial cache writes are ok even if the query doesn't tolerate it since it will perform a network request to fulfill the whole result in that situation.

I think the proper thing for us to do instead would be to only avoid the partial cache broadcast on queries that had previously errored, but allow it on queries without errors to ensure that network request happens.

That said, would you be willing to provide me with a bit more "real world" reproduction of the bug? While the example you gave me properly demonstrates the issue, I'd argue that the reproduction is a misuse of the merge function since its returning a different data type than the schema allows. I know this was likely only for reproduction purposes, but I'd like to write a unit test for this in our codebase that better represents the actual scenerio that is causing the breakage.

---

@iamdylancurran and @jonnyleeharris does the above sound like the situation you're seeing as well? If either of you have a scenerio that I can use as a test case, that would also be awesome.

[jerelmiller]

Scratch that, I think I was able to get a believable situation for my tests. I've opened #11839 which I believe addresses this issue.

@av-k @iamdylancurran @jonnyleeharris would you be able to try the following snapshot release to see if it addresses your issue?

npm i @apollo/[email protected]

@av-k I've tried that snapshot release on your reproduction and it seems to fix the issue: https://codesandbox.io/p/sandbox/issue-apollo-client-v-3-9-5-notes-app-forked-qcn3ct?file=%2Fpackage.json%3A10%2C25

[av-k]

Hi @jerelmiller, thank you for the update!

Firstly, I reverted to my source code at the point when I initially encountered the incorrect behavior with @apollo/client versions 3.9.5 and 3.10.3. I then took several steps within my app to ensure that the issue was reproducible with the current versions.

Next, I installed npm i @apollo/[email protected] and repeated the steps within my app to confirm whether the issue no more persisted with this specific version.

Following these steps, I can confirm that the issue has been resolved.

Is there anything else I can assist you with, @jerelmiller?
Once again, thank you for the fix!

[jerelmiller]

Awesome to hear. Thanks @av-k! At this point I should just need a code review from the team to get it in. I'll try and get this in the next patch version. Thanks for confirming!

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

[iamdylancurran]

@jerelmiller Can confirm your fix solved our issue and we've updated to the latest release now, thank you 🙏

[jerelmiller]

Glad to hear it! Thanks for confirming.

[github-actions]

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.