Quality cleanups: dedup Duration.seconds, statusCodeType placement, decouple composedURL error#323
Merged
Conversation
…wnload skeleton Four review leftovers, all behavior-preserving (full suite green): - Consolidate Duration→seconds into one extension (Helpers); drop the duplicate static on CacheExpiry and the private one in HTTPInterceptor. - Move HTTP status classification off the global Int namespace: replace the public Int.statusCodeType extension with Networking.StatusCodeType (statusCode:), so consumers' Int isn't polluted. (Breaking; v8.) - Decouple composedURL from mapThrownError: composedURL throws a typed NetworkingError.invalidRequest(.invalidURL) instead of an NSError the mapper had to recognize by domain string. - Extract the shared download skeleton (request-id/clock/emit/catch/ complete) into performDownload; handleDataRequest/handleImageRequest keep their type-specific branches, which differ in real ways (data caches pre-status-check, image success-only). CHANGELOG updated for the two public-surface changes.
The performDownload wrapper passed an async work-closure capturing cachingLevel/self into an awaited call, which Swift 6.2's region isolation rejects (sending … risks data races) even though 6.3 accepts it. It was the least-valuable of the four cleanups (the two handlers differ in real ways), so restore them to their CI-green form, keeping only the StatusCodeType(statusCode:) substitution. Fixes the other three stand.
3lvis
changed the title
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
1 participant
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.
Three of the four "review leftovers" from the cache-fixes review, in their own PR. Behavior-preserving — full suite (180) green, build warning-free.
1.
Duration → secondsdeduplicatedOne
extension Duration { var seconds }inHelpers.swift. Dropped the duplicateCacheExpiry.seconds(_:)static and the private copy inHTTPInterceptor.swift;CacheExpiry/CacheStore/RetryInterceptorall share it.2.
statusCodeTypemoved off the globalIntnamespace (breaking; v8)public extension Int { var statusCodeType }polluted every consumer'sInt. Replaced withNetworking.StatusCodeType(statusCode:)— classification lives on the type it describes. Call sites + test updated; CHANGELOG noted under Changed/Removed.3.
composedURL/mapThrownErrordecoupledcomposedURLthrew anNSErrorthatmapThrownErrorrecognized by domain string. It now throws a typedNetworkingError.invalidRequest(.invalidURL), caught by the existingas? NetworkingErrorbranch. The domain-string check remains only as a fallback for the rare other internalNSErrors, with an accurate comment.Dropped: #4 (download-handler dedup)
Attempted to extract the shared skeleton of
handleDataRequest/handleImageRequestinto aperformDownloadwrapper, but passing theasyncwork-closure (capturingcachingLevel/self) into an awaited call trips Swift 6.2's region isolation (sending … risks data races) — even though 6.3 accepts it. It was the least-valuable of the four (the two handlers differ in real ways: data caches before the status check, image success-only), so I reverted it rather than contort around the isolation checker. The two handlers stay as-is.Tests
180 green, warning-free. Behavior-preserving refactors guarded by the existing suite; the one breaking change (#2) is exercised by the updated
testStatusCodeType.