Pagination: Errors on initial page not reported

[pixelmatrix]

Summary

I'm using an instance of AsyncGraphQLQueryPager to fetch some data. As I was testing today I had a problem where the subscribe callback was not being called. After looking into it, the first page query actually failed to load.

It looks like the onFetch method switches over the Result<GraphQLResult<DataType>, Error> to check for errors, but inside the Success case, it assumes there will always be data:

https://github.com/apollographql/apollo-ios-pagination/blob/main/Sources/ApolloPagination/AsyncGraphQLQueryPagerCoordinator.swift#L302

switch result {
case .failure(let error):
    if isLoadingAll {
        queuedValue = .failure(error)
    } else {
        currentValue = .failure(error)
    }
    publisher.send(completion: .finished)
case .success(let data):
    guard let pageData = data.data else {
        publisher.send(completion: .finished)
        return   
    }

In my case, data.data is nil, and data.errors contains an error from the server. Instead, I was expecting that the subscriber would be called with an error.

Version

0.1.0

Steps to reproduce the behavior

1. Setup an AsyncGraphQLQueryPager instance with a query that returns a network error
2. Set a subscriber to print the results
3. Call fetch() on the pager
4. Observe that the subscriber closure is never called

Logs

No response

Anything else?

No response

[Iron-Ham]

@pixelmatrix thanks for reporting! I'll take a look at this one today.

[Iron-Ham]

To summarize the issue:

• We receive a 200 OK response from the client, but the response either contains no data or contains errors.
• errors (of type GraphQLError) aren't considered or surfaced to the user, meaning that we don't surface the partial-success case well.
• data is assumed to be present always.

The net effect of this is we aren't surfacing errors from the server with 200OK responses. There's some thought that's going to have to be made about how we would want to handle partial success – as well as multiple errors. At present, our failure case only takes one error. Similarly, I don't think that our binary success/failure cases cleanly map to a partial success.

[pixelmatrix]

Related to this, it seems if there is a 200 OK failure when another page is loading via loadAll() it causes the pager to retry that page over and over again.

[Iron-Ham]

Related to this, it seems if there is a 200 OK failure when another page is loading via loadAll() it causes the pager to retry that page over and over again.

That should also be handled by apollographql/apollo-ios-dev#428 – thank you for reporting.

[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 iOS usage and allow us to serve you better.