From ff25f25adc4b0e060d0f420144a6b631d0b81bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Thu, 10 Jul 2025 14:50:41 +0100 Subject: [PATCH 01/12] Allow injection of a custom logger --- Source/HotwireConfig.swift | 11 +++++- Source/HotwireLogger.swift | 81 ++++++++++++++++++++++++++++++++++---- 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/Source/HotwireConfig.swift b/Source/HotwireConfig.swift index a25bcf4b..8c45e59b 100644 --- a/Source/HotwireConfig.swift +++ b/Source/HotwireConfig.swift @@ -26,7 +26,16 @@ public struct HotwireConfig { /// connecting, disconnecting, receiving/sending messages, and more. public var debugLoggingEnabled = false { didSet { - HotwireLogger.debugLoggingEnabled = debugLoggingEnabled + HotwireLogger.update(debugLoggingEnabled: debugLoggingEnabled, log: log) + } + } + + /// Inject your own logger here to override the default ``enabledLogger``. + /// This will be used only when ``debugLoggingEnabled`` + /// is set to `true`. + public var log: Logger? = nil { + didSet { + HotwireLogger.update(debugLoggingEnabled: debugLoggingEnabled, log: log) } } diff --git a/Source/HotwireLogger.swift b/Source/HotwireLogger.swift index 2b2dbb05..bf721908 100644 --- a/Source/HotwireLogger.swift +++ b/Source/HotwireLogger.swift @@ -1,15 +1,80 @@ import Foundation -import os.log +import OSLog -enum HotwireLogger { - static var debugLoggingEnabled: Bool = false { - didSet { - logger = debugLoggingEnabled ? enabledLogger : disabledLogger +public enum LogLevel { + case debug, info, warning, error + + var osLogLevel: OSLogType { + switch self { + case .debug: + return .debug + case .info: + return .info + case .warning: + return .error + case .error: + return .error } } +} + +public protocol Logger { + func log(message: String, level: LogLevel, file: String, function: String, line: UInt) + func debug(_ message: String, file: String, function: String, line: UInt) + func info(_ message: String, file: String, function: String, line: UInt) + func warning(_ message: String, file: String, function: String, line: UInt) + func error(_ message: String, file: String, function: String, line: UInt) +} + +public struct OSLogWrapper: Logger { + private let osLogger: os.Logger + + public init(subsystem: String, category: String) { + self.osLogger = os.Logger(subsystem: subsystem, category: category) + } + + public static var disabled: Logger { + OSLogWrapper(osLogger: os.Logger(.disabled)) + } + + public func log(message: String, level: LogLevel, file: String, function: String, line: UInt) { + osLogger.log(level: level.osLogLevel, "\(message)") + } + + private init(osLogger: os.Logger) { + self.osLogger = osLogger + } +} - static let enabledLogger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "Hotwire") - static let disabledLogger = Logger(.disabled) +extension Logger { + public func debug(_ message: String, file: String = #file, function: String = #function, line: UInt = #line) { + log(message: message, level: .debug, file: file, function: function, line: line) + } + + public func info(_ message: String, file: String = #file, function: String = #function, line: UInt = #line) { + log(message: message, level: .info, file: file, function: function, line: line) + } + + public func warning(_ message: String, file: String = #file, function: String = #function, line: UInt = #line) { + log(message: message, level: .warning, file: file, function: function, line: line) + } + + public func error(_ message: String, file: String = #file, function: String = #function, line: UInt = #line) { + log(message: message, level: .error, file: file, function: function, line: line) + } +} + +enum HotwireLogger { + static let enabledLogger = OSLogWrapper(subsystem: Bundle.module.bundleIdentifier!, category: "HotwireNative") + static let disabledLogger = OSLogWrapper.disabled + + static func update(debugLoggingEnabled: Bool, log: Logger?) { + if debugLoggingEnabled { + logger = log ?? enabledLogger + } else { + logger = disabledLogger + } + } } -var logger = HotwireLogger.disabledLogger +var logger: Logger = HotwireLogger.disabledLogger From c1825820daf4f8140dc47c8d37356f6cf0ba932e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Thu, 10 Jul 2025 19:38:09 +0100 Subject: [PATCH 02/12] Lower logging level for WebViewPolicyManager --- Source/Turbo/Navigator/WebViewPolicy/WebViewPolicyManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Turbo/Navigator/WebViewPolicy/WebViewPolicyManager.swift b/Source/Turbo/Navigator/WebViewPolicy/WebViewPolicyManager.swift index 10a8c62d..40652472 100644 --- a/Source/Turbo/Navigator/WebViewPolicy/WebViewPolicyManager.swift +++ b/Source/Turbo/Navigator/WebViewPolicy/WebViewPolicyManager.swift @@ -22,7 +22,7 @@ public final class WebViewPolicyManager { } } - logger.warning("[WebViewPolicyManager] no handler for navigation action: \(navigationAction)") + logger.info("[WebViewPolicyManager] no handler for navigation action: \(navigationAction)") return .allow } } From 6638de428eb772224edd55cd003e702f7b49958c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Thu, 10 Jul 2025 19:42:30 +0100 Subject: [PATCH 03/12] Additional logging --- Source/Turbo/Navigator/Navigator.swift | 27 +++++++++++++++---- .../AppNavigationRouteDecisionHandler.swift | 1 + .../ReloadWebViewPolicyDecisionHandler.swift | 1 + Source/Turbo/Session/Session.swift | 6 ++++- .../Visitable/VisitableViewController.swift | 6 +++++ 5 files changed, 35 insertions(+), 6 deletions(-) diff --git a/Source/Turbo/Navigator/Navigator.swift b/Source/Turbo/Navigator/Navigator.swift index 505119f8..15df558d 100644 --- a/Source/Turbo/Navigator/Navigator.swift +++ b/Source/Turbo/Navigator/Navigator.swift @@ -54,6 +54,7 @@ public class Navigator { return } + logger.info("Navigator starting at \(configuration.startLocation.absoluteString)") route(configuration.startLocation) } @@ -78,6 +79,7 @@ public class Navigator { } guard let controller = controller(for: proposal) else { return } + logger.info("Routing \(proposal.url.absoluteString)") hierarchyController.route(controller: controller, proposal: proposal) } @@ -263,6 +265,7 @@ extension Navigator: WKUIControllerDelegate { extension Navigator { private func inspectAllSessions() { + logger.info("Inspecting sessions") [session, modalSession].forEach { inspect($0) } } @@ -276,9 +279,13 @@ extension Navigator { /// side-effects for the next visit (like showing the wrong bridge components). We can't just /// check if the view controller is visible, since it may be further back in the stack of a navigation controller. /// Seeing if there is a parent was the best solution I could find. - guard let viewController = session.activeVisitable?.visitableViewController, - viewController.parent != nil - else { + guard let viewController = session.activeVisitable?.visitableViewController else { + logger.info("Skipping session reload: no visitableViewController found") + return + } + + guard viewController.parent != nil else { + logger.info("Skipping session reload: no visitableViewControlle parent found") return } @@ -287,6 +294,7 @@ extension Navigator { // and reload it when the app is back in foreground. if appLifecycleObserver.appState == .background { if !backgroundTerminatedWebViewSessions.contains(where: { $0 === session }) { + logger.info("Skipping session reload: app in background") backgroundTerminatedWebViewSessions.append(session) } return @@ -310,16 +318,21 @@ extension Navigator { private func inspect(_ session: Session) { if let index = backgroundTerminatedWebViewSessions.firstIndex(where: { $0 === session }) { backgroundTerminatedWebViewSessions.remove(at: index) + logger.debug("Reloading background terminated web view") reload(session) return } guard let _ = session.topmostVisitable?.initialVisitableURL else { + logger.debug("Skipping inspection: no topmostVisitable found") return } session.webView.queryWebContentProcessState { [weak self] state in - guard case .terminated = state else { return } + guard case .terminated = state else { + logger.debug("Skipping webview recreation: no topmostVisitable found") + return + } self?.recreateWebView(for: session) } } @@ -329,8 +342,12 @@ extension Navigator { /// - Parameter session: The session to recreate. private func recreateWebView(for session: Session) { guard let _ = session.activeVisitable?.visitableViewController, - let url = session.activeVisitable?.initialVisitableURL else { return } + let url = session.activeVisitable?.initialVisitableURL else { + logger.debug("Skipping web view recreation: no initialVisitableURL found") + return + } + logger.debug("Recreating web view for \(url.absoluteString)") let newSession = Session(webView: Hotwire.config.makeWebView()) newSession.pathConfiguration = session.pathConfiguration newSession.delegate = self diff --git a/Source/Turbo/Navigator/Routing/Handlers/AppNavigationRouteDecisionHandler.swift b/Source/Turbo/Navigator/Routing/Handlers/AppNavigationRouteDecisionHandler.swift index b5f92b63..b7f0047c 100644 --- a/Source/Turbo/Navigator/Routing/Handlers/AppNavigationRouteDecisionHandler.swift +++ b/Source/Turbo/Navigator/Routing/Handlers/AppNavigationRouteDecisionHandler.swift @@ -17,6 +17,7 @@ public final class AppNavigationRouteDecisionHandler: RouteDecisionHandler { public func handle(location: URL, configuration: Navigator.Configuration, navigator: Navigator) -> Router.Decision { + logger.info("Routing \(location.absoluteString)") return .navigate } } diff --git a/Source/Turbo/Navigator/WebViewPolicy/Handlers/ReloadWebViewPolicyDecisionHandler.swift b/Source/Turbo/Navigator/WebViewPolicy/Handlers/ReloadWebViewPolicyDecisionHandler.swift index 4d77491e..1f0ea152 100644 --- a/Source/Turbo/Navigator/WebViewPolicy/Handlers/ReloadWebViewPolicyDecisionHandler.swift +++ b/Source/Turbo/Navigator/WebViewPolicy/Handlers/ReloadWebViewPolicyDecisionHandler.swift @@ -18,6 +18,7 @@ public struct ReloadWebViewPolicyDecisionHandler: WebViewPolicyDecisionHandler { public func handle(navigationAction: WKNavigationAction, configuration: Navigator.Configuration, navigator: Navigator) -> WebViewPolicyManager.Decision { + logger.info("ReloadWebViewPolicyDecisionHandler: Reloading at \(navigationAction.request.url?.absoluteString ?? "")") navigator.reload() return .cancel diff --git a/Source/Turbo/Session/Session.swift b/Source/Turbo/Session/Session.swift index 0e64ef0e..c8c34a97 100644 --- a/Source/Turbo/Session/Session.swift +++ b/Source/Turbo/Session/Session.swift @@ -81,8 +81,12 @@ public class Session: NSObject { } public func reload() { - guard let visitable = topmostVisitable else { return } + guard let visitable = topmostVisitable else { + log("Skipping session reload: no visitable found") + return + } + log("Reloading session with visitable: \(visitable)") initialized = false visit(visitable) topmostVisit = currentVisit diff --git a/Source/Turbo/Visitable/VisitableViewController.swift b/Source/Turbo/Visitable/VisitableViewController.swift index ff2a887c..df2032ce 100644 --- a/Source/Turbo/Visitable/VisitableViewController.swift +++ b/Source/Turbo/Visitable/VisitableViewController.swift @@ -72,6 +72,9 @@ open class VisitableViewController: UIViewController, Visitable { } open func visitableWillDeactivateWebView() { + let webViewURL = visitableView.webView?.url + let locationStateDescription = webViewURL != nil ? "webView's url: \(webViewURL!.absoluteString)" : "initialVisitableURL: \(initialVisitableURL.absoluteString)" + logger.info("visitableWillDeactivateWebView: setting location state to \(locationStateDescription)") visitableLocationState = .deactivated(visitableView.webView?.url ?? initialVisitableURL) } @@ -109,6 +112,9 @@ open class VisitableViewController: UIViewController, Visitable { private func resolveVisitableLocation() -> URL { switch visitableLocationState { case .resolved: + if visitableView.webView?.url == nil { + logger.info("Resolving visitable location to initialVisitableURL: \(initialVisitableURL.absoluteString)") + } return visitableView.webView?.url ?? initialVisitableURL case .initialized(let url): return url From cac484f03e2df82a9ebbbd376342d0a5c294f842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Tue, 17 Feb 2026 18:13:01 -0500 Subject: [PATCH 04/12] Update to allow retries for offline and timeout errors --- Source/Turbo/Errors/HotwireNativeError.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Turbo/Errors/HotwireNativeError.swift b/Source/Turbo/Errors/HotwireNativeError.swift index 0fe3d84c..6c9db0c6 100644 --- a/Source/Turbo/Errors/HotwireNativeError.swift +++ b/Source/Turbo/Errors/HotwireNativeError.swift @@ -45,8 +45,8 @@ public enum HotwireNativeError: LocalizedError, Equatable, Sendable { switch self { case .http(let error): return error.isRetryable - case .web(let error): - return !error.isOffline && !error.isTimeout + case .web: + return true case .load: return false } From de05e834953b8408c88409d033df84edf8f428ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Tue, 17 Feb 2026 18:23:32 -0500 Subject: [PATCH 05/12] Rename to JSFetchRecoveryHandler --- ...dler.swift => JSFetchRecoveryHandler.swift} | 18 +++++++++--------- Source/Turbo/Session/Session.swift | 6 +++--- ...swift => JSFetchRecoveryHandlerTests.swift} | 10 +++++----- 3 files changed, 17 insertions(+), 17 deletions(-) rename Source/Turbo/Networking/{RedirectHandler.swift => JSFetchRecoveryHandler.swift} (76%) rename Tests/Turbo/{RedirectHandlerTests.swift => JSFetchRecoveryHandlerTests.swift} (94%) diff --git a/Source/Turbo/Networking/RedirectHandler.swift b/Source/Turbo/Networking/JSFetchRecoveryHandler.swift similarity index 76% rename from Source/Turbo/Networking/RedirectHandler.swift rename to Source/Turbo/Networking/JSFetchRecoveryHandler.swift index a5d797e6..7874782f 100644 --- a/Source/Turbo/Networking/RedirectHandler.swift +++ b/Source/Turbo/Networking/JSFetchRecoveryHandler.swift @@ -1,6 +1,6 @@ import Foundation -enum RedirectHandlerError: LocalizedError { +enum JSFetchRecoveryError: LocalizedError { case requestFailed(Error) case responseValidationFailed(reason: ResponseValidationFailureReason) @@ -13,19 +13,19 @@ enum RedirectHandlerError: LocalizedError { var errorDescription: String? { switch self { case .requestFailed(let error): - return "Redirect resolution failed: \(error.localizedDescription)" + return "Failed to recover js fetch: \(error.localizedDescription)" case .responseValidationFailed(let reason): switch reason { case .missingURL: - return "Redirect resolution failed: missing URL" + return "Failed to validate js fetch resolution response: missing URL" case .invalidResponse: - return "Redirect resolution response invalid" + return "Failed to validate js fetch resolution response: response invalid" } } } } -struct RedirectHandler { +struct JSFetchRecoveryHandler { enum Result { case noRedirect case sameOriginRedirect(URL) @@ -40,7 +40,7 @@ struct RedirectHandler { let httpResponse = try validateResponse(response) guard let responseUrl = httpResponse.url else { - throw RedirectHandlerError.responseValidationFailed(reason: .missingURL) + throw JSFetchRecoveryError.responseValidationFailed(reason: .missingURL) } let isRedirect = location != responseUrl @@ -55,16 +55,16 @@ struct RedirectHandler { } return .sameOriginRedirect(responseUrl) - } catch let error as RedirectHandlerError { + } catch let error as JSFetchRecoveryError { throw error } catch { - throw RedirectHandlerError.requestFailed(error) + throw JSFetchRecoveryError.requestFailed(error) } } private func validateResponse(_ response: URLResponse) throws -> HTTPURLResponse { guard let httpResponse = response as? HTTPURLResponse else { - throw RedirectHandlerError.responseValidationFailed(reason: .invalidResponse) + throw JSFetchRecoveryError.responseValidationFailed(reason: .invalidResponse) } return httpResponse diff --git a/Source/Turbo/Session/Session.swift b/Source/Turbo/Session/Session.swift index c8c34a97..3fb7aa36 100644 --- a/Source/Turbo/Session/Session.swift +++ b/Source/Turbo/Session/Session.swift @@ -400,10 +400,10 @@ extension Session: WebViewDelegate { private func resolveRedirect(to location: URL, identifier: String, statusCode: Int) async { do { - let result = try await RedirectHandler().resolve(location: location) + let result = try await JSFetchRecoveryHandler().resolve(location: location) switch result { case .noRedirect: - // The server is reachable (RedirectHandler confirmed an HTTP response + // The server is reachable (JSFetchRecoveryHandler confirmed an HTTP response // with no redirect). The Turbo.js fetch failure was transient — // retry with a cold boot visit before showing an error. log("resolveRedirect: no redirect, retrying", @@ -433,7 +433,7 @@ extension Session: WebViewDelegate { visitIdentifier: identifier ) } - } catch let error as RedirectHandlerError { + } catch let error as JSFetchRecoveryError { let visitError: HotwireNativeError switch error { case .responseValidationFailed(reason: .missingURL), diff --git a/Tests/Turbo/RedirectHandlerTests.swift b/Tests/Turbo/JSFetchRecoveryHandlerTests.swift similarity index 94% rename from Tests/Turbo/RedirectHandlerTests.swift rename to Tests/Turbo/JSFetchRecoveryHandlerTests.swift index a9862334..49b2f896 100644 --- a/Tests/Turbo/RedirectHandlerTests.swift +++ b/Tests/Turbo/JSFetchRecoveryHandlerTests.swift @@ -3,8 +3,8 @@ import OHHTTPStubs import OHHTTPStubsSwift import XCTest -final class RedirectHandlerTests: XCTestCase { - private let handler = RedirectHandler() +final class JSFetchRecoveryHandlerTests: XCTestCase { + private let handler = JSFetchRecoveryHandler() private let testURL = URL(string: "https://example.com/page")! override func tearDown() { @@ -72,7 +72,7 @@ final class RedirectHandlerTests: XCTestCase { do { _ = try await handler.resolve(location: testURL) XCTFail("Expected requestFailed error") - } catch let error as RedirectHandlerError { + } catch let error as JSFetchRecoveryError { guard case .requestFailed = error else { return XCTFail("Expected .requestFailed, got \(error)") } @@ -123,8 +123,8 @@ final class RedirectHandlerTests: XCTestCase { // MARK: - Equatable for test assertions -extension RedirectHandler.Result: @retroactive Equatable { - public static func == (lhs: RedirectHandler.Result, rhs: RedirectHandler.Result) -> Bool { +extension JSFetchRecoveryHandler.Result: Equatable { + public static func == (lhs: JSFetchRecoveryHandler.Result, rhs: JSFetchRecoveryHandler.Result) -> Bool { switch (lhs, rhs) { case (.noRedirect, .noRedirect): return true From bc55f1eab0e42e7a4a702dad0de3bec1e66ed376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Thu, 19 Feb 2026 18:25:10 -0500 Subject: [PATCH 06/12] Update tests --- Tests/Turbo/HotwireNativeErrorTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Turbo/HotwireNativeErrorTests.swift b/Tests/Turbo/HotwireNativeErrorTests.swift index 8ba5b542..39457eff 100644 --- a/Tests/Turbo/HotwireNativeErrorTests.swift +++ b/Tests/Turbo/HotwireNativeErrorTests.swift @@ -179,11 +179,11 @@ final class HotwireNativeErrorTests: XCTestCase { } func test_isRetryable_web_offline_isFalse() { - XCTAssertFalse(HotwireNativeError.web(WebError(urlError: URLError(.notConnectedToInternet))).isRetryable) + XCTAssertTrue(HotwireNativeError.web(WebError(urlError: URLError(.notConnectedToInternet))).isRetryable) } func test_isRetryable_web_timeout_isFalse() { - XCTAssertFalse(HotwireNativeError.web(WebError(errorCode: -1, message: "Timeout")).isRetryable) + XCTAssertTrue(HotwireNativeError.web(WebError(errorCode: -1, message: "Timeout")).isRetryable) } func test_isRetryable_load_isFalse() { From ea979162fe3eec0d10166cc223fb42560d44c22e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Fri, 27 Feb 2026 17:11:27 -0500 Subject: [PATCH 07/12] Remove isRetryable logic and always provide clients with a retry handler --- Source/Turbo/Errors/HTTPError.swift | 22 ------ Source/Turbo/Errors/HotwireNativeError.swift | 14 ---- Source/Turbo/Navigator/Navigator.swift | 2 +- Tests/Turbo/HotwireNativeErrorTests.swift | 46 ------------ Tests/Turbo/HttpErrorTests.swift | 74 -------------------- 5 files changed, 1 insertion(+), 157 deletions(-) diff --git a/Source/Turbo/Errors/HTTPError.swift b/Source/Turbo/Errors/HTTPError.swift index 315de35f..94c90113 100644 --- a/Source/Turbo/Errors/HTTPError.swift +++ b/Source/Turbo/Errors/HTTPError.swift @@ -24,14 +24,6 @@ public enum HTTPError: LocalizedError, Equatable, Sendable { } } - /// Whether the HTTP error is transient and worth retrying. - public var isRetryable: Bool { - switch self { - case .client(let error): return error.isRetryable - case .server(let error): return error.isRetryable - } - } - /// Creates an HTTPError from an HTTP status code. /// Returns `nil` for status codes outside the 400-599 error range. public init?(statusCode: Int) { @@ -106,13 +98,6 @@ extension HTTPError { } } - public var isRetryable: Bool { - switch self { - case .requestTimeout, .tooManyRequests: return true - default: return false - } - } - public init(statusCode: Int) { switch statusCode { case 400: self = .badRequest @@ -172,13 +157,6 @@ extension HTTPError { } } - public var isRetryable: Bool { - switch self { - case .serviceUnavailable, .gatewayTimeout: return true - default: return false - } - } - public init(statusCode: Int) { switch statusCode { case 500: self = .internalServerError diff --git a/Source/Turbo/Errors/HotwireNativeError.swift b/Source/Turbo/Errors/HotwireNativeError.swift index 6c9db0c6..5bc4964a 100644 --- a/Source/Turbo/Errors/HotwireNativeError.swift +++ b/Source/Turbo/Errors/HotwireNativeError.swift @@ -38,20 +38,6 @@ public enum HotwireNativeError: LocalizedError, Equatable, Sendable { return nil } - /// Whether the error is recoverable by retrying the request. - /// HTTP 408/429 and 503/504 are retryable. Network-level timeouts, - /// offline errors, and configuration errors are not. - public var isRetryable: Bool { - switch self { - case .http(let error): - return error.isRetryable - case .web: - return true - case .load: - return false - } - } - /// Creates an error from a Turbo.js status code. /// - Positive status codes are HTTP errors /// - 0 = network failure, -1 = timeout, -2 = content type mismatch diff --git a/Source/Turbo/Navigator/Navigator.swift b/Source/Turbo/Navigator/Navigator.swift index 15df558d..5bd84c18 100644 --- a/Source/Turbo/Navigator/Navigator.swift +++ b/Source/Turbo/Navigator/Navigator.swift @@ -200,7 +200,7 @@ extension Navigator: SessionDelegate { } public func session(_ session: Session, didFailRequestForVisitable visitable: Visitable, error: HotwireNativeError) { - let retryHandler: (() -> Void)? = error.isRetryable ? { session.reload() } : nil + let retryHandler: (() -> Void)? = { session.reload() } delegate?.visitableDidFailRequest(visitable, error: error, retryHandler: retryHandler) } diff --git a/Tests/Turbo/HotwireNativeErrorTests.swift b/Tests/Turbo/HotwireNativeErrorTests.swift index 39457eff..d24e24f0 100644 --- a/Tests/Turbo/HotwireNativeErrorTests.swift +++ b/Tests/Turbo/HotwireNativeErrorTests.swift @@ -144,52 +144,6 @@ final class HotwireNativeErrorTests: XCTestCase { XCTAssertNil(HotwireNativeError.load(.notPresent).urlError) } - // MARK: - isRetryable - - func test_isRetryable_http_requestTimeout_isTrue() { - XCTAssertTrue(HotwireNativeError.http(.client(.requestTimeout)).isRetryable) - } - - func test_isRetryable_http_tooManyRequests_isTrue() { - XCTAssertTrue(HotwireNativeError.http(.client(.tooManyRequests)).isRetryable) - } - - func test_isRetryable_http_notFound_isFalse() { - XCTAssertFalse(HotwireNativeError.http(.client(.notFound)).isRetryable) - } - - func test_isRetryable_http_unauthorized_isFalse() { - XCTAssertFalse(HotwireNativeError.http(.client(.unauthorized)).isRetryable) - } - - func test_isRetryable_http_internalServerError_isFalse() { - XCTAssertFalse(HotwireNativeError.http(.server(.internalServerError)).isRetryable) - } - - func test_isRetryable_http_serviceUnavailable_isTrue() { - XCTAssertTrue(HotwireNativeError.http(.server(.serviceUnavailable)).isRetryable) - } - - func test_isRetryable_http_gatewayTimeout_isTrue() { - XCTAssertTrue(HotwireNativeError.http(.server(.gatewayTimeout)).isRetryable) - } - - func test_isRetryable_web_networkFailure_isTrue() { - XCTAssertTrue(HotwireNativeError.web(WebError(errorCode: 0, message: nil)).isRetryable) - } - - func test_isRetryable_web_offline_isFalse() { - XCTAssertTrue(HotwireNativeError.web(WebError(urlError: URLError(.notConnectedToInternet))).isRetryable) - } - - func test_isRetryable_web_timeout_isFalse() { - XCTAssertTrue(HotwireNativeError.web(WebError(errorCode: -1, message: "Timeout")).isRetryable) - } - - func test_isRetryable_load_isFalse() { - XCTAssertFalse(HotwireNativeError.load(.contentTypeMismatch).isRetryable) - } - // MARK: - errorDescription func test_errorDescription_forHttpError() { diff --git a/Tests/Turbo/HttpErrorTests.swift b/Tests/Turbo/HttpErrorTests.swift index ac43dbdf..6fc3860f 100644 --- a/Tests/Turbo/HttpErrorTests.swift +++ b/Tests/Turbo/HttpErrorTests.swift @@ -171,80 +171,6 @@ final class HTTPErrorTests: XCTestCase { XCTAssertEqual(HTTPError.ServerError.other(statusCode: 599).errorDescription, "Server Error (599)") } - // MARK: - ClientError isRetryable - - func test_clientError_requestTimeout_isRetryable() { - XCTAssertTrue(HTTPError.ClientError.requestTimeout.isRetryable) - } - - func test_clientError_tooManyRequests_isRetryable() { - XCTAssertTrue(HTTPError.ClientError.tooManyRequests.isRetryable) - } - - func test_clientError_badRequest_isNotRetryable() { - XCTAssertFalse(HTTPError.ClientError.badRequest.isRetryable) - } - - func test_clientError_unauthorized_isNotRetryable() { - XCTAssertFalse(HTTPError.ClientError.unauthorized.isRetryable) - } - - func test_clientError_forbidden_isNotRetryable() { - XCTAssertFalse(HTTPError.ClientError.forbidden.isRetryable) - } - - func test_clientError_notFound_isNotRetryable() { - XCTAssertFalse(HTTPError.ClientError.notFound.isRetryable) - } - - func test_clientError_other_isNotRetryable() { - XCTAssertFalse(HTTPError.ClientError.other(statusCode: 418).isRetryable) - } - - // MARK: - ServerError isRetryable - - func test_serverError_serviceUnavailable_isRetryable() { - XCTAssertTrue(HTTPError.ServerError.serviceUnavailable.isRetryable) - } - - func test_serverError_gatewayTimeout_isRetryable() { - XCTAssertTrue(HTTPError.ServerError.gatewayTimeout.isRetryable) - } - - func test_serverError_internalServerError_isNotRetryable() { - XCTAssertFalse(HTTPError.ServerError.internalServerError.isRetryable) - } - - func test_serverError_badGateway_isNotRetryable() { - XCTAssertFalse(HTTPError.ServerError.badGateway.isRetryable) - } - - func test_serverError_other_isNotRetryable() { - XCTAssertFalse(HTTPError.ServerError.other(statusCode: 599).isRetryable) - } - - // MARK: - HTTPError isRetryable Delegation - - func test_httpError_client_requestTimeout_isRetryable() { - XCTAssertTrue(HTTPError.client(.requestTimeout).isRetryable) - } - - func test_httpError_client_tooManyRequests_isRetryable() { - XCTAssertTrue(HTTPError.client(.tooManyRequests).isRetryable) - } - - func test_httpError_client_notFound_isNotRetryable() { - XCTAssertFalse(HTTPError.client(.notFound).isRetryable) - } - - func test_httpError_server_serviceUnavailable_isRetryable() { - XCTAssertTrue(HTTPError.server(.serviceUnavailable).isRetryable) - } - - func test_httpError_server_badGateway_isNotRetryable() { - XCTAssertFalse(HTTPError.server(.badGateway).isRetryable) - } - // MARK: - Helpers private func assertClientErrorRoundTrip( From 37e4e52ecaebcf4958eb60f296bdbc73724af383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Mon, 2 Mar 2026 10:45:27 -0500 Subject: [PATCH 08/12] Retry failed visit directly when no topmost visitable --- Source/Turbo/Navigator/Navigator.swift | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Source/Turbo/Navigator/Navigator.swift b/Source/Turbo/Navigator/Navigator.swift index 5bd84c18..6ba00490 100644 --- a/Source/Turbo/Navigator/Navigator.swift +++ b/Source/Turbo/Navigator/Navigator.swift @@ -200,7 +200,14 @@ extension Navigator: SessionDelegate { } public func session(_ session: Session, didFailRequestForVisitable visitable: Visitable, error: HotwireNativeError) { - let retryHandler: (() -> Void)? = { session.reload() } + let retryHandler: (() -> Void)? = { + if session.topmostVisitable == nil { + // Preserve reload semantics (force cold boot) when there is no topmost visitable yet. + session.visit(visitable, reload: true) + } else { + session.reload() + } + } delegate?.visitableDidFailRequest(visitable, error: error, retryHandler: retryHandler) } From 6ecdbafd26cce05d0c998fa550456201f6e23a51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Tue, 3 Mar 2026 10:11:18 -0500 Subject: [PATCH 09/12] Add logging to JSFetch recovery handler and error paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Log the resolve request, its outcome (no redirect, same-origin, or cross-origin), and—crucially—the error details in the catch blocks of Session.resolveRedirect which were previously silent. --- Source/Turbo/Networking/JSFetchRecoveryHandler.swift | 5 +++++ Source/Turbo/Session/Session.swift | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/Source/Turbo/Networking/JSFetchRecoveryHandler.swift b/Source/Turbo/Networking/JSFetchRecoveryHandler.swift index 7874782f..795c1461 100644 --- a/Source/Turbo/Networking/JSFetchRecoveryHandler.swift +++ b/Source/Turbo/Networking/JSFetchRecoveryHandler.swift @@ -33,6 +33,8 @@ struct JSFetchRecoveryHandler { } func resolve(location: URL) async throws -> Result { + logger.debug("[JSFetchRecoveryHandler] resolve: \(location.absoluteString)") + do { var request = URLRequest(url: location) request.timeoutInterval = 30 @@ -47,13 +49,16 @@ struct JSFetchRecoveryHandler { let redirectIsCrossOrigin = isRedirect && location.host != responseUrl.host guard isRedirect else { + logger.debug("[JSFetchRecoveryHandler] no redirect, status: \(httpResponse.statusCode)") return .noRedirect } if redirectIsCrossOrigin { + logger.debug("[JSFetchRecoveryHandler] cross-origin redirect to \(responseUrl.absoluteString)") return .crossOriginRedirect(responseUrl) } + logger.debug("[JSFetchRecoveryHandler] same-origin redirect to \(responseUrl.absoluteString)") return .sameOriginRedirect(responseUrl) } catch let error as JSFetchRecoveryError { throw error diff --git a/Source/Turbo/Session/Session.swift b/Source/Turbo/Session/Session.swift index 3fb7aa36..3273e270 100644 --- a/Source/Turbo/Session/Session.swift +++ b/Source/Turbo/Session/Session.swift @@ -434,6 +434,11 @@ extension Session: WebViewDelegate { ) } } catch let error as JSFetchRecoveryError { + log("resolveRedirect: recovery failed", + ["location": location, + "visitIdentifier": identifier, + "error": error.localizedDescription]) + let visitError: HotwireNativeError switch error { case .responseValidationFailed(reason: .missingURL), @@ -444,6 +449,11 @@ extension Session: WebViewDelegate { } await retryOrFailCurrentVisit(with: visitError, visitIdentifier: identifier) } catch { + log("resolveRedirect: unexpected error", + ["location": location, + "visitIdentifier": identifier, + "error": "\(error)"]) + await retryOrFailCurrentVisit( with: .web(WebError(error)), visitIdentifier: identifier From 88dd5561e100ff1d65128d2ca085d114e025ac0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zo=C3=AB=20Smith?= Date: Tue, 3 Mar 2026 10:37:17 -0500 Subject: [PATCH 10/12] Log HTTP status code on cold boot visit failures ColdBootVisit silently swallowed the status code when canceling non-2xx responses. Now logs at warning level with the status code and URL so errors like 404s are visible in the log files. --- Source/Turbo/Visit/ColdBootVisit.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/Turbo/Visit/ColdBootVisit.swift b/Source/Turbo/Visit/ColdBootVisit.swift index 919aa64e..7d9d7581 100644 --- a/Source/Turbo/Visit/ColdBootVisit.swift +++ b/Source/Turbo/Visit/ColdBootVisit.swift @@ -105,6 +105,7 @@ extension ColdBootVisit: WKNavigationDelegate { decisionHandler(.allow) } else { decisionHandler(.cancel) + logger.warning("[ColdBootVisit] request failed with status \(httpResponse.statusCode) for \(self.location.absoluteString)") if let httpError = HTTPError(statusCode: httpResponse.statusCode) { fail(with: .http(httpError)) } else { From 5b59e68763a7e05ffaf9a5b5f8c510429a71f412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20S=CC=8Cvara?= Date: Tue, 30 Jun 2026 13:55:05 +0200 Subject: [PATCH 11/12] Fix inaccurate log messages in Navigator session inspection - Correct the web-view-recreation skip log: it fires when the process is not terminated, not when a topmost visitable is missing - Fix typo and clarify the no-parent reload skip message - Drop a stray whitespace-only line --- Source/Turbo/Navigator/Navigator.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Turbo/Navigator/Navigator.swift b/Source/Turbo/Navigator/Navigator.swift index b5c707e8..35821654 100644 --- a/Source/Turbo/Navigator/Navigator.swift +++ b/Source/Turbo/Navigator/Navigator.swift @@ -296,9 +296,9 @@ extension Navigator { logger.info("Skipping session reload: no visitableViewController found") return } - + guard viewController.parent != nil else { - logger.info("Skipping session reload: no visitableViewControlle parent found") + logger.info("Skipping session reload: visitableViewController has no parent") return } @@ -343,7 +343,7 @@ extension Navigator { session.webView.queryWebContentProcessState { [weak self] state in guard case .terminated = state else { - logger.debug("Skipping webview recreation: no topmostVisitable found") + logger.debug("Skipping web view recreation: process not terminated") return } self?.recreateWebView(for: session) From 1e0d427064d49c66bc60c3d59e2dacc6c3e9d651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20S=CC=8Cvara?= Date: Tue, 30 Jun 2026 14:08:05 +0200 Subject: [PATCH 12/12] Add tests for always-on visit retry handler Covers the new Navigator behavior: a retry handler is always provided (even for previously non-retryable errors), the handler retries the visit directly with a cold boot when there is no topmost visitable, and falls back to reloading the session otherwise. --- .../NavigatorRetryHandlerTests.swift | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 Tests/Turbo/Navigator/NavigatorRetryHandlerTests.swift diff --git a/Tests/Turbo/Navigator/NavigatorRetryHandlerTests.swift b/Tests/Turbo/Navigator/NavigatorRetryHandlerTests.swift new file mode 100644 index 00000000..d2c967b1 --- /dev/null +++ b/Tests/Turbo/Navigator/NavigatorRetryHandlerTests.swift @@ -0,0 +1,87 @@ +@testable import HotwireNative +import WebKit +import XCTest + +final class NavigatorRetryHandlerTests: XCTestCase { + private var delegate: RetryCapturingNavigatorDelegate! + private var session: RetryRecordingSessionSpy! + private var navigator: Navigator! + private let visitable = TestVisitable(url: URL(string: "https://example.com")!) + + override func setUp() { + delegate = RetryCapturingNavigatorDelegate() + session = RetryRecordingSessionSpy(webView: Hotwire.config.makeWebView()) + navigator = Navigator( + session: session, + modalSession: Session(webView: Hotwire.config.makeWebView()), + delegate: delegate, + configuration: .init(name: "", startLocation: URL(string: "https://example.com")!) + ) + } + + /// A retry handler is now always provided, even for errors that were + /// previously considered non-retryable (e.g. a 404). + func test_didFailRequest_alwaysProvidesRetryHandler() { + navigator.session(session, didFailRequestForVisitable: visitable, error: .http(.client(.notFound))) + + XCTAssertTrue(delegate.didFailRequestCalled) + XCTAssertNotNil(delegate.capturedRetryHandler) + } + + /// When there is no topmost visitable yet (e.g. a failed cold boot), + /// `session.reload()` would no-op, so the handler retries the visit directly. + func test_retryHandler_withNoTopmostVisitable_visitsVisitableWithReload() { + session.stubbedTopmostVisitable = nil + + navigator.session(session, didFailRequestForVisitable: visitable, error: .http(.client(.notFound))) + delegate.capturedRetryHandler?() + + XCTAssertEqual(session.visitWithReloadCallCount, 1) + XCTAssertIdentical(session.visitedVisitable, visitable) + XCTAssertEqual(session.reloadCallCount, 0) + } + + /// When a topmost visitable exists, the handler reloads the session as before. + func test_retryHandler_withTopmostVisitable_reloadsSession() { + session.stubbedTopmostVisitable = visitable + + navigator.session(session, didFailRequestForVisitable: visitable, error: .http(.client(.notFound))) + delegate.capturedRetryHandler?() + + XCTAssertEqual(session.reloadCallCount, 1) + XCTAssertEqual(session.visitWithReloadCallCount, 0) + } +} + +// MARK: - Test doubles + +private final class RetryCapturingNavigatorDelegate: NavigatorDelegate { + private(set) var didFailRequestCalled = false + private(set) var capturedRetryHandler: RetryBlock? + + func visitableDidFailRequest(_ visitable: Visitable, error: HotwireNativeError, retryHandler: RetryBlock?) { + didFailRequestCalled = true + capturedRetryHandler = retryHandler + } +} + +private final class RetryRecordingSessionSpy: Session { + var stubbedTopmostVisitable: Visitable? + private(set) var reloadCallCount = 0 + private(set) var visitWithReloadCallCount = 0 + private(set) var visitedVisitable: Visitable? + + override var topmostVisitable: Visitable? { + stubbedTopmostVisitable + } + + override func reload() { + reloadCallCount += 1 + } + + override func visit(_ visitable: any Visitable, options: VisitOptions?, reload: Bool) { + guard reload else { return } + visitWithReloadCallCount += 1 + visitedVisitable = visitable + } +}