returnCacheDataElseFetch returning nil instead of fetching

[joelmbell]

Describe the bug
When using the CachePolicy.returnCacheDataElseFetch while Querying, a call is never made to the NetworkTransport class to return data even though the cache does not have a value for this Query yet. This happens when you have already completed a different query first.

To Reproduce
Steps to reproduce the behavior:

1. Ensure no previous cache values exist (easy solution is to delete the app off device before this test)
2. Run a query with this cache policy -- fetchIgnoringCacheData
3. Run a DIFFERENT query with this cache policy -- returnCacheDataElseFetch
4. See the second query coming back with nil, instead of fetching the data from the server

Expected behavior
If there has not been a query for that data yet, then it should fetch that data from the server and return instead of just returning nil from the cache

Environment(please complete the following information):

• AppSync SDK Version: 2.6.22 (Verified it is working properly in 2.6.21)
• Dependency Manager: Cocoapods
• Swift Version: My project is using swift 4.2 (AppSync dependencies are using swift 4.1.2)

Device Information (please complete the following information):

• Device: Simulator and iPad mini (real device)
• iOS Version: iOS 11.4, 11.3.1
• Specific to simulators: no

Additional context
Everything works properly in 2.6.21, this issue only shows up when I upgrade my dependency to 2.6.22.

If you view the comments below, this bug has been isolated to a change in the Apollo GraphQLExecutor. commit c417b11

Additionally tests have been written in the comments below that demonstrate this issue.

[joelmbell]

We're still encountering this issue on the latest build (2.6.24)

Here is some additional information we've encountered while testing.

It seems to only happen after a different query with the cache policy (fetchIgnoringCacheData) has already completed.

Our use-case is that when a user-logs in, a query is performed (fetchIgnoringCacheData) that loads in data that the user will need. This is successful. Once that query is completed, we do a different query fetching some additional information the user needs. For this second query we use the cache policy (returnCacheDataElseFetch). This second query is the one that encounters the issue. No network request is made, and a value from the cache is returned with nil.

We have verified there are no cached values for this query when nil is returned from the cache.
We have verified that if we do not do the first query, then the second one works as expected.

[peteslavich]

We created two tests inside ApolloTests/FetchQueryTests that I believe reproduce the issue. The first test, testSICSecondQueryAlone(), runs successfully, and is essentially a duplicate of testFetchIgnoringCacheData() but uses a StarshipQuery and a .returnCacheDataElseFetch cache policy. The second test, testSICFetchIgnoringCacheData(), first performs the fetch from testFetchIgnoringCacheData(), then in the completion handler of the first query performs the fetch from testSICSecondQueryAlone(). In this case the StarshipQuery returns nil.

func testSICSecondQueryAlone() {
    let query2 = StarshipQuery()
    let initialRecords: RecordSet = [:]
    
    withCache(initialRecords: initialRecords) { cache in
        let store = ApolloStore(cache: cache)
        
        let networkTransport2 = MockNetworkTransport(body: [
            "data": [
                "starship": [
                    "name": "ASDF",
                    "__typename": "Starship"
                ]
            ]
            ])
        let client2 = ApolloClient(networkTransport: networkTransport2, store: store)
        
        let expectation = self.expectation(description: "Fetching query")
        
        client2.fetch(query: query2, cachePolicy: .returnCacheDataElseFetch) { (result, error) in
            defer { expectation.fulfill() }
            
            guard let result = result else { XCTFail("No query result");  return
            }
            guard let name = result.data?.starship?.name else {
                XCTFail("No query result")
                return
            }
            
            XCTAssertEqual(name, "ASDF")
        }
        
        self.waitForExpectations(timeout: 5, handler: nil)
    }
}

func testSICFetchIgnoringCacheData() throws {
    let query = HeroNameQuery()
    
    let initialRecords: RecordSet = [:]
    
    
    withCache(initialRecords: initialRecords) { cache in
        let store = ApolloStore(cache: cache)
        
        let networkTransport = MockNetworkTransport(body: [
            "data": [
                "hero": [
                    "name": "Luke Skywalker",
                    "__typename": "Human"
                ]
            ]
            ])
        
        let client = ApolloClient(networkTransport: networkTransport, store: store)
        
        let expectation = self.expectation(description: "Fetching query")
        let expectation2 = self.expectation(description: "Fetching query2")

        client.fetch(query: query, cachePolicy: .fetchIgnoringCacheData) { (result, error) in
            defer { expectation.fulfill() }
            
            guard let result = result else { XCTFail("No query result");  return }
            
            XCTAssertEqual(result.data?.hero?.name, "Luke Skywalker")
            let query2 = StarshipQuery()
            let networkTransport2 = MockNetworkTransport(body: [
                "data": [
                    "starship": [
                        "name": "ASDF",
                        "__typename": "Starship"
                    ]
                ]
                ])
            let client2 = ApolloClient(networkTransport: networkTransport2, store: store)
            
            client2.fetch(query: query2, cachePolicy: .returnCacheDataElseFetch)
            { (result, error) in
                defer { expectation2.fulfill() }
                
                guard let result = result else { XCTFail("No query result");  return }
                
                guard let name = result.data?.starship?.name else {
                     XCTFail("No query result")
                    return
                }
                
                XCTAssertEqual(name, "ASDF")
            }
        }
        
        self.waitForExpectations(timeout: 5, handler: nil)
    }
}

[peteslavich]

Additionally, we believe the code change that is causing the issue is in commit c417b11 in GraphQLExecutor line 214. In previous versions of app sync that do not show the bug, the second query executes the throw JSONDecodingError.missingValue, while since this change, the second query executes self.complete(...)

[joelmbell]

Thank you @peteslavich!

@rohandubal @palpatim would you like us to submit a pull-request with this test breaking? @peteslavich has narrowed down what looks like the likely cause, but we don't yet fully understand the reason for that change so we are hesitant to change it ourselves.

[palpatim]

Hi @joelmbell,

Sorry for the delayed response on this. I have a test that demonstrates this behavior and am investigating the proper way to handle this. I'll update this thread as soon as I have more info.

[palpatim]

I was able to make some headway on this during investigations into updating the cache on optimistic updates. PR #182 should fix this, assuming that the approach works as I expect. (I'm investigating whether it is in fact the right approach; I need to validate the assumptions I'm making testing for "empty result values" in that PR.)

[palpatim]

This fix is released in 2.10.1. Please let us know if you continue to see issues.

[stale]

This issue has been automatically closed because of inactivity. Please open a new issue if are still encountering problems.