GraphQLQueryWatcher does not propagate failure to watching code if watch is not supposed to fetch

[dfed]

Summary

I just spent most of today debugging an issue that I think would have been much easier to debug if the GraphQLQueryWatcher had a small change to it.

These lines mean that any query watch whose cache policy is .returnCacheDataDontFetch will fail silently if a query is made that results in the watch encountering a ApolloAPI.JSONDecodingError.missingValue error (or other error, but that's the one I got stuck on today).

Ideally, the GraphQLQueryWatcher would propagate an error to the self.resultHandler callback rather than eating the error. This could be as simple as adding an else after the linked lines above.

Version

1.16.1

Steps to reproduce the behavior

I've attempted to simplify the problem I hit earlier today to a very minimal repro. I have not tested these steps, however.

Have a schema:

type Query {
  me: User!
}

type User implements Node {
  id: ID!
  name: String!
  isActive: Bool!
}

Have a query that is executed on app launch, and then watched (with a cache policy .returnCacheDataDontFetch) after the first query successfully pulls data from the network:

query AppLaunchQuery {
  me {
    id
    name
    isActive
  }
}

Have a malformed query that is executed later:

query MalformedQuery {
  me {
    # No ID in this query means that the watch doesn't know what user we're dealing with
    name
    # No isActive in this query which means the user that we pull down is not a complete `User`, and therefore is missing a value
  }
}

After the MalformedQuery is executed, the watch on the AppLaunchQuery will permanently fail, and the adopting app will never be notified via a callback.

Logs

Anything else?

This is tangentially related to #3501

[calvincestari]

Thanks for creating the issue @dfed. I can understand the simple example and your desired behaviour but I'm unsure of what is desired when the results are more complex.

What would be your expectation if AppLaunchQuery fetched more than just me at the root and MalformedQuery continued to fetch only me but failed?

[calvincestari]

Hi @dfed - @AnthonyMDev and I discussed this issue today but we're not fully understanding what the issue is and I think it could be down to the example you've provided. In the example above the query watcher would never be fired because the 'malformed' query has no id for the 'me' object so it has no way to update the cache.

It will probably be better if you could share the actual operation/workflow you're having an issue with. Then we can better try uncover the root cause and what the best solution might be. If you cannot share those details publicly then you can always email us privately and we can continue the conversation there.

[dfed]

Apologies for the delay here!

What would be your expectation if AppLaunchQuery fetched more than just me at the root and MalformedQuery continued to fetch only me but failed?

In the scenario you describe, I believe that the AppLaunchQuery would fail, since once me was in a bad state in the cache-only watch it can't recover. My highest-level reaction is "if the AppLaunchQuery/. returnCacheDataDontFetch watch enters a state where it is failing (and it can't recover because it won't ever fetch), it should not do so silently".

In the example above the query watcher would never be fired because the 'malformed' query has no id for the 'me' object so it has no way to update the cache.

Very possible I'm misunderstanding how exactly my malformed query caused the issue – I know that adding an id to the malformed query made it well-formed, but maybe there was something else at play too? I'm happy to email – our full schema and queries has a lot in it that I'd prefer not to expose to the world. I'll send that shortly. EDIT: I'm guessing your work emails. If it doesn't reach you, feel free to shoot me one at dan at portola.ai

Zooming out a bit through – what's the reasoning behind GraphQLQueryWatcher not propagating .failures in the store method?