Apollo-android: Allow caching partial responses with error

[lwasyl]

I hate to be the one to bring up _it broke my workflow_ thing, but #2281 broke our workflow and I'm not sure how to deal with it. Basically we do make requests that return errors, because we have common fragments that we reuse for various queries. Also because of some backend logic, it's expected that some of those queries return errors due to fields being unavailable (as the user doesn't have permissions to view them). Until now everything worked, and we're fine with caching of these _partial_ responses, as the permissions level doesn't change between queries and we don't seem to end up with bad state.

Now we could refrain from updating Apollo until we figure it out, but we do need #2349 to fix another issue 🙂 I was looking for a way to work around this by e.g. substituting errors array with an empty list if all the errors are _expected_, but didn't find any. For example this:

override fun onResponse(response: ApolloInterceptor.InterceptorResponse) {
    callBack.onResponse(
        ApolloInterceptor.InterceptorResponse(
            response.httpResponse.orNull(),
            response.parsedResponse.map { it ->
                it.copy(errors = emptyList())
            }.orNull(),
            response.cacheRecords.orNull()
        )
    )
}

in an ApolloInterceptor added with addApplicationInterceptor doesn't seem to do the trick.

All in all it's kind of difficult to just _Make sure that your code doesn't rely on this behaviour_. We'd appreciate an optional, even if deprecated, fallback to the original behavior, or at least a suggestion on how to work around this without having to adjust the queries that might be returning errors.

[martinbonnin]

I'd say the RemoveErrorsInterceptor approach doesn't work because right now it is called after ApolloCacheInterceptor. I was thinking about adding APIs to allow complete customisation of the interceptor chain (see https://github.com/apollographql/apollo-android/issues/2330#issuecomment-640526461). So that might be a workaround.

If this usage is widespread, we might want to give a less workaround-y solution though?

[lwasyl]

I think the header suggested here but to _store_ these partial responses would be nice. In the long term exposing Apollo interceptors chain might be more general solution, although I'd be afraid of breaking things by messing so much with low-level apis like that. Even with the interceptor removing errors I wondered if it's okay to pass original response.httpResponse, but modified response.parsedResponse as they don't match anymore, and I have 0 idea what's happening with both internally.

[lwasyl]

I opened #2363 as a potential workaround until this is properly discussed.

[lwasyl]

Thank you :) @martinbonnin any chance of having a patch release with the merged changed?

[martinbonnin]

Releases typically happen every couple of weeks. We have snapshots to test changes in between releases:

repositories {
  maven { 
    url = uri("https://oss.sonatype.org/content/repositories/snapshots/")
  }
}

[lwasyl]

@martinbonnin I'll ask you to reconsider publishing a release with the fix. We started using snapshot release, and we pinned specific snapshot version (2.2.1-20200616.122014-10) to have deterministic builds and to avoid pulling in untested, unexpected changes. Unfortunately it seems that old snapshots are removed from sonatype, as the most recent snapshot right now is 2.2.1-20200618.143347-12. This makes it not really possible to use a snapshot release to pull a fix without breaking build reproducibility.

Seeing as the entire issue is about a breaking change in a minor release, I think it's reasonable to at least provide the workaround in a patch release ahead of the usual schedule.

[martinbonnin]

@lwasyl sounds reasonable, it's almost been 2 weeks already in all cases :). I'll make a new release tomorrow morning first thing.

Also interesting that sonatype removes old snapshot. For what it's worth, you can also find the snapshots, including the 2.2.1-20200616.122014-10 on bintray ojo: https://oss.jfrog.org/artifactory/oss-snapshot-local/com/apollographql/apollo/apollo-runtime/2.2.1-SNAPSHOT/. We publish to both locations.

I'm not sure what the bintray retention startegy is, it looks like they keep them a bit longuer.

[lwasyl]

Thank you! :) Good to know about bintray as well

[martinbonnin]

2.2.1 should be available.