Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error handling for CacheWriteInterceptor added in 1.14.0 causing issues #3420

Closed
aduflo-envoy opened this issue Jul 31, 2024 · 2 comments
Closed
Labels
bug Generally incorrect behavior duplicate Issues that have been determined to be a dupe of another issue

Comments

@aduflo-envoy
Copy link

Summary

Apollo 1.14.0 introduced handling of a new edge case for CacheWriteInterceptor, noted as CacheWriteError.missingCacheRecords.

In handling this new edge case, a new control flow was introduced:

guard
      let createdResponse = response,
      let cacheRecords = createdResponse.cacheRecords
    else {
      chain.handleErrorAsync(
        CacheWriteError.missingCacheRecords,
        request: request,
        response: response,
        completion: completion
      )
      return
    }

Given that this control flow calls handleErrorAsync() instead of proceedAsync(), it prematurely exits the request chain.

In 1.13.0, given this edge case was unhandled, the request chain would continue through interceptors registered after CacheWriteInterceptor, so this was a nonissue.

This new control flow changes expectations for Apollo Interceptors, and for my specific use case, I'd need to omit CacheWriteInterceptor from the list of interceptors to reestablish the previous control flow. At this time I'll need to have Apollo marked as 1.13.0 exact version in SPM, until this is adjusted.

Version

1.14.0

Steps to reproduce the behavior

Trigger the edge case outlined above, so anytime there aren't cache records available.

Logs

No response

Anything else?

No response

@aduflo-envoy aduflo-envoy added bug Generally incorrect behavior needs investigation labels Jul 31, 2024
@calvincestari calvincestari added duplicate Issues that have been determined to be a dupe of another issue and removed needs investigation labels Jul 31, 2024
@calvincestari
Copy link
Member

Hi @aduflo-envoy, thanks for reporting the issue. Good news is that this has already been resolved and merged into main. It'll go out in the next release which we're planning to publish this week still.

@calvincestari calvincestari closed this as not planned Won't fix, can't repro, duplicate, stale Jul 31, 2024
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Generally incorrect behavior duplicate Issues that have been determined to be a dupe of another issue
Projects
None yet
Development

No branches or pull requests

2 participants