fix(workflows): Don't serve stale workflows when the backend rejects the request#6946
Merged
Conversation
The getWorkflowsList failure branch restored the cached list + details on any error. That's right for transient errors (5xx / offline) but wrong for a 4xx, where the backend is authoritatively rejecting the request (workflows disabled, unauthorized), so we shouldn't serve stale prefetched data. Generalize the existing offerings classification into BackendError/NetworkError.shouldFallBackToCache (the offerings property now delegates to it, no behavior change) and gate the workflows disk restore on it. 4xx skips the restore; 5xx and connection failures still restore. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
|
cool, I actually I am working on the Android counterpart here |
vegaro
approved these changes
Jun 8, 2026
…fix (#6948) - testGetWorkflowsListCallsOnCompleteOnClientError: verifies onComplete() fires on 4xx even though disk restore is skipped (callers must not block) - testGetWorkflowsListRestoresDetailsFromDiskOnServerError: verifies that on 5xx both list mapping AND workflow details are restored from disk, not just the list mapping Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Member
Author
|
thanks @vegaro 👨🎤 |
Per review: shouldFallBackToCachedOfferings was only a thin delegate to shouldFallBackToCache after the prior refactor. Remove it on NetworkError and BackendError and point callers (OfferingsManager, tests) at shouldFallBackToCache directly. Behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #6917.
What
#6917 made the workflows list survive the backend being down: if the
/workflowsfetch fails, we fall back to the last copy on disk so a cold start can still render the prefetched paywalls.The problem is we did that on any failure, including a 4xx. A 4xx isn't "the backend is down", it's the backend telling us no (workflows turned off for the app, this user isn't allowed, etc). Serving the stale prefetched data we just got rejected for is the wrong move. So now we only fall back on the transient stuff (5xx, offline); a 4xx skips the restore.
Nothing new to decide here. Offerings already draw this exact line, so I reused their rule instead of inventing one: pulled the classification up into
shouldFallBackToCacheonNetworkError/BackendError, and the offerings property just delegates to it now (so offerings behaviour is identical,OfferingsManageruntouched).flowchart TD A["getWorkflowsList: /workflows fetch"] --> B{result} B -->|success| C[cache list + prefetch details] B -->|failure| D{4xx?} D -->|"yes, backend rejected it"| E["skip restore<br/>(don't serve stale)"] D -->|"no, 5xx / offline"| F["restore list + details from disk<br/>(render offline)"] E --> G([onComplete]) F --> G C --> GIt only gates the disk restore, it doesn't wipe anything already in memory (same as offerings).
AI session context
Metadata
facu/workflow-list-4xx-no-stale(follow-up to feat(workflows): persist prefetched workflow detail on disk #6917)mainGoal
Stop the workflows list-fetch failure path from restoring stale prefetched data on a 4xx, while keeping the 5xx / offline backend-down recovery from #6917. Workflows-side + shared error helper only.
Initial Prompt
Identified as a missing follow-up from #6917 (its original body listed a "status-based fallback gate so a 4xx doesn't restore" non-goal). The human picked it up: "Let's tackle 2."
Agent Contribution
shouldFallBackToCachedOfferings: 5xx/offline fall back, 4xx doesn't) and reused it rather than inventing a rule.NetworkError.shouldFallBackToCache/BackendError.shouldFallBackToCacheand pointed all callers (OfferingsManager+ the error tests) at it directly. Behavior-preserving (the old offerings name only ever returned the same value).WorkflowManager.getWorkflowsList's failure-branch disk restore onerror.shouldFallBackToCache.shouldFallBackToCachedOfferingsdelegating alias was redundant, so it was removed entirely instead of kept.Human Decisions
Key Implementation Decisions
shouldFallBackToCacheinstead of a workflows-specific rule, so both paths treat backend errors consistently. Per review, the oldshouldFallBackToCachedOfferingsname was dropped (not kept as an alias) and callers useshouldFallBackToCachedirectly.Files / Symbols Touched
Sources/Networking/HTTPClient/NetworkError.swift—shouldFallBackToCachedOfferingsrenamed toshouldFallBackToCache.Sources/Error Handling/BackendError.swift— same rename.Sources/Purchasing/OfferingsManager.swift— usesshouldFallBackToCache.Sources/Purchasing/WorkflowManager.swift—getWorkflowsListfailure branch guards restore onshouldFallBackToCache.Tests/UnitTests/Purchasing/WorkflowManagerTests.swift— 4xx-no-restore + 5xx-restore tests.Tests/UnitTests/Networking/{BackendError,NetworkError}Tests.swift— updated toshouldFallBackToCache.Validation
swift build: clean.swiftlinton changed files: no violations.xcodebuild test -scheme UnitTests:BackendErrorTests9,NetworkErrorTests8,OfferingsManagerTests39,WorkflowManagerTests39 all passing (95 total, 0 failures) after the rename, iPhone 16 Pro sim.Validation Gaps
Review Focus
shouldFallBackToCachesemantics match the priorshouldFallBackToCachedOfferingsexactly (4xx → false, 5xx/no-status → true); offerings now call it directly, so no offerings behavior change.Non-goals / Out of Scope
Omitted Context
Note
Medium Risk
Changes offline/paywall behavior when
/workflowsreturns 4xx and touches shared cache-fallback rules used by offerings, though offerings semantics are intended to stay the same.Overview
Fixes workflows list fetch failures restoring stale disk cache on 4xx responses, which could show prefetched paywalls after the backend had explicitly rejected the request.
shouldFallBackToCachedOfferingsis renamed and generalized toshouldFallBackToCacheonNetworkErrorandBackendError(5xx / connection failures → allow cache fallback; 4xx → do not).OfferingsManagernow gates disk fallback on the same property with unchanged semantics.WorkflowManager.getWorkflowsListonly runs list/detail disk restore whenerror.shouldFallBackToCacheis true; on 4xx it skips restore and still callsonComplete. Unit tests cover 4xx vs 5xx restore behavior and completion.Reviewed by Cursor Bugbot for commit 88491a6. Bugbot is set up for automated code reviews on this repo. Configure here.