From 2437e544c1a491e197cc8bf5cb4dffec8a0d4f7f Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 16 Sep 2026 12:45:19 -0700 Subject: [PATCH 1/2] fix(build): qualify InterlinedDomain.Document so the app compiles on Xcode 27 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dev` stopped building when Xcode was updated on this machine mid-session. The macOS 27 SDK adds a `Document` protocol to SwiftUI — public protocol Document: ReadableDocument, WritableDocument — which collides with the domain's `Document` struct in any file importing both. That is every documents-feature view, and the same unchanged source went from compiling to eleven `'Document' is ambiguous for type lookup` errors across eight files. Only the SwiftUI-importing files are affected, which is what makes the diagnosis unambiguous: the view models import Foundation, Observation and InterlinedDomain but not SwiftUI, and they compile untouched. The fix is to qualify the type at the use sites. Three alternatives were considered and rejected. Renaming the domain model is the tail wagging the dog — `Document` is the right name, and it is correct across Kit, Domain, Persistence and their tests. A module-level typealias would shorten the use sites at the cost of giving one concept two names, so the next reader has to learn they are the same thing. Dropping `import SwiftUI` is not available; these are views. Every edit is a type position. No user-facing string, accessibility label or other identifier containing the word Document is touched — the diff is eleven lines, each one a `Document` that the compiler itself pointed at. Worth knowing rather than fixing: this is a standing hazard. Any domain type sharing a name with a SwiftUI symbol is one SDK update away from the same break, and the diagnosis is written down at the top of DocumentsListView so the next occurrence takes minutes. Refs #98 Co-Authored-By: Claude Opus 5 --- App/Features/AI/AIDocumentSheet.swift | 2 +- .../Documents/ConflictBannerView.swift | 2 +- .../Documents/DocumentEditorView.swift | 2 +- .../DocumentTemplatePickerView.swift | 2 +- .../Documents/DocumentsListView.swift | 23 ++++++++++++++++--- .../Documents/DocumentsRootView.swift | 4 ++-- .../Documents/PublicUserDocumentsView.swift | 2 +- App/Features/Search/SearchRootView.swift | 2 +- 8 files changed, 28 insertions(+), 11 deletions(-) diff --git a/App/Features/AI/AIDocumentSheet.swift b/App/Features/AI/AIDocumentSheet.swift index bd9ba96..cb19b29 100644 --- a/App/Features/AI/AIDocumentSheet.swift +++ b/App/Features/AI/AIDocumentSheet.swift @@ -22,7 +22,7 @@ struct AIDocumentSheet: View { /// populated without the host having to hold lists it does not otherwise need. @State private var lists: [OwnedList] = [] /// Documents the user owns, offered when deriving from an article. - var documents: [Document] = [] + var documents: [InterlinedDomain.Document] = [] /// Called after a drafted document is created, so the host can reload it. var onCreated: (() async -> Void)? diff --git a/App/Features/Documents/ConflictBannerView.swift b/App/Features/Documents/ConflictBannerView.swift index 04b1586..8997aeb 100644 --- a/App/Features/Documents/ConflictBannerView.swift +++ b/App/Features/Documents/ConflictBannerView.swift @@ -15,7 +15,7 @@ import InterlinedDomain struct ConflictBannerView: View { let pending: ConflictBannerViewModel.Pending - let onOpenLocalCopy: (Document.ID) -> Void + let onOpenLocalCopy: (InterlinedDomain.Document.ID) -> Void let onDismiss: () -> Void var body: some View { diff --git a/App/Features/Documents/DocumentEditorView.swift b/App/Features/Documents/DocumentEditorView.swift index d4d6070..9d67308 100644 --- a/App/Features/Documents/DocumentEditorView.swift +++ b/App/Features/Documents/DocumentEditorView.swift @@ -21,7 +21,7 @@ import Textual struct DocumentEditorView: View { let viewModel: DocumentEditorViewModel - let onOpenLocalCopy: (Document.ID) -> Void + let onOpenLocalCopy: (InterlinedDomain.Document.ID) -> Void var body: some View { VStack(spacing: 0) { diff --git a/App/Features/Documents/DocumentTemplatePickerView.swift b/App/Features/Documents/DocumentTemplatePickerView.swift index ad4b203..330a2da 100644 --- a/App/Features/Documents/DocumentTemplatePickerView.swift +++ b/App/Features/Documents/DocumentTemplatePickerView.swift @@ -36,7 +36,7 @@ struct DocumentTemplatePickerView: View { /// Called with the created document on success so the caller (the root /// view) can bind the editor to it. Not called on failure. - let onCreated: (Document) -> Void + let onCreated: (InterlinedDomain.Document) -> Void /// The built-in catalog to present. Defaults to the bundled built-ins; /// injectable so previews can substitute a list. diff --git a/App/Features/Documents/DocumentsListView.swift b/App/Features/Documents/DocumentsListView.swift index e0935c4..90436dd 100644 --- a/App/Features/Documents/DocumentsListView.swift +++ b/App/Features/Documents/DocumentsListView.swift @@ -9,10 +9,27 @@ import SwiftUI import InterlinedDomain +// `Document` is written as `InterlinedDomain.Document` throughout the SwiftUI +// files in this feature, and that qualification is load-bearing. +// +// The macOS 27 SDK added a `Document` **protocol** to SwiftUI +// (`protocol Document: ReadableDocument, WritableDocument`), which collides with +// the domain's `Document` **struct** in any file importing both — which is every +// documents view. Before Xcode 27 the bare name resolved; after it, the same +// source stopped compiling with `'Document' is ambiguous for type lookup` +// (GitHub #98). +// +// The domain type is not renamed: `Document` is the right name for it, it is +// correct across Kit, Domain, Persistence and their tests, and renaming a core +// model to dodge a collision in one consumer is the tail wagging the dog. A +// `typealias` would shorten the use sites at the cost of giving one concept two +// names. Only the SwiftUI-importing files need this; the view models import +// Foundation and Observation, not SwiftUI, and are unaffected. + struct DocumentsListView: View { let viewModel: DocumentsListViewModel - let onSelect: (Document.ID?) -> Void + let onSelect: (InterlinedDomain.Document.ID?) -> Void /// Source of the **Move to folder** destinations. Optional so the column /// still renders in isolation (previews, and any future host that has no @@ -21,7 +38,7 @@ struct DocumentsListView: View { /// Called with the document that was moved, so the host can rebind an open /// editor to the server's relocated copy. - var onMoved: ((Document) -> Void)? = nil + var onMoved: ((InterlinedDomain.Document) -> Void)? = nil var body: some View { List(selection: Binding( @@ -103,7 +120,7 @@ struct DocumentsListView: View { // MARK: - DocumentRowView private struct DocumentRowView: View { - let document: Document + let document: InterlinedDomain.Document var body: some View { VStack(alignment: .leading, spacing: 2) { diff --git a/App/Features/Documents/DocumentsRootView.swift b/App/Features/Documents/DocumentsRootView.swift index b52561b..dd1fa43 100644 --- a/App/Features/Documents/DocumentsRootView.swift +++ b/App/Features/Documents/DocumentsRootView.swift @@ -433,7 +433,7 @@ struct DocumentsRootView: View { /// was started from the editor, so there is exactly one optimistic-rollback /// implementation rather than two that can disagree. private func handleMove( - documentID: Document.ID, + documentID: InterlinedDomain.Document.ID, to destination: FolderNode.ID?, folderTree: FolderTreeViewModel, documentsList: DocumentsListViewModel, @@ -449,7 +449,7 @@ struct DocumentsRootView: View { } private func handleOpenLocalCopy( - _ id: Document.ID, + _ id: InterlinedDomain.Document.ID, documentsList: DocumentsListViewModel, editor: DocumentEditorViewModel ) { diff --git a/App/Features/Documents/PublicUserDocumentsView.swift b/App/Features/Documents/PublicUserDocumentsView.swift index 9613180..8914bef 100644 --- a/App/Features/Documents/PublicUserDocumentsView.swift +++ b/App/Features/Documents/PublicUserDocumentsView.swift @@ -111,7 +111,7 @@ struct PublicUserDocumentsView: View { private struct PublicDocumentRow: View { - let document: Document + let document: InterlinedDomain.Document var body: some View { VStack(alignment: .leading, spacing: 2) { diff --git a/App/Features/Search/SearchRootView.swift b/App/Features/Search/SearchRootView.swift index dad88a5..c5479de 100644 --- a/App/Features/Search/SearchRootView.swift +++ b/App/Features/Search/SearchRootView.swift @@ -266,7 +266,7 @@ struct SearchRootView: View { /// a relative "updated" stamp. Kept local to the Search feature because /// the Documents feature's own row component is file-private. private struct DocumentSearchRow: View { - let document: Document + let document: InterlinedDomain.Document var body: some View { VStack(alignment: .leading, spacing: 6) { From 02a9b47589d2f2e54d72d05044fc6b87906fd1cb Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 16 Sep 2026 13:14:34 -0700 Subject: [PATCH 2/2] refactor(kit): keep the response body on an HTTP failure, opt-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit APIError keeps only the decoded `{error}` string; everything else in the body is discarded before any caller sees it. That is right for almost every failure, where a sentence is all the UI needs — and wrong for the few where the server answers a question rather than reporting a malfunction. The list-schema destructive-change guard replies 400 with a `propertiesWithData` array naming the columns that still hold data, and the app-settings family is compare-and-set, replying 409 with the current document attached. Both callers exist and both are written around the gap with a comment saying so. The obvious fix — a second associated value on the error cases — is not viable. 420 sites across 78 files construct or pattern-match those cases, including every test. That is an enormous mechanical change, every line of it a chance to get a case wrong, in service of two call sites. So this is opt-in. `sendCapturingFailure(_:)` throws an `APIFailure` carrying the body; every other entry point keeps throwing plain `APIError` and not one of the 420 sites changes. `APIFailure` is deliberately not an APIError, so a caller opts in by choosing the richer send rather than by accident — and it forwards `localizedDescription` so nothing regresses if one reaches generic error display. `details(as:)` returns nil rather than throwing on a mismatch. A caller asking for details is asking an optional question, and a decode failure there must degrade to today's behaviour instead of masking the HTTP failure being handled. The transport raises the richer error internally so one code path serves both entry points, and `send`/`sendVoid`/`sendRaw`/`sendWithRateLimitInfo` flatten it back. Writing that revealed a real trap: `performWithSafetyNet` and `performWithRetry` both catch `APIError`, so raising `APIFailure` silently disabled the 401 session-retry and the whole retry policy. The existing AuthTransport test caught it. Both now match on `underlyingError` and rethrow the `APIFailure` whole — flattening there would drop the body before `sendCapturingFailure` ever saw it, which is the entire point of the type. A decode failure on a 2xx stays a plain `APIError.decoding`: it is a client-side problem with no server body to offer, and dressing it as an `APIFailure` carrying nothing would be a lie about where the fault is. Refs #103 Co-Authored-By: Claude Opus 5 --- .../InterlinedKit/APIClient/APIClient.swift | 117 ++++++++++++++-- .../InterlinedKit/Errors/APIFailure.swift | 74 ++++++++++ .../InterlinedKitTests/APIFailureTests.swift | 132 ++++++++++++++++++ 3 files changed, 312 insertions(+), 11 deletions(-) create mode 100644 Packages/InterlinedKit/Sources/InterlinedKit/Errors/APIFailure.swift create mode 100644 Packages/InterlinedKit/Tests/InterlinedKitTests/APIFailureTests.swift diff --git a/Packages/InterlinedKit/Sources/InterlinedKit/APIClient/APIClient.swift b/Packages/InterlinedKit/Sources/InterlinedKit/APIClient/APIClient.swift index 053d4d3..baaafa2 100644 --- a/Packages/InterlinedKit/Sources/InterlinedKit/APIClient/APIClient.swift +++ b/Packages/InterlinedKit/Sources/InterlinedKit/APIClient/APIClient.swift @@ -28,6 +28,25 @@ public protocol APIClientProtocol: Sendable { /// Used for CSV export endpoints (`/api/exports/*`). func sendRaw(_ request: Request) async throws -> (Data, String?) + /// Executes a request and, on an HTTP failure, throws an ``APIFailure`` + /// **carrying the response body** instead of discarding it (GitHub #103). + /// + /// Opt-in on purpose. `APIError` keeps only the decoded `{error}` string, + /// which is all the UI needs for almost every failure — and not enough for + /// the few where the server answers a *question*: the list-schema + /// destructive-change guard names the columns that still hold data, and the + /// app-settings compare-and-set returns the current document on a conflict. + /// + /// Only callers that need the body use this. Everything else keeps throwing + /// plain `APIError`, so the 420 existing pattern-match sites are untouched. + /// + /// - Important: this throws `APIFailure`, **not** `APIError`. A caller that + /// opts in must catch accordingly; `APIFailure.underlyingError` carries the + /// `APIError` for anything that only wants the status or the message. + func sendCapturingFailure( + _ request: Request + ) async throws -> Response + /// Executes a request, decodes its JSON body into `Response`, and returns /// any rate-limit metadata extracted from the response headers. /// @@ -47,6 +66,29 @@ public protocol APIClientProtocol: Sendable { // MARK: - Default implementation extension APIClientProtocol { + + /// Default for conformers that cannot capture a response body — primarily + /// stubs and fakes. + /// + /// It wraps whatever `send(_:)` threw with `body: nil`, so an opted-in + /// caller still sees an `APIFailure` and its `details(as:)` simply answers + /// `nil`. That is the correct degradation: "no details available" is a real + /// state (a transport failure has no body either), so a stub that cannot + /// supply one is not lying. + /// + /// A non-`APIError` failure is rethrown untouched rather than wrapped — + /// a `CancellationError` is not an HTTP failure and must not start looking + /// like one. + public func sendCapturingFailure( + _ request: Request + ) async throws -> Response { + do { + return try await send(request) + } catch let error as APIError { + throw APIFailure(underlyingError: error, body: nil) + } + } + /// Conformers that do not need real rate-limit header extraction — primarily /// stubs and fakes — get this default which calls `send(_:)` and returns /// `nil`, correctly signalling "no limit enforced". @@ -106,7 +148,7 @@ public final class APIClient: APIClientProtocol { public func send( _ request: Request ) async throws -> Response { - let (data, _) = try await performWithSafetyNet(request) + let (data, _) = try await unwrappingFailure { try await performWithSafetyNet(request) } do { return try decoder.decode(Response.self, from: data) } catch { @@ -122,11 +164,48 @@ public final class APIClient: APIClientProtocol { } public func sendVoid(_ request: Request) async throws { - _ = try await performWithSafetyNet(request) + _ = try await unwrappingFailure { try await performWithSafetyNet(request) } + } + + public func sendCapturingFailure( + _ request: Request + ) async throws -> Response { + // Deliberately does NOT unwrap: this is the one entry point whose + // caller asked for the body. + let (data, _) = try await performWithSafetyNet(request) + do { + return try decoder.decode(Response.self, from: data) + } catch { + let detail = String(reflecting: error) + appLog.error("Decode failed [\(request.path)] type=\(String(describing: Response.self)): \(detail)") + // A *decode* failure is not an HTTP failure and has no server body + // to offer, so it surfaces as the plain `APIError` it has always + // been rather than an `APIFailure` with nothing in it. + throw APIError.decoding( + type: String(describing: Response.self), + message: detail + ) + } + } + + /// Runs `work` and flattens any `APIFailure` back to its `APIError`. + /// + /// The transport now raises the richer error so one code path serves both + /// entry points. Every caller that did not opt in must still see exactly + /// what it saw before — `catch let error as APIError` has to keep matching — + /// so the unwrap happens here rather than at 420 call sites. + private func unwrappingFailure( + _ work: () async throws -> T + ) async throws -> T { + do { + return try await work() + } catch let failure as APIFailure { + throw failure.underlyingError + } } public func sendRaw(_ request: Request) async throws -> (Data, String?) { - let (data, response) = try await performWithSafetyNet(request) + let (data, response) = try await unwrappingFailure { try await performWithSafetyNet(request) } let contentType = response.value(forHTTPHeaderField: "Content-Type") return (data, contentType) } @@ -134,7 +213,7 @@ public final class APIClient: APIClientProtocol { public func sendWithRateLimitInfo( _ request: Request ) async throws -> (Response, RateLimitInfo?) { - let (data, response) = try await performWithSafetyNet(request) + let (data, response) = try await unwrappingFailure { try await performWithSafetyNet(request) } do { let decoded = try decoder.decode(Response.self, from: data) // RateLimitInfo.parse returns nil when headers are absent — @@ -165,15 +244,22 @@ public final class APIClient: APIClientProtocol { ) async throws -> (Data, HTTPURLResponse) { do { return try await performWithRetry(request, forceSession: false) - } catch let error as APIError { + } catch let failure as APIFailure { // Safety net: a Bearer request that comes back 401 should // transparently try once via the session transport before we // give up. This catches future API drift in either direction. - if case .unauthorized = error, request.auth == .bearer { + // + // Matched on `underlyingError`, because the transport now raises + // `APIFailure` so the response body survives to a caller that asked + // for it (GitHub #103). The `APIFailure` is rethrown intact rather + // than flattened — flattening here would drop the body before + // `sendCapturingFailure` ever saw it, which is the entire point of + // the type. + if case .unauthorized = failure.underlyingError, request.auth == .bearer { appLog.warning("Bearer request returned 401 [\(request.path)] — retrying via session transport") return try await performWithRetry(request, forceSession: true) } - throw error + throw failure } } @@ -185,13 +271,16 @@ public final class APIClient: APIClientProtocol { while true { do { return try await performOnce(request, forceSession: forceSession) - } catch let error as APIError { - if let delay = retryPolicy.delay(error, attempt) { + } catch let failure as APIFailure { + // Same reasoning as the safety net above: the retry policy asks + // about the `APIError`, and the `APIFailure` is rethrown whole so + // the body reaches whoever asked for it. + if let delay = retryPolicy.delay(failure.underlyingError, attempt) { attempt += 1 try await Task.sleep(nanoseconds: UInt64(delay * 1_000_000_000)) continue } - throw error + throw failure } } } @@ -231,11 +320,17 @@ public final class APIClient: APIClientProtocol { guard (200..<300).contains(response.statusCode) else { let serverMessage = decodeServerMessage(from: data) appLog.notice("HTTP \(response.statusCode) [\(request.path)]: \(serverMessage ?? "no server message")") - throw APIError.from( + let apiError = APIError.from( statusCode: response.statusCode, serverMessage: serverMessage, retryAfter: parseRetryAfter(response.value(forHTTPHeaderField: "Retry-After")) ) + // The body is kept alongside the error so `sendCapturingFailure` + // can hand it to a caller that asked for it (GitHub #103). + // `send(_:)` unwraps this back to a plain `APIError`, so every + // existing caller is unaffected — the richer error never leaks into + // a code path that did not opt in. + throw APIFailure(underlyingError: apiError, body: data) } return (data, response) } diff --git a/Packages/InterlinedKit/Sources/InterlinedKit/Errors/APIFailure.swift b/Packages/InterlinedKit/Sources/InterlinedKit/Errors/APIFailure.swift new file mode 100644 index 0000000..64f9623 --- /dev/null +++ b/Packages/InterlinedKit/Sources/InterlinedKit/Errors/APIFailure.swift @@ -0,0 +1,74 @@ +import Foundation + +/// An HTTP failure **with its response body intact** (GitHub #103). +/// +/// `APIError` keeps only the decoded `{error}` string; everything else in the +/// body is discarded before any caller sees it. That is fine for the +/// overwhelming majority of failures, where a sentence is all the UI needs — and +/// wrong for the handful where the server answers a *question* rather than +/// reporting a malfunction: +/// +/// - `PUT /api/lists/{id}/schema` refuses a destructive rebuild with `400` plus +/// a **`propertiesWithData` array naming the columns** that still hold data. +/// The UI should name them and offer the confirmation; without the body it can +/// only repeat the server's sentence. +/// - The app-settings family is compare-and-set: a stale `baseVersion` answers +/// `409` with the **`current` document attached**, so a client can show what +/// changed, merge, or re-base and retry. +/// +/// ## Why this is a separate type rather than a case on `APIError` +/// +/// `APIError`'s cases carry a single `serverMessage` associated value, and +/// **420 sites across 78 files** pattern-match or construct them. Adding a +/// second associated value is a mechanical change to every one of those, +/// including every test — enormous churn, and every line of it a chance to get +/// a case wrong, in service of two call sites. +/// +/// So this is **opt-in**: `sendCapturingFailure(_:)` throws an `APIFailure`, and +/// only the callers that actually need the body use it. Every other call site +/// keeps throwing and catching plain `APIError`, unchanged. +/// +/// - Important: an `APIFailure` is **not** an `APIError`, so `catch let error as +/// APIError` will not match it. That is deliberate — a caller opts into the +/// richer error by choosing the richer send. `underlyingError` and the +/// forwarded `localizedDescription` mean nothing is lost if it reaches generic +/// error-display code. +public struct APIFailure: Error, Sendable { + + /// The failure as `APIError` models it. Callers that only care about the + /// status or the message switch on this exactly as they would have. + public let underlyingError: APIError + + /// The raw response body, when the failure carried one. + /// + /// `nil` for a transport failure, which never had a body to keep. + public let body: Data? + + public init(underlyingError: APIError, body: Data?) { + self.underlyingError = underlyingError + self.body = body + } + + /// Decodes the body as `T`, or `nil` when it was absent or did not match. + /// + /// Returning `nil` rather than throwing is the point: a caller asking for + /// details is asking an **optional** question, and a decode failure here + /// must degrade to today's behaviour — a message with no details — rather + /// than masking the original error with a decoding one. The failure the + /// caller is handling is the HTTP failure, not this. + public func details(as type: T.Type) -> T? { + guard let body else { return nil } + return try? JSONCoders.makeDecoder().decode(T.self, from: body) + } + + /// The HTTP status, when there was one. + public var httpStatusCode: Int? { underlyingError.httpStatusCode } +} + +extension APIFailure: LocalizedError, CustomStringConvertible { + /// Forwarded, so an `APIFailure` that reaches generic error-display code + /// reads exactly as the `APIError` would have. Nothing regresses by opting + /// in to the richer send. + public var errorDescription: String? { underlyingError.errorDescription } + public var description: String { underlyingError.description } +} diff --git a/Packages/InterlinedKit/Tests/InterlinedKitTests/APIFailureTests.swift b/Packages/InterlinedKit/Tests/InterlinedKitTests/APIFailureTests.swift new file mode 100644 index 0000000..df4e0d8 --- /dev/null +++ b/Packages/InterlinedKit/Tests/InterlinedKitTests/APIFailureTests.swift @@ -0,0 +1,132 @@ +// APIFailureTests +// +// The opt-in error carrier (GitHub #103). +// +// `APIError` keeps only the decoded `{error}` string, which is enough for almost +// every failure and not enough for the few where the server answers a *question* +// — the list-schema destructive-change guard names the columns that still hold +// data, and the app-settings compare-and-set returns the current document. +// +// The most important property under test is the one that is easy to break by +// accident: **opting in must not change anything for callers that did not**. + +import XCTest +@testable import InterlinedKit + +final class APIFailureTests: XCTestCase { + + private let baseURL = URL(string: "https://example.test")! + + private func makeClient() -> (APIClient, StubHTTPDataTransport) { + let transport = StubHTTPDataTransport() + let auth = DefaultAuthTransport( + tokenStore: InMemoryTokenStore(initial: "il_tok_test"), + sessionTransport: StubHTTPDataTransport(), + sessionEstablisher: NullSessionEstablisher() + ) + return (APIClient(baseURL: baseURL, transport: transport, authTransport: auth), transport) + } + + private struct Probe: Decodable, Sendable { let ok: Bool } + + /// The shape the list-schema guard actually answers with. + private struct SchemaConflict: Decodable { let propertiesWithData: [String]? } + + // MARK: - Happy path + + func test_givenAFailureWithABody_whenCapturing_thenTheDetailsDecode() async throws { + let (client, transport) = makeClient() + await transport.enqueue(.json(#"{"error":"Removing these columns would delete data.","code":"bad_request","propertiesWithData":["year","status"]}"#, status: 400)) + + do { + _ = try await client.sendCapturingFailure(Request(method: .get, path: "/probe", auth: .bearer)) + XCTFail("expected a failure") + } catch let failure as APIFailure { + XCTAssertEqual(failure.httpStatusCode, 400) + XCTAssertEqual( + failure.details(as: SchemaConflict.self)?.propertiesWithData, + ["year", "status"], + "the structured half of the body is what this type exists for" + ) + } + } + + func test_givenAFailure_whenCapturing_thenTheMessageIsUnchangedFromAPIError() async throws { + // Nothing regresses by opting in: an `APIFailure` that reaches generic + // error-display code has to read exactly as the `APIError` would have. + let (client, transport) = makeClient() + await transport.enqueue(.json(#"{"error":"title required"}"#, status: 400)) + + do { + _ = try await client.sendCapturingFailure(Request(method: .get, path: "/probe", auth: .bearer)) + XCTFail("expected a failure") + } catch let failure as APIFailure { + XCTAssertEqual(failure.underlyingError, .badRequest(serverMessage: "title required")) + XCTAssertEqual(failure.localizedDescription, APIError.badRequest(serverMessage: "title required").localizedDescription) + } + } + + // MARK: - The property that must not break + + func test_givenTheSameFailure_whenSentNormally_thenItIsStillAPlainAPIError() async throws { + // The transport raises `APIFailure` internally now so one code path + // serves both entry points. Every caller that did not opt in must still + // match `catch let error as APIError` — there are 420 such sites. + let (client, transport) = makeClient() + await transport.enqueue(.json(#"{"error":"title required","propertiesWithData":["year"]}"#, status: 400)) + + do { + _ = try await client.send(Request(method: .get, path: "/probe", auth: .bearer)) + XCTFail("expected a failure") + } catch let error as APIError { + XCTAssertEqual(error, .badRequest(serverMessage: "title required")) + } catch { + XCTFail("send(_:) must throw APIError, not \(type(of: error))") + } + } + + // MARK: - Invalid / absent details + + func test_givenABodyThatDoesNotMatch_whenDecodingDetails_thenItDegradesToNil() async throws { + // A caller asking for details is asking an *optional* question. A decode + // failure here must not mask the HTTP failure it is handling. + let (client, transport) = makeClient() + await transport.enqueue(.json(#"{"error":"nope"}"#, status: 400)) + + do { + _ = try await client.sendCapturingFailure(Request(method: .get, path: "/probe", auth: .bearer)) + XCTFail("expected a failure") + } catch let failure as APIFailure { + XCTAssertNil(failure.details(as: SchemaConflict.self)?.propertiesWithData) + XCTAssertEqual(failure.underlyingError, .badRequest(serverMessage: "nope")) + } + } + + func test_givenNoBodyAtAll_whenDecodingDetails_thenItIsNil() { + // A transport failure never had a body to keep. + let failure = APIFailure(underlyingError: .transport(message: "offline"), body: nil) + XCTAssertNil(failure.details(as: SchemaConflict.self)) + XCTAssertNil(failure.httpStatusCode) + } + + // MARK: - Boundary — a decode failure is not an HTTP failure + + func test_givenAnUndecodableSuccess_whenCapturing_thenItIsAPlainDecodingError() async throws { + // A 200 whose body does not match is a client-side decode problem with + // no server body to offer, so it surfaces as the `APIError.decoding` it + // has always been rather than an `APIFailure` carrying nothing useful. + let (client, transport) = makeClient() + await transport.enqueue(.json(#"{"unexpected":true}"#)) + + do { + _ = try await client.sendCapturingFailure(Request(method: .get, path: "/probe", auth: .bearer)) + XCTFail("expected a failure") + } catch let error as APIError { + guard case .decoding = error else { + return XCTFail("expected .decoding, got \(error)") + } + } catch { + XCTFail("expected APIError.decoding, got \(type(of: error))") + } + } +}