Watching a query leads to constant refetch when value of a nullable type is null

[aleksanderlorenc-lw]

Summary

Hi, at Lifeworks we are watching queries in parts of the app. After upgrading to Apollo 1.2.1 we started observing failures when watching a query when a nullable type is null, leading to constant re-fetches of this query.

We ran a quick investigation and this is the result:

1. Watch is done using GraphQLQueryWatcher.store
2. GraphQLSelectionSetMapper is called with default initializer which assumes (handleMissingValues: HandleMissingValues = .disallow) in the accumulator parameter of transaction.readObject - which disallows null values for a field
3. Because handleMissingValues is disallow, GraphQLSelectionSetMapper.acceptMissingValue throws JSONDecodingError.missingValue
4. Error is passed to ApolloStore.withinReadTransaction

(lldb) po error
▿ GraphQLExecutionError
  ▿ path : some.field.on.schema.11.nullableFieldItself
    ▿ head : Optional<Node>
      ▿ some : <Node: 0x600001f0f600>
  - underlying : ApolloAPI.JSONDecodingError.missingValue

6. GraphQLQueryWatcher.store failure condition triggers a re-fetch on failure, cycle repeats

We observed that contrary to load, the values are stored with allowForOptionalFields, if that is of any help.

To summarize:

• cache shouldn’t fail when null if is nullable
• the watcher probably shouldn't do a remote refresh on an error
• insert/updates on the cache should fail if wrong data is being inserted (fail early, not let us insert and then fail when retrieving)

Version

1.2.1

Steps to reproduce the behavior

1. Use a query which contains a nullable type
2. Watch the query
3. Backend should respond with the value being null
4. When written to and loaded from the cache, an error will be thrown and will result in a remote refetch

Logs

(lldb) po error
▿ GraphQLExecutionError
  ▿ path : some.field.on.schema.11.nullableFieldItself
    ▿ head : Optional<Node>
      ▿ some : <Node: 0x600001f0f600>
  - underlying : ApolloAPI.JSONDecodingError.missingValue

### Anything else?

_No response_

[calvincestari]

Thanks for reporting the issue @aleksanderlorenc-lw.

After upgrading to Apollo 1.2.1 we started observing failures when watching a query when a nullable type is null, leading to constant re-fetches of this query.

Do you know if/which earlier versions behaved differently?

[aleksanderlorenc-lw]

Thanks for reporting the issue @aleksanderlorenc-lw.

After upgrading to Apollo 1.2.1 we started observing failures when watching a query when a nullable type is null, leading to constant re-fetches of this query.

Do you know if/which earlier versions behaved differently?

Hey @calvincestari , I'm not sure about this exactly, but the main issue is that the stored values fail to get read. The re-fetch is just a side-effect

[calvincestari]

Are you able to try 1.2.2, or 1.3 preferably, to see whether those versions behave differently? Those releases contained some important bug fixes. Please take note of the migration required for the jump to 1.3 though.

[aleksanderlorenc-lw]

I've just tried out 1.3 in our codebase - the problem persists in this version

[AnthonyMDev]

Hmmm, I'm not quite sure what's going on here yet. You say your backend is responding with the value being null. In which case the null for the field should be written to the cache and then we would be calling handleNullValue instead of handleMissingValue. If handleMissingValue is being called, it means that the key for the field doesn't exist on the object in the cache.

insert/updates on the cache should fail if wrong data is being inserted (fail early, not let us insert and then fail when retrieving)

Can you clarify here, I believe that is how it is supposed to work today. Are you doing some manual cache update prior to the watcher getting the failure, or is it just a network response being written to the cache?

[aleksanderlorenc-lw]

Are you doing some manual cache update prior to the watcher getting the failure, or is it just a network response being written to the cache?

Yes, we're manually updating the cache using ApolloStore.updateObject within ApolloStore.withinReadWriteTransaction in the resultHandler of ApolloClient.fetch

[AnthonyMDev]

Okay got it. It sounds like your cache mutation is doing something that makes the data for the watcher incomplete. Are you setting the value of the nullable field to nil in your mutation?

[aleksanderlorenc-lw]

Not explicitly, but we're basically appending the results of the current fetch to a cache, which already has some results stored, so this results in nils being appended

[AnthonyMDev]

Okay, I'm still just struggling to wrap my head around what specific shape of data is causing this. We handle "missing values" and "explicitly null values" differently. It sounds like there may be a bug in how we are handling that, but I'm not sure what would be the situation in which it's appending a null and its getting treated as a nil.

Let me see if I can write up a unit test that reproduces this. If I'm not able to, I might need to get an example project from you that reproduces it...

[AnthonyMDev]

I think I'm going to need you to provide me a reproduction. Instead of creating an entire example project, you could try writing a unit test in WatchQueryTests that shows the behavior you are experiencing.

[aleksanderlorenc-lw]

I managed to reproduce the issue in a sample iOS project (Apollo using SPM in Xcode) with a GraphQL express.js-based server (index.js) in the root.

Repo: https://github.com/aleksanderlorenc-lw/apollo-cache-with-fragments

So in the real project we also:

1. watch
2. fetch + updateObject using a fragment within withinReadWriteTransaction within fetch's resultHandler