diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index 8fbedc2f..a97d05e5 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -910,9 +910,12 @@ final class AccountsManager { // A server that cannot be reached says nothing about the credential. // Recorded all the same: the account has to stop reporting itself as // usable, or reads never fall back and the user is shown failures - // instead of local data. + // instead of local data. A host that *refuses* the connection counts: + // it is as unreachable as one that never answers, and being told so + // instantly rather than after a timeout does not make it a different + // verdict. switch error { - case APIError.timeout, APIError.noConnection: + case APIError.timeout, APIError.noConnection, APIError.connectionFailed: setStatus(.unreachable, for: account.id) case APIError.cancelled: return diff --git a/Yattee/Core/InstancesManager.swift b/Yattee/Core/InstancesManager.swift index 2999b780..f2afecdf 100644 --- a/Yattee/Core/InstancesManager.swift +++ b/Yattee/Core/InstancesManager.swift @@ -332,7 +332,7 @@ final class InstancesManager { switch apiError { case .unauthorized: updateStatus(.authFailed, for: instance) - case .noConnection, .timeout: + case .noConnection, .timeout, .connectionFailed: updateStatus(.offline, for: instance) default: // Don't change status for other errors diff --git a/Yattee/Services/Credentials/CredentialProbeService.swift b/Yattee/Services/Credentials/CredentialProbeService.swift index 6c6f6564..5a575ae2 100644 --- a/Yattee/Services/Credentials/CredentialProbeService.swift +++ b/Yattee/Services/Credentials/CredentialProbeService.swift @@ -222,7 +222,10 @@ final class CredentialProbeService { return .rejected case let APIError.httpError(statusCode, _) where statusCode == 401 || statusCode == 403: return .rejected - case APIError.timeout, APIError.noConnection: + case APIError.timeout, APIError.noConnection, APIError.connectionFailed: + // A refused connection or an unresolvable host answers the probe no + // better than silence does: the question never reached anything + // that could judge the credential. return .unreachable default: // Cancellation included: a probe that was torn down never asked the diff --git a/Yattee/Services/Networking/APIError.swift b/Yattee/Services/Networking/APIError.swift index 0927b65b..42d7991e 100644 --- a/Yattee/Services/Networking/APIError.swift +++ b/Yattee/Services/Networking/APIError.swift @@ -24,6 +24,16 @@ enum APIError: Error, LocalizedError, Equatable, Sendable { /// No network connection available. case noConnection + /// The device is on a network, but the server could not be reached at all: + /// the connection was refused, or the host does not resolve. + /// + /// Distinct from `noConnection`, which says the *device* is offline. A + /// refusing or unresolvable host is one server's problem, and everything + /// that reacts to it — falling back to local data, retrying later — has to + /// stay scoped to that server. Carries the system's own description so the + /// user is told which of the two it was. + case connectionFailed(String) + /// The request was cancelled. case cancelled @@ -72,6 +82,12 @@ enum APIError: Error, LocalizedError, Equatable, Sendable { return "Request timed out" case .noConnection: return "No network connection" + case .connectionFailed(let message): + // Already the system's localized sentence ("Could not connect to + // the server.", "A server with the specified hostname could not be + // found."), which names the failure more precisely than a single + // fixed string could. + return message case .cancelled: return "Request was cancelled" case .serverError(let message): @@ -106,7 +122,11 @@ enum APIError: Error, LocalizedError, Equatable, Sendable { /// Whether this error is likely recoverable by retrying. var isRetryable: Bool { switch self { - case .timeout, .noConnection, .rateLimited: + // A refused connection or a failed lookup is the same kind of answer as + // a timeout: nothing about the request was wrong, the server simply was + // not there for that moment. Restarts and DNS hiccups are exactly what + // a second attempt exists for. + case .timeout, .noConnection, .connectionFailed, .rateLimited: return true case .httpError(let statusCode, _): return statusCode >= 500 || statusCode == 429 @@ -136,6 +156,8 @@ enum APIError: Error, LocalizedError, Equatable, Sendable { return lMsg == rMsg case (.serverError(let lMsg), .serverError(let rMsg)): return lMsg == rMsg + case (.connectionFailed(let lMsg), .connectionFailed(let rMsg)): + return lMsg == rMsg case (.rateLimited(let lRetry), .rateLimited(let rRetry)): return lRetry == rRetry case (.unknown(let lMsg), .unknown(let rMsg)): diff --git a/Yattee/Services/Networking/HTTPClient.swift b/Yattee/Services/Networking/HTTPClient.swift index 5f4a109e..bda3438e 100644 --- a/Yattee/Services/Networking/HTTPClient.swift +++ b/Yattee/Services/Networking/HTTPClient.swift @@ -280,6 +280,14 @@ actor HTTPClient { return .timeout case .notConnectedToInternet, .networkConnectionLost: return .noConnection + case .cannotConnectToHost, .cannotFindHost, .dnsLookupFailed: + // A server that refuses the connection, or a name that does not + // resolve, used to arrive as `unknown` — which nothing classifies, + // so an account pointed at a downed instance never went + // `unreachable`, never fell back to local data and never got its + // retry. It is not the same failure as a timeout, but it deserves + // the same treatment. + return .connectionFailed(error.localizedDescription) case .cancelled: return .cancelled default: diff --git a/Yattee/Services/SubscriptionFeedCache.swift b/Yattee/Services/SubscriptionFeedCache.swift index 1127e161..780afef6 100644 --- a/Yattee/Services/SubscriptionFeedCache.swift +++ b/Yattee/Services/SubscriptionFeedCache.swift @@ -37,6 +37,31 @@ enum FeedLoadState: Equatable { return error } + /// Whether a feed left in this state should keep asking again on its own. + /// + /// A network failure is its own reason to try again. `yatteeServerRequired` + /// usually is not — an account that genuinely needs a Yattee Server goes on + /// needing one, and a timer there would never end and never change + /// anything. The exception is the local fallback standing in for a server + /// that merely could not be reached: the feed is asking for a Yattee Server + /// only because reads moved to local data, and they move back the moment + /// the server answers again — silently, on a status that expires with + /// nothing on this screen watching for it. + /// + /// - Parameter unavailability: why the selected context is not the one + /// answering, or `nil` when it is — local data selected by the user is + /// never unavailable, so its Yattee Server request is the real answer. + func warrantsPeriodicRetry(unavailability: DataContextUnavailability?) -> Bool { + switch self { + case .error(.networkError): + return true + case .error(.yatteeServerRequired): + return unavailability == .unreachable + default: + return false + } + } + /// Specific error types for feed loading. enum FeedLoadError: Equatable { case yatteeServerRequired diff --git a/Yattee/Services/Subscriptions/DataContextValidator.swift b/Yattee/Services/Subscriptions/DataContextValidator.swift index 8f86910b..c6707374 100644 --- a/Yattee/Services/Subscriptions/DataContextValidator.swift +++ b/Yattee/Services/Subscriptions/DataContextValidator.swift @@ -100,9 +100,10 @@ final class DataContextValidator { } // The account's own status says nothing about the instance being - // switched off, and `availableContexts` already hides disabled - // instances — a context that stayed valid here would keep sending - // requests to a server the user has turned off. + // switched off, and the switcher only *lists* a disabled source's + // contexts — it refuses to switch to them — so a context that + // stayed valid here would keep sending requests to a server the + // user has turned off. guard instance.isEnabled else { return .temporarilyUnavailable(.instanceDisabled) } @@ -145,17 +146,62 @@ final class DataContextValidator { // MARK: - Available contexts + /// Every context the switcher *lists*, usable or not. + /// + /// A source the user switched off does not make its logins disappear. The + /// switcher listing only what can be switched to meant the badge on the + /// toolbar icon pointed at a menu the account had dropped out of — the + /// selected account included, so the one control that explains the badge + /// stopped naming the thing it was badging for. The rows stay, carrying + /// their status, and `isSwitchable(_:)` decides which of them can be picked. + /// + /// The selection is the row that may never be missing, so it is appended + /// even when it is no longer among the selectable accounts — as long as the + /// account still exists. A context whose account is gone is not listed: + /// there is nothing to name it with, and correction handles deletions. + var listedContexts: [DataContext] { + var contexts: [DataContext] = [.local] + contexts += accountsManager.selectableAccounts.map { DataContext.account($0.id) } + + let current = accountsManager.currentContext + if case let .account(id) = current, + accountsManager.account(id: id) != nil, + !contexts.contains(current) { + contexts.append(current) + } + + return contexts + } + /// Every context the user can switch to. /// /// Includes accounts that are currently signed out: hiding them would make /// a temporarily unreachable account look deleted, and the switcher is - /// where the user goes to fix exactly that. + /// where the user goes to fix exactly that. A disabled source is the one + /// exception — nothing may be read from or written to a server the user has + /// turned off — and it is expressed by refusing the switch, not by hiding + /// the row. var availableContexts: [DataContext] { - let accountContexts = accountsManager.selectableAccounts - .filter { accountsManager.instance(for: $0)?.isEnabled ?? false } - .map { DataContext.account($0.id) } + listedContexts.filter { isSwitchable($0) } + } - return [.local] + accountContexts + /// Whether picking this context would land the user somewhere usable. + /// + /// Not the same question as `validity(of:)`: a rejected or unreachable + /// account is still a perfectly good thing to switch *to* — that is where + /// the user goes to fix it — while a login on a switched-off source is not. + func isSwitchable(_ context: DataContext) -> Bool { + switch context { + case .local: + return true + + case let .account(id): + guard accountsManager.selectableAccounts.contains(where: { $0.id == id }), + let account = accountsManager.account(id: id) else { + return false + } + return accountsManager.instance(for: account)?.isEnabled ?? false + } } // MARK: - Correction diff --git a/Yattee/Views/Channel/ChannelView.swift b/Yattee/Views/Channel/ChannelView.swift index 3460bc05..9f23bd50 100644 --- a/Yattee/Views/Channel/ChannelView.swift +++ b/Yattee/Views/Channel/ChannelView.swift @@ -1292,7 +1292,11 @@ struct ChannelView: View { } } } label: { - Image(systemName: channelMenuIcon) + // A bare Image leaves the menu's accessibility title as the raw SF + // Symbol name; a Label carries the localized title to the AX bridge + // while `.iconOnly` keeps the toolbar rendering icon-only. + Label(String(localized: "channel.menu.title"), systemImage: channelMenuIcon) + .labelStyle(.iconOnly) } .accessibilityLabel(Text(String(localized: "channel.menu.title"))) } diff --git a/Yattee/Views/Components/DataContextSwitcherMenu.swift b/Yattee/Views/Components/DataContextSwitcherMenu.swift index 338f2a03..fc768106 100644 --- a/Yattee/Views/Components/DataContextSwitcherMenu.swift +++ b/Yattee/Views/Components/DataContextSwitcherMenu.swift @@ -22,7 +22,7 @@ struct DataContextSwitcherRows: View { private var validator: DataContextValidator? { appEnvironment?.dataContextValidator } var body: some View { - ForEach(validator?.availableContexts ?? [.local], id: \.self) { context in + ForEach(validator?.listedContexts ?? [.local], id: \.self) { context in row(for: context) } } @@ -31,7 +31,10 @@ struct DataContextSwitcherRows: View { private func row(for context: DataContext) -> some View { // A menu row is a single label — a `VStack` inside one renders as // nothing on macOS — so the status travels in the title rather than as - // a second line. + // a second line. A context that cannot be switched to says so there + // too: `title(for:in:)` appends the *display* status, which is what + // turns a login on a switched-off source into "… — Source switched + // off" rather than a row that looks ordinary and does nothing. let title = DataContextSwitcherLabels.title(for: context, in: appEnvironment) #if os(macOS) @@ -48,6 +51,7 @@ struct DataContextSwitcherRows: View { )) { Text(title) } + .disabled(isRefused(context)) #else Button { switchTo(context) @@ -58,6 +62,7 @@ struct DataContextSwitcherRows: View { Text(title) } } + .disabled(isRefused(context)) #if os(tvOS) .buttonStyle(TVSettingsButtonStyle()) #endif @@ -65,6 +70,10 @@ struct DataContextSwitcherRows: View { } private func switchTo(_ context: DataContext) { + // The row is disabled as well; this is what makes the refusal true on + // any presentation where a disabled control can still be activated. + guard !isRefused(context) else { return } + appEnvironment?.switchDataContext(to: context) onSwitch?() } @@ -72,6 +81,18 @@ struct DataContextSwitcherRows: View { private func isCurrent(_ context: DataContext) -> Bool { appEnvironment?.accountsManager.currentContext == context } + + /// Listed, but not something the user may pick. + /// + /// The current context is never refused, whatever its source is doing: it + /// is already the selection, picking it changes nothing, and a disabled + /// macOS menu item is not the place to hang the app's only checkmark. It + /// carries its status in the title instead, which is the part that has to + /// survive. + private func isRefused(_ context: DataContext) -> Bool { + guard let validator, !isCurrent(context) else { return false } + return !validator.isSwitchable(context) + } } // MARK: - Labels @@ -352,23 +373,13 @@ struct DataContextSwitcherSettingsRow: View { // trailing name was dropped at every width, leaving the row silent // about which account is selected. So the name is the menu's *own* // label, and the row carries only what names the control. - LabeledContent { - Menu { - DataContextSwitcherRows() - } label: { - Text(contextTitle) - .lineLimit(1) - } - .menuStyle(.borderlessButton) - .help(contextTitle) - .accessibilityLabel(Text(String(localized: "accounts.switcher.title"))) - .accessibilityValue(Text(contextTitle)) - } label: { - Label { - Text(String(localized: "accounts.switcher.title")) - } icon: { - DataContextSwitcherIcon() - } + // `LabeledContent` overflows rather than compress its label, so at the + // sidebar's 140 pt minimum a captioned row (caption + spacing + the + // name's floor ≈ 158 pt) juts past both edges. The caption is the one + // part the icon already says, so it is the part that goes. + ViewThatFits(in: .horizontal) { + macOSRow(captioned: true) + macOSRow(captioned: false) } #else Menu { @@ -379,11 +390,58 @@ struct DataContextSwitcherSettingsRow: View { #endif } + #if os(macOS) + @ViewBuilder private func macOSRow(captioned: Bool) -> some View { + LabeledContent { + Menu { + DataContextSwitcherRows() + } label: { + Text(contextTitle) + .lineLimit(1) + .truncationMode(.tail) + } + .menuStyle(.borderlessButton) + // The floor must sit on the menu itself: a borderless menu button + // lays out its label alone, so a frame inside it never reaches the + // row's layout, and the caption would keep its ideal width while + // the name shrank to an ellipsis. Out here the menu claims the + // width and the caption is what gives way. + .frame(minWidth: Self.minimumNameWidth, alignment: .trailing) + .layoutPriority(1) + .help(contextTitle) + .accessibilityLabel(Text(String(localized: "accounts.switcher.title"))) + .accessibilityValue(Text(contextTitle)) + } label: { + Label { + if captioned { + Text(String(localized: "accounts.switcher.title")) + .lineLimit(1) + .truncationMode(.tail) + } + } icon: { + DataContextSwitcherIcon() + } + } + } + #endif + + /// What the trailing name may never be squeezed below. + /// + /// A row this narrow — the macOS sidebar's footer at a dragged-in column + /// width — hands the leading label its ideal width first and leaves the + /// name whatever is left, and a `Text` will shrink all the way to a lone + /// ellipsis before it complains. A floor turns that around: the name keeps + /// enough characters to be recognised, and the word naming the control — + /// which the icon beside it already says — is what truncates instead. + private static let minimumNameWidth: CGFloat = 80 + #if !os(macOS) private var rowLabel: some View { HStack { Label { Text(String(localized: "accounts.switcher.title")) + .lineLimit(1) + .truncationMode(.tail) } icon: { DataContextSwitcherIcon() } @@ -391,6 +449,8 @@ struct DataContextSwitcherSettingsRow: View { Text(contextTitle) .foregroundStyle(.secondary) .lineLimit(1) + .truncationMode(.tail) + .frame(minWidth: Self.minimumNameWidth, alignment: .trailing) } } #endif diff --git a/Yattee/Views/Subscriptions/SubscriptionsView.swift b/Yattee/Views/Subscriptions/SubscriptionsView.swift index 7aba3bbb..82d4f5a7 100644 --- a/Yattee/Views/Subscriptions/SubscriptionsView.swift +++ b/Yattee/Views/Subscriptions/SubscriptionsView.swift @@ -26,8 +26,8 @@ struct SubscriptionsView: View { /// The effective context the visible subscriptions were loaded for, so a /// status change that does not move it does not force a reload. @State private var loadedContext: DataContext? - /// How many times the feed has been tried again since it last failed on the - /// network, so each attempt restarts the wait for the next one. + /// How many times the feed has been tried again since it last stopped on a + /// failure that can lift, so each attempt restarts the wait for the next one. @State private var feedRetryAttempt = 0 @State private var selectedChannelID: String? = nil @State private var errorMessage: String? @@ -195,8 +195,16 @@ struct SubscriptionsView: View { /// The failure the feed is currently sitting on, if it is one that trying /// again could answer differently. + /// + /// A network error always is. So is the Yattee Server the local feed asks + /// for while it is only standing in for an account whose server could not + /// be reached — that request is not the real answer, and the one that is + /// comes back on a status expiry nothing here is watching. An account that + /// genuinely has no Yattee Server never arms this: the unavailability it + /// gates on is `nil` for a context the user selected themselves. private var feedRetryToken: Int? { - guard case .error(.networkError(_)) = feedCache.feedLoadState else { return nil } + guard feedCache.feedLoadState.warrantsPeriodicRetry(unavailability: contextUnavailability) + else { return nil } return feedRetryAttempt } @@ -622,6 +630,11 @@ struct SubscriptionsView: View { // a feed that failed on the way to it is not watching anything // that would notice — so it asks again on its own, for as long // as the failure is on screen. + // + // One attempt in flight at a time, and none at all outside the + // states above: the id going nil, the fence dropping the state + // back to idle, and the view disappearing all cancel this task, + // and the next attempt is only armed by the last one finishing. .task(id: feedRetryToken) { guard feedRetryToken != nil else { return } try? await Task.sleep(for: .seconds(Self.feedRetryInterval)) diff --git a/YatteeTests/AccountsManagerTests.swift b/YatteeTests/AccountsManagerTests.swift index 4c35ec6a..7ffe2657 100644 --- a/YatteeTests/AccountsManagerTests.swift +++ b/YatteeTests/AccountsManagerTests.swift @@ -734,6 +734,35 @@ struct DataContextValidatorTests { #expect(validator.availableContexts == [.local, .account(usable.id)]) } + + @Test("A switched-off source keeps its rows in the switcher, refused rather than hidden") + func disabledSourceStaysListed() { + let instances = makeTestInstancesManager() + let enabled = Instance(type: .invidious, url: URL(string: "https://inv-listed-a.example.com")!) + var disabled = Instance(type: .invidious, url: URL(string: "https://inv-listed-b.example.com")!) + for instance in [enabled, disabled] { instances.add(instance) } + defer { for instance in [enabled, disabled] { instances.remove(instance) } } + + let (accounts, validator) = makeValidator(instances: instances) + let usable = accounts.addAccount(instanceID: enabled.id) + let switchedOff = accounts.addAccount(instanceID: disabled.id) + accounts.setStatus(.signedIn, for: switchedOff.id) + accounts.setContext(.account(switchedOff.id)) + + disabled.isEnabled = false + instances.update(disabled) + + // The menu still names every context, the selected one included: it is + // the control the badged toolbar icon sends the user to. + #expect(validator.listedContexts == [.local, .account(usable.id), .account(switchedOff.id)]) + // What it may not do is offer to switch to it. + #expect(validator.availableContexts == [.local, .account(usable.id)]) + #expect(!validator.isSwitchable(.account(switchedOff.id))) + #expect(validator.isSwitchable(.account(usable.id))) + #expect(validator.isSwitchable(.local)) + // And the row says why, in the same words the accounts screen uses. + #expect(accounts.displayStatus(for: switchedOff) == .instanceDisabled) + } } @Suite("Data context repair", .serialized) diff --git a/YatteeTests/CredentialProbeTests.swift b/YatteeTests/CredentialProbeTests.swift index c86064fe..2895218a 100644 --- a/YatteeTests/CredentialProbeTests.swift +++ b/YatteeTests/CredentialProbeTests.swift @@ -244,6 +244,36 @@ struct CredentialProbeTests { #expect(harness.status == .signedIn) } + @Test("A refused connection marks the account unreachable and lifts again") + func refusedConnectionMarksUnreachable() async throws { + let harness = ProbeHarness(unreachableRetryInterval: 0.05) + defer { harness.tearDown() } + + // A server that answers "no" instantly used to arrive as an unknown + // error, which nothing classified: the account went on calling itself + // signed in, reads never fell back and no retry was ever scheduled. + harness.record(APIError.connectionFailed("Could not connect to the server.")) + #expect(harness.status == .unreachable) + + // Nothing was learned about the credential, so it rides the same expiry. + try await Task.sleep(for: .seconds(0.5)) + #expect(harness.status == .signedIn) + + // And it is not an auth question: no probe was spent on it. + #expect(harness.transportCalls == 0) + } + + @Test("A host that does not resolve is unreachable too") + func unresolvableHostMarksUnreachable() async throws { + let harness = ProbeHarness() + defer { harness.tearDown() } + + harness.record( + APIError.connectionFailed("A server with the specified hostname could not be found.") + ) + #expect(harness.status == .unreachable) + } + // MARK: - What must not be probed @Test("An instance behind basic auth is never probed") @@ -463,6 +493,11 @@ struct CredentialProbeTests { #expect(CredentialProbeService.classify(APIError.timeout) == .unreachable) #expect(CredentialProbeService.classify(APIError.noConnection) == .unreachable) + // A host that refuses the connection, or does not resolve, answered + // nothing about the credential either. + #expect(CredentialProbeService.classify( + APIError.connectionFailed("Could not connect to the server.") + ) == .unreachable) // A probe that never completed answers nothing. #expect(CredentialProbeService.classify(APIError.cancelled) == .indeterminate) diff --git a/YatteeTests/NetworkingTests.swift b/YatteeTests/NetworkingTests.swift index a03e700f..0a7221ba 100644 --- a/YatteeTests/NetworkingTests.swift +++ b/YatteeTests/NetworkingTests.swift @@ -122,6 +122,18 @@ struct APIErrorTests { #expect(error.errorDescription == "Rate limited") } + @Test("Connection failed description is the system's own sentence") + func connectionFailedDescription() { + // Passed through rather than replaced: "could not connect" and "no such + // host" are different things to tell a user, and the OS already says + // which one happened. + let refused = APIError.connectionFailed("Could not connect to the server.") + #expect(refused.errorDescription == "Could not connect to the server.") + + // Emphatically not the offline message: the device's network is fine. + #expect(refused.errorDescription != APIError.noConnection.errorDescription) + } + @Test("Unknown error description") func unknownErrorDescription() { let error = APIError.unknown("Something went wrong") @@ -150,6 +162,10 @@ struct APIErrorTests { #expect(APIError.rateLimited(retryAfter: nil) == APIError.rateLimited(retryAfter: nil)) #expect(APIError.rateLimited(retryAfter: 30) != APIError.rateLimited(retryAfter: 60)) + #expect(APIError.connectionFailed("refused") == APIError.connectionFailed("refused")) + #expect(APIError.connectionFailed("refused") != APIError.connectionFailed("no such host")) + #expect(APIError.connectionFailed("refused") != APIError.noConnection) + #expect(APIError.unknown("msg") == APIError.unknown("msg")) #expect(APIError.unknown("msg1") != APIError.unknown("msg2")) @@ -179,6 +195,9 @@ struct APIErrorTests { func retryableErrors() { #expect(APIError.timeout.isRetryable == true) #expect(APIError.noConnection.isRetryable == true) + // A refused connection is a moment, not a verdict: the server may be + // restarting, the lookup may have missed. + #expect(APIError.connectionFailed("Could not connect to the server.").isRetryable == true) #expect(APIError.rateLimited(retryAfter: 60).isRetryable == true) #expect(APIError.httpError(statusCode: 500, message: nil).isRetryable == true) #expect(APIError.httpError(statusCode: 429, message: nil).isRetryable == true) @@ -287,6 +306,80 @@ struct URLBuildingTests { } } +// MARK: - URLError Mapping Tests + +/// Fails every request with a prepared `URLError`, so what a test measures is +/// the client's own mapping and nothing else. +private final class FailingURLProtocol: URLProtocol, @unchecked Sendable { + nonisolated(unsafe) static var failure = URLError(.cannotConnectToHost) + + override class func canInit(with request: URLRequest) -> Bool { true } + override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } + + override func startLoading() { + client?.urlProtocol(self, didFailWithError: Self.failure) + } + + override func stopLoading() {} +} + +@Suite("HTTPClient URLError mapping", .serialized) +struct URLErrorMappingTests { + + private func mappedError(for code: URLError.Code) async -> APIError? { + FailingURLProtocol.failure = URLError(code) + + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [FailingURLProtocol.self] + let client = HTTPClient(session: URLSession(configuration: configuration)) + + do { + _ = try await client.fetchData( + GenericEndpoint.get("/api/v1/stats"), + baseURL: URL(string: "https://refusing.example.com")! + ) + return nil + } catch let error as APIError { + return error + } catch { + return nil + } + } + + @Test("A server that refuses or cannot be found maps to connectionFailed") + func unreachableHostsMapToConnectionFailed() async { + // All three used to fall through to `unknown`, which no status + // classifier recognises — so an account pointed at a downed instance + // never went unreachable, never fell back to local data and never got + // its retry. + for code in [URLError.Code.cannotConnectToHost, .cannotFindHost, .dnsLookupFailed] { + let error = await mappedError(for: code) + guard case .connectionFailed = error else { + Issue.record("\(code) mapped to \(String(describing: error)), not connectionFailed") + continue + } + } + } + + @Test("The mapping still separates offline, timed out and cancelled") + func otherCodesKeepTheirMeaning() async { + #expect(await mappedError(for: .timedOut) == .timeout) + #expect(await mappedError(for: .notConnectedToInternet) == .noConnection) + #expect(await mappedError(for: .networkConnectionLost) == .noConnection) + #expect(await mappedError(for: .cancelled) == .cancelled) + } + + @Test("Unreachable hosts are recorded against the account, offline is too") + @MainActor + func mappedErrorsReachTheStatusClassifiers() async throws { + let error = try #require(await mappedError(for: .cannotConnectToHost)) + + // The whole point of the new case: both classifiers have to see it. + #expect(CredentialProbeService.classify(error) == .unreachable) + #expect(error.isRetryable) + } +} + // MARK: - HTTPMethod Tests @Suite("HTTPMethod Tests") diff --git a/YatteeTests/SubscriptionFeedErrorTests.swift b/YatteeTests/SubscriptionFeedErrorTests.swift index 3456c3a2..b064a288 100644 --- a/YatteeTests/SubscriptionFeedErrorTests.swift +++ b/YatteeTests/SubscriptionFeedErrorTests.swift @@ -53,6 +53,69 @@ struct SubscriptionFeedErrorTests { ) } + // MARK: - Asking again on a timer + + @Test("A network failure keeps trying whatever the context is doing") + func networkErrorAlwaysRetries() { + let state = FeedLoadState.error(.networkError("The request timed out.")) + let reasons: [DataContextUnavailability?] = [ + nil, .checking, .instancePending, .instanceDisabled, + .credentialMissing, .credentialRejected, .unreachable + ] + + for reason in reasons { + #expect(state.warrantsPeriodicRetry(unavailability: reason)) + } + } + + @Test("The Yattee Server the unreachable fallback asks for is worth asking again") + func yatteeServerRequiredRetriesWhileUnreachable() { + let state = FeedLoadState.error(.yatteeServerRequired) + + #expect(state.warrantsPeriodicRetry(unavailability: .unreachable)) + } + + @Test("A context that genuinely needs a Yattee Server never starts a timer") + func yatteeServerRequiredDoesNotRetryOtherwise() { + let state = FeedLoadState.error(.yatteeServerRequired) + // `nil` is local data the user selected: the request is the real answer. + // The rest lift through changes this view already reloads on, and none + // of them is the silent status expiry the retry exists for. + let reasons: [DataContextUnavailability?] = [ + nil, .checking, .instancePending, .instanceDisabled, + .credentialMissing, .credentialRejected + ] + + for reason in reasons { + #expect(!state.warrantsPeriodicRetry(unavailability: reason)) + } + } + + @Test("An account error is not answered by asking the feed again") + func notAuthenticatedNeverRetries() { + let state = FeedLoadState.error(.notAuthenticated) + + #expect(!state.warrantsPeriodicRetry(unavailability: .unreachable)) + #expect(!state.warrantsPeriodicRetry(unavailability: .credentialRejected)) + #expect(!state.warrantsPeriodicRetry(unavailability: nil)) + } + + @Test("A feed that is not sitting on an error has nothing to retry") + func nonErrorStatesNeverRetry() { + let states: [FeedLoadState] = [ + .idle, + .loading, + .loadingMore, + .ready, + .partiallyLoaded(readyCount: 1, pendingCount: 2, errorCount: 0) + ] + + for state in states { + #expect(!state.warrantsPeriodicRetry(unavailability: nil)) + #expect(!state.warrantsPeriodicRetry(unavailability: .unreachable)) + } + } + // MARK: - Other states @Test("States that are not errors have nothing to show")