Type the currency fetch errors and say what actually failed - #22
Merged
Conversation
The refresh banner read "Refresh failed, showing cached data" whatever had gone wrong, because that was the only sentence it could say: everything reached it as `.network(cause: .json)` or `.network(cause: .unknown)`. Two things caused that. `URLSession.resultData` never looked at the HTTP status, so a 401 error envelope went straight to the decoder and was reported as a malformed response; and anything thrown by the transport collapsed into `.unknown`. - ApiError gains noConnection, timeout, unauthorized, rateLimited, notFound, serverError and unexpectedStatus, classified from the status code and from URLError, with DataError.NetworkCause mirroring it. - StorageError distinguishes a failed read from an unclassified error. - Each cause carries its own copy and symbol in ErrorModel. - The banner shows the typed cause as its headline, demotes "showing the rates saved on this device" to a second line, and offers Retry. Fix why the currencies could not update in the first place: the API keys were being injected into Core/Sources/..., a path that stopped existing when the module directory was renamed to SwiftlyCore. The `cd` failed, `exit 0` hid it, and every build shipped the checked-in ApiKey.swift with three empty strings, so every request went out as `?apikey=` and came back 401. Both injection scripts now target the compiled file and fail loudly if it moves again; the stale Core/ copy is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WGP73fJTZHDfydoSMf2Hw
|
Review changes with Changed Files
|
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.
The refresh banner read "Refresh failed, showing cached data" whatever had gone wrong, because that was the only sentence it could say: everything reached it as
.network(cause: .json)or.network(cause: .unknown).Why the currencies can't update
Tracing the vague message turned up the reason there is anything to report in the first place: the shipped app has no API keys.
2996b67 "Chore: move Core -> SwiftlyCore"renamed the module directory, but both key-injection scripts still didcd Core/Sources/Currency/Data/Api, and a staleCore/.../ApiKey.swiftsurvived the move. So the keys were written into a file nothing compiles,exit 0swallowed the failedcd, and every build since has shipped the checked-inApiKey.swiftwith three empty strings. Requests go out as?apikey=, the services answer 401, and the app falls back to cached rates forever — which is exactly the "Updated 5 months ago" in the screenshot.Both
ci_scripts/ci_pre_xcodebuild.shand the "Add API keys" Xcode build phase now target the compiled file and fail the build if it isn't there, rather than silently shipping empty keys. The deadCore/copy is deleted.Why the message was vague
Two causes, both in
URLSession.resultData:.unknown.The typed errors
noConnectiontimeoutunauthorized(401/403)rateLimited(429)notFound(404)serverError(5xx)unexpectedStatusjsonDataError.NetworkCausemirrorsApiError;StorageErrorgainsreadFailedto separate a refused read from an unclassified one. Each cause carries its own copy and SF Symbol.The banner leads with the typed cause, demotes "Showing the rates saved on this device" to a second line, and offers Retry.
Tests
ApiErrorTests: status-code andURLErrorclassification, plusresultDatadriven through a stubbedURLProtocol— including the case that motivated all of this, that a 401 with an error body is reported asunauthorizedrather than as bad JSON.RealCurrencyRepositoryTests: the typed cause survives the fallback to cache.ConverterViewModelTests:refreshErrorcarries the typed cause, and is absent when the refresh succeeds.ErrorView's layout and the.jsoncopy are byte-identical, so the committed snapshot baselines still match.Two things to flag
RealCurrencyRepositoryTestsis dead code: none of its ~30 methods carry@Test, so swift-testing never runs them. The one added here does. I left the rest alone rather than enable 30 unverifiable assertions in the same PR, but that file asserts nothing today.Generated by Claude Code