diff --git a/Sources/KitchenSyncApp/Networking/DeviceFault.swift b/Sources/KitchenSyncApp/Networking/DeviceFault.swift new file mode 100644 index 0000000..8eb6fa8 --- /dev/null +++ b/Sources/KitchenSyncApp/Networking/DeviceFault.swift @@ -0,0 +1,35 @@ +import Foundation + +/// T-021: the ONE place that answers "what kind of failure was that?" for a device request. +/// +/// The transport layer throws `KitchenSyncClientError` / `DecodingError`; those types must not +/// leak into every view model's `catch`, and the three-way "answered-and-read / answered-but- +/// undecodable / didn't-answer" decision must not be re-derived per call site (that drift is the +/// recurring bug — T-009/T-010/T-016/T-018). Each view model maps this one fault onto its own +/// presentation enum; the classification lives here. +enum DeviceFault: Equatable { + /// The device ANSWERED, but the body was unreadable (`DecodingError`). It is there — its + /// firmware just speaks a different shape. The way out is a firmware update, not a network fix. + case undecodable + /// HTTP 404 — the route is absent on this (older) firmware. The confidently-attributable case. + case notFound + /// The device answered with another error status — it is right there and talking; this is + /// about the request/value, not the connection. + case rejected(Int) + /// No answer, or a transport-level failure. "Can't reach it", not "it said no". + case unreachable +} + +extension DeviceFault { + static func classify(_ error: Error) -> DeviceFault { + if error is DecodingError { return .undecodable } + if let e = error as? KitchenSyncClientError { + switch e { + case .httpStatus(404): return .notFound + case .httpStatus(let code): return .rejected(code) + case .invalidResponse: return .unreachable + } + } + return .unreachable + } +} diff --git a/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift b/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift index d1d6f37..fd444fb 100644 --- a/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift +++ b/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift @@ -96,14 +96,16 @@ final class DeviceDetailViewModel: ObservableObject { status = fresh statusAvailability = .available isRebooting = false // the device answering IS the signal - } catch is DecodingError { - // It's there. We just can't read it. - statusAvailability = .unsupported } catch { - status = nil - statusAvailability = .unreachable - // A failed poll while rebooting is the device still being down — which is - // exactly what we're waiting out. Only an ANSWER clears that flag. + switch DeviceFault.classify(error) { + case .undecodable: + statusAvailability = .unsupported // it's there; we just can't read it + case .notFound, .rejected, .unreachable: + status = nil + statusAvailability = .unreachable + // A failed poll while rebooting is the device still being down — which is + // exactly what we're waiting out. Only an ANSWER clears that flag. + } } } @@ -131,13 +133,14 @@ final class DeviceDetailViewModel: ObservableObject { do { config = try await client.fetchConfig() configAvailability = .available - } catch KitchenSyncClientError.httpStatus(404) { - // The one case we can attribute confidently. - config = nil - configAvailability = .unsupportedByFirmware } catch { config = nil - configAvailability = .failed + switch DeviceFault.classify(error) { + case .notFound: + configAvailability = .unsupportedByFirmware // the one case we attribute confidently + case .undecodable, .rejected, .unreachable: + configAvailability = .failed + } } } @@ -176,12 +179,15 @@ final class DeviceDetailViewModel: ObservableObject { try await client.postLive(edit) config?.apply(edit) liveEditFailure = nil - } catch KitchenSyncClientError.httpStatus(404) { - liveEditFailure = .unsupportedByFirmware - } catch KitchenSyncClientError.httpStatus { - liveEditFailure = .rejectedByDevice } catch { - liveEditFailure = .unreachable + switch DeviceFault.classify(error) { + case .notFound: + liveEditFailure = .unsupportedByFirmware + case .rejected: + liveEditFailure = .rejectedByDevice // it answered and refused — about the value + case .undecodable, .unreachable: + liveEditFailure = .unreachable + } } } diff --git a/Sources/KitchenSyncApp/ViewModels/DeviceListViewModel.swift b/Sources/KitchenSyncApp/ViewModels/DeviceListViewModel.swift index f82e0a2..94c4f20 100644 --- a/Sources/KitchenSyncApp/ViewModels/DeviceListViewModel.swift +++ b/Sources/KitchenSyncApp/ViewModels/DeviceListViewModel.swift @@ -129,10 +129,13 @@ final class DeviceListViewModel: ObservableObject { let client = KitchenSyncClient(host: device.host, session: session) do { return (device.id, .ok(try await client.fetchStatus())) - } catch is DecodingError { - return (device.id, .undecodable) } catch { - return (device.id, .noAnswer) + switch DeviceFault.classify(error) { + case .undecodable: + return (device.id, .undecodable) // it's THERE, just unreadable + case .notFound, .rejected, .unreachable: + return (device.id, .noAnswer) + } } } } diff --git a/Tests/KitchenSyncAppTests/DeviceFaultTests.swift b/Tests/KitchenSyncAppTests/DeviceFaultTests.swift new file mode 100644 index 0000000..533d640 --- /dev/null +++ b/Tests/KitchenSyncAppTests/DeviceFaultTests.swift @@ -0,0 +1,35 @@ +import XCTest +@testable import KitchenSyncApp + +/// T-021: the failure classifier, tested directly at its interface — feed it errors, assert the +/// fault. No view model, no `StubURLProtocol`; the decision is a pure function. +final class DeviceFaultTests: XCTestCase { + + // A DecodingError means the device ANSWERED but the body was unreadable — undecodable, + // NOT unreachable. Collapsing those two is the exact bug this classifier exists to prevent. + func test_decoding_error_is_undecodable() { + let err = DecodingError.dataCorrupted(.init(codingPath: [], debugDescription: "bad")) + XCTAssertEqual(DeviceFault.classify(err), .undecodable) + } + + // HTTP 404 — the route is absent on older firmware. The confidently-attributable case, + // kept apart from unreachable so a stale-firmware user isn't told to check their network. + func test_http_404_is_not_found() { + XCTAssertEqual(DeviceFault.classify(KitchenSyncClientError.httpStatus(404)), .notFound) + } + + // Any other error status: the device answered and refused — rejected, carrying the code. + func test_other_http_status_is_rejected_with_code() { + XCTAssertEqual(DeviceFault.classify(KitchenSyncClientError.httpStatus(500)), .rejected(500)) + } + + // A malformed/absent HTTP response is a transport-level failure, not an answer. + func test_invalid_response_is_unreachable() { + XCTAssertEqual(DeviceFault.classify(KitchenSyncClientError.invalidResponse), .unreachable) + } + + // Anything unrecognised (URLError and friends) means we never got a usable answer. + func test_unknown_error_is_unreachable() { + XCTAssertEqual(DeviceFault.classify(URLError(.notConnectedToInternet)), .unreachable) + } +} diff --git a/tasks/T-021.md b/tasks/T-021.md index fce24a5..648439c 100644 --- a/tasks/T-021.md +++ b/tasks/T-021.md @@ -1,7 +1,7 @@ --- id: T-021 title: One classifier for a failed device request, not four catch-ladders -status: todo +status: done assignee: null priority: high tags: @@ -38,18 +38,18 @@ fault onto its own presentation enum. ## Acceptance Criteria -- [ ] A pure function classifies a thrown request error into a single `DeviceFault` value -- [ ] All four call sites above translate a `DeviceFault` into their local enum — none re-inspect +- [x] A pure function classifies a thrown request error into a single `DeviceFault` value +- [x] All four call sites above translate a `DeviceFault` into their local enum — none re-inspect `KitchenSyncClientError` / `DecodingError` directly -- [ ] The classifier is tested **directly at its interface** (feed it errors, assert the fault) — +- [x] The classifier is tested **directly at its interface** (feed it errors, assert the fault) — no need to drive a whole view model through `StubURLProtocol` to exercise the decision -- [ ] The distinct-signals invariant is preserved: undecodable, unreachable and old-firmware stay +- [x] The distinct-signals invariant is preserved: undecodable, unreachable and old-firmware stay three separate outcomes (T-009, T-010, T-016, T-018 — do not re-collapse them) -- [ ] `DeviceListViewModel`'s consecutive-miss counting stays where it is — that is list policy, not +- [x] `DeviceListViewModel`'s consecutive-miss counting stays where it is — that is list policy, not classification, and only `noAnswer` feeds it -- [ ] Existing `DeviceDetailViewModelTests`, `LiveEditFeedbackTests`, `DeviceListViewModelTests`, +- [x] Existing `DeviceDetailViewModelTests`, `LiveEditFeedbackTests`, `DeviceListViewModelTests`, `UnsupportedDeviceTests` all still pass -- [ ] Written test-first per the /tdd rule +- [x] Written test-first per the /tdd rule ## Notes @@ -66,3 +66,12 @@ Source: architecture review 2026-07-14 (candidate 02). Pairs naturally with T-02 "a decision spread across call sites, give it one home." Do T-020 first (higher leverage). ## Progress + +**Done 2026-07-17 (TDD).** `Networking/DeviceFault.swift` — `DeviceFault.classify(Error) -> DeviceFault` +with `undecodable` / `notFound` / `rejected(Int)` / `unreachable`, tested directly at its interface +(`DeviceFaultTests`, 5 cases, no view model / StubURLProtocol). All four catch-ladders now translate +one fault into their own presentation enum and none re-inspect `KitchenSyncClientError` / +`DecodingError`: `refreshStatus`, `loadConfig`, `apply` (DeviceDetailViewModel) and `refreshAll` +(DeviceListViewModel). The three distinct signals (undecodable / old-firmware / unreachable) are +preserved; the list's consecutive-miss counting stays where it is (only `noAnswer` feeds it). Full +suite green: 148 tests, 0 failures.