Closed
Conversation
…handling Replace TurboError with unified HotwireNativeError enum for exhaustive error handling
Cover all four error description cases (notPresent, notReady, contentTypeMismatch, invalidResponse) in a table-driven test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Test classification helpers (isOffline, isTimeout, isConnectionError, isSslError), error descriptions for all URLError branches including the unhandled-code fallback, and all three factory methods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Test status code range boundaries (399–600), round-trip all named ClientError and ServerError cases, unmapped codes falling to .other, and error descriptions for client, server, and unknown categories. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Test the turboJSStatusCode initializer against real Turbo.js SystemStatusCode values (0, -1, -2), common HTTP status codes routed through visitRequestFailed, unknown 4xx/5xx codes, unexpected codes that shouldn't arrive through normal flow, convenience properties (statusCode, urlError), and error description delegation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ertions - Replace all table-driven for-loops with individual test methods (124 total, up from 38) - Add `final` to all 4 test classes - Replace `if case` + `XCTFail` with `XCTAssertEqual` using constructed WebError values - Strengthen weak `XCTAssertNotNil` assertions to exact value checks - Add LoadError.description property tests (4 new) - Add HttpError.unknownError.statusCode round-trip tests - Remove duplicated from(turboStatusCode:) tests from WebErrorTests (internal method, covered via HotwireNativeError init in HotwireNativeErrorTests) - Add all 7 SSL error codes to isSslError tests (was only 4 of 7) - Add isTimeout edge case for raw -1001 errorCode without URLError - Add cross-classification tests (offline vs timeout vs connection vs SSL) - Add status code 1 edge case for unexpected positive non-HTTP codes - Replace string-based category matching with concrete HttpError equality assertions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WebViewNavigationSimulator always called decisionHandler(.allow), which let WKWebView actually navigate to https://example.com over the network. This blocked the shared WKProcessPool and stalled subsequent tests for 12+ minutes in CI. Now the simulator calls .cancel once it has captured the WKNavigationAction (which is all the tests need), and .allow only for intermediate navigations that must complete (e.g., the initial loadHTMLString page load). Also adds stopLoading() cleanup in tearDown as a safety net. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a JavaScript visit fails with a non-HTTP status code (e.g., network interruption, fetch abort), the framework checks for cross-origin redirects via RedirectHandler. In the .noRedirect case, the server confirmed reachable with a 2xx response — meaning the Turbo.js fetch failure was transient. Instead of immediately showing an error, retry once with a cold boot visit which bypasses JS fetch and uses WKWebView's native loading. This transparently recovers from transient network failures without user intervention. Loop safety: each visit identifier is retried at most once, and the cold boot uses a completely different failure path (WKNavigationDelegate) that cannot recurse back into resolveRedirect. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only transient HTTP errors (408 Request Timeout, 429 Too Many Requests) are now retryable — all other 4xx and all 5xx return false. Navigator conditionally provides a retry handler based on isRetryable. Also adds turboIsReady JS message, renames WebError.description to .message and LoadError.description to .title, types ErrorPresenter with HotwireNativeError, adds Sendable conformance, and safely unwraps HTTPError.from(statusCode:). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The default URLSession timeout is 60s, which adds unnecessary delay when the server is truly unreachable. 30s matches Turbo's own TURBO_LOAD_TIMEOUT while still accommodating spotty connections. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove status code validation from RedirectHandler: any HTTP response (even 401/500) means the server is reachable for redirect detection. Eliminates RedirectHandlerError.unacceptableStatusCode entirely. - Retry with cold boot when redirect check itself fails (requestFailed, invalidResponse) instead of failing immediately. This addresses ~10% of error-0 events where URLSession couldn't reach the server but WKWebView's native loading may still succeed. - Guard against duplicate sessions in backgroundTerminatedWebViewSessions when both process termination and fetch failure fire for the same session. - Add RedirectHandler unit tests using OHHTTPStubs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
URLError.Code.unknown.rawValue is -1, same as the Turbo.js timeout status code. When a URLError is present, check the typed code instead of the raw value to avoid misclassifying unknown errors as timeouts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
503 Service Unavailable and 504 Gateway Timeout are canonically transient errors where an immediate retry is appropriate. Update the doc comment to accurately describe which errors are retryable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Type conversions should use initializers, not static factory methods. Renames HTTPError.from(statusCode:), ClientError.from(statusCode:), ServerError.from(statusCode:), WebError.from(_:), and WebError.from(turboStatusCode:) to init overloads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Both cases previously returned identical errorDescription strings, making them indistinguishable in logs and user-facing messages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace force-unwraps with guard-let for the visitRequestFailedWithNonHttpStatusCode message handler to avoid a crash if the JS message is malformed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Parent HTTPError already declares Sendable. Match on nested public enums for clarity and forward-safety. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Session is a long-lived object; capture weakly in the unstructured Task to avoid a retain cycle per concurrency guidelines. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep the diff focused on functional changes only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
Author
|
Closed in favor of #231 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.