Apollo-ios: Query watcher not being called when cache is updated on an element by another query/subscription/mutation

[benoitletondor]

Bug report

Query watcher not being called when cache is updated on an element of a collection that is added after calling watch

Versions

Please fill in the versions you're currently using:

• apollo-ios SDK version: 0.34.0
• Xcode version: 12.0.1
• Swift version: 5.3

Steps to reproduce

I've been trying a lot of things on that one and wasn't able to find a way to fix my problem. So in my app I'm having a list of conversations, so I'm creating a watch on a query that returns the list of conversations, this is something like that:

user {
    uuid
    firstName
    lastName
    conversations {
        uuid
        unreadMessagesCount
    }
}

In the app we use the uuid key to handle cache, so we make sure to always pass uuid in our queries to automatically handle cache update.

So the role of the watcher I'm talking about is both to update existing conversations and also be able to catch when a new conversation is created (it can happen and not be initiated by the user, we then trigger an event from the backend that is listen from a subscription in the app). When this event happens it returns something like that

event {
     newConversation {
          conversation {
               uuid
               unreadMessagesCount
               // This is the part that adds the new conversation to the existing ones of the users in the cache
               user {
                    uuid
                    conversations {
                          uuid
                          unreadMessagesCount
                    }
               } 
          }
     }
}

It works great, meaning that when this event happens, the watcher is being called with the newly created conversation, but the issue is that any new cache update for that specific conversation doesn't trigger the watcher again.

After investigating a bit, I realised that the cache is being updated because if I'm adding 3 new messages into the new conversation (setting the unreadMessagesCount to 3), the watcher doesn't get called but then if I add 1 new message into an old one, the watcher is being called with both the new message on the old conversation and the 3 new on the new one.

So it really seems like watch is not being called again for changes on an item that wasn't in a collection when the watch was initially made. I've also take a look at https://github.com/apollographql/apollo-ios/issues/281 and making a fetch on the same query again after the event doesn't fix the issue.

Let me know if I'm not clear on something as the whole thing is a bit complicated to explain.

[benoitletondor]

To add more to this, I feel like this is not related to collections only. If you have 1 query that watches something, then another query/mutation updates this cache indirectly (not using the same query, but another query that contains the same element with the same uuid), it doesn't work.

[designatednerd]

Do you have any thoughts on why the test that was added would be passing, but what you're working with would not be working?

[benoitletondor]

I'm afraid no, but all I can tell is that it works with 0.32.0 and not with 0.34.0

[Narayane]

Hi @designatednerd,

I think I have the same problem but not on a collection. My watcher is not triggered anymore since I have updated Apollo from 0.31.0 to 0.34.1. I have a watcher in cachePolicy .returnCacheDataAndFetch on this Query.

query GetRide($rideId: ID!) {
    ride(id: $rideId) {
        ...EndRideFragment
    }
}

And I have a mutation that updates my cache and normally triggers my watcher.

mutation StopRide($input: StopActiveRideInput!) {
    stopActiveRide(input: $input) {
        ride {
            ...EndRideFragment
        }
    }
}

Since my Apollo pod upgrade, not working anymore. My watcher is not triggered when mutation payload updates my cache.

[dhritzkiv]

This is also occurring for me. 0.33.0 (non beta) worked; but not 0.34.0 or 0.34.1

[designatednerd]

@dhritzkiv That makes sense, the network stack and how it interacts with the cache changed completely in 0.34.0.

Apologies, I've been under the weather. I'm gonna do some digging starting tomorrow to try to figure out how I broke this without breaking the tests 🙃

[dhritzkiv]

No rush from me! I was able to roll back to 0.33.0 no problem

[Narayane]

I can wait 1 week to have feedback from you @designatednerd on this issue.
If you need more time to investigate or to solve it, I could roll back to 0.33.0 to avoid blocking me in production

[benoitletondor]

Same here, I rollbacked to 0.32.1 so no rush from me, and thanks for taking time investigating this!

[martijnwalraven]

I'm trying to reproduce this issue to investigate what could be going on, but it's been hard because unfortunately our tests aren't set up to run in real life conditions, where concurrency may play a role. The existing watcher tests all continue to pass, and so do some new ones I've been adding.

It would be helpful to learn more about the way people who experience this are using the framework, and what behavior they are seeing:

• How are you initializing and configuring the client, cache, store and network transport?
• Are you calling methods on ApolloClient from the main thread or from a background thread? Do you receive results on the main queue as well, or are you passing in a custom queue?
• Does the issue occur 100% of the time, or only sometimes?
• Are there other operations in flight when this happens, or does it also happen with just a single watcher and a single related query or mutation?
• Anything else unusual or noticeable?

I know this is a long shot, but it would be even more helpful if anyone was able to share a project that reliably exhibits this issue.

[martijnwalraven]

A few more thoughts after taking a closer look at this:

1) It seems any operations using the WebSocketTransport always ignore the cache completely starting in 0.33 (I believe https://github.com/apollographql/apollo-ios/commit/444c465671c786659599a16da4b2d23d43648a74 is where this change was introduced). That means subscription results are not actually published to the store, and thus will not trigger watchers. (@benoitletondor I think that would at least explain your initial bug report).

2) That doesn't yet explain why queries and mutations would also fail to trigger watchers (assuming those are not using the WebSocketTransport). There is a possibility for misconfiguration however, which would lead to inadvertently having multiple stores (see https://github.com/apollographql/apollo-ios/issues/1438). Could some of you be bitten by this maybe?

3) Using a custom NetworkTransport also currently ignores the cache completely (see https://github.com/apollographql/apollo-ios/pull/1442 for an in progress PR to work around this). Any chance this could explain the behavior some of you are seeing?

[dhritzkiv]

I'm not using WebSocketTransport, but 3. sound like it could be what's plaguing us! Will have some time in two weeks to poke around that

[Narayane]

@martijnwalraven I guess 3. could be an explanation in my case, I have a custom NetworkTransport implementation (http request headers additions, checks on http response)

public func send<Operation: GraphQLOperation>(operation: Operation, completionHandler: @escaping (_ result: Result<GraphQLResponse<Operation.Data>, Error>) -> Void) -> Cancellable in 0.33

becomes

public func send<Operation>(operation: Operation, cachePolicy: CachePolicy, contextIdentifier: UUID?, callbackQueue: DispatchQueue, completionHandler: @escaping (Result<GraphQLResult<Operation.Data>, Error>) -> Void) -> Cancellable where Operation : GraphQLOperation in 0.34

I need to change

do {
    let body = try self.serializationFormat.deserialize(data: data) as! JSONObject
    let response = GraphQLResponse(operation: operation, body: body)
    completionHandler(.success(response))
} catch { }

into

do {
    let body = try self.serializationFormat.deserialize(data: data) as! JSONObject
    let response = GraphQLResponse(operation: operation, body: body)
    let result = try response.parseResultFast()
    completionHandler(.success(result))
} catch { }

to try to conform my code to the new signature

[Narayane]

@designatednerd @martijnwalraven any progress?

[designatednerd]

not yet - also, be aware that parseResultFast doesn't hit the cache, so you'd probably need to use the completion-closure based parsing.

[Narayane]

What?
A code example could save my day :)

[designatednerd]

@Narayane @benoitletondor @dhritzkiv I think this should be resolved by 0.37.0. Could y'all give that a try and let me know if it's still an issue on that version?

[dhritzkiv]

@designatednerd A preliminary run using 0.37.0: it looks like the cache+watchers work as expected! Thanks. :)

[Narayane]

@designatednerd Still an issue in my case, watchers are not triggered in 0.37.0 with the code below in my custom NetworkTransport.

do {
    let body = try self.serializationFormat.deserialize(data: data) as! JSONObject
    let response = GraphQLResponse(operation: operation, body: body)
    response.parseResultWithCompletion(completion: { result in
         switch result {
         case .success(let gqlResult):
              completionHandler(.success(gqlResult.0))
         case .failure(let error):
              completionHandler(.failure(LRError.clientError(error.localizedDescription)))
         }
     })
} catch { }

[designatednerd]

@Narayane Yeah, the caching on custom network transports is basically broken at the moment, so that would make sense. The current workaround is to write a custom interceptor for your network usage similar to the default Network interceptor but using your own networking.

I'm working on a longer-term fix but this is something that was missed through the RFC and Beta processes so there's a decent amount of rethinking that needs to be done there, unfortunately 😭

[Narayane]

Hi @designatednerd,

any progress on my problem?

It begins to be disturbing to be stuck in v0.33 since v0.39 is the current...

[designatednerd]

At this point the recommended workaround remains creating your own custom network interceptor and using the RequestChainNetworkTransport rather than creating your own network transport - this should work in almost all cases, I would love to hear any cases where it doesn't so I can fix whatever needs to be fixed to make that work.

This is definitely something that needs to be revisited but at this point i think the workaround is sufficient that it'll be better long-term to do it as part of enhancements to the chain system that will make it easier to use this stuff for all network requests (including, hopefully, web sockets).