Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,26 @@ final class NavigationDelegate: NSObject, WKNavigationDelegate {
}

func webView(_ webView: WKWebView, didFail navigation: WKNavigation!, withError error: Error) {
if Self.shouldIgnoreNavigationError(error) {
self.completeOnce(.success(()))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Don't mark cancelled navigations as completed success

Calling completeOnce(.success(())) for NSURLErrorCancelled in the failure delegates turns an intermediate cancellation into a terminal success. In this class, hasCompleted then suppresses all later callbacks, so a redirect-driven -999 can cause prepareWebView to resume before the real navigation finishes (or before a later hard failure), which can lead to scraping stale/partial dashboard content and masking real load errors.

Useful? React with 👍 / 👎.

return
}
Comment on lines 22 to 25

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Complete navigation on ignored cancel errors

Returning early on NSURLErrorCancelled means the delegate never calls completeOnce, so prepareWebView’s checked continuation in Sources/CodexBarCore/OpenAIWeb/OpenAIDashboardWebViewCache.swift can remain suspended forever when cancellation is terminal (i.e., no follow-up didFinish/didFail arrives after the cancel). In that case acquire(...) never returns and the higher-level dashboard timeout logic is bypassed because its deadline is started only after makeWebView completes.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 375de22. Ignored NSURLErrorCancelled now completes the delegate continuation with success (instead of returning early), so prepareWebView cannot hang waiting for a callback. Added a focused test in OpenAIDashboardNavigationDelegateTests to cover cancelled-failure completion behavior.

self.completeOnce(.failure(error))
}

func webView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation!, withError error: Error) {
if Self.shouldIgnoreNavigationError(error) {
self.completeOnce(.success(()))
return
}
self.completeOnce(.failure(error))
}

nonisolated static func shouldIgnoreNavigationError(_ error: Error) -> Bool {
let nsError = error as NSError
return nsError.domain == NSURLErrorDomain && nsError.code == NSURLErrorCancelled
}

private func completeOnce(_ result: Result<Void, Error>) {
guard !self.hasCompleted else { return }
self.hasCompleted = true
Expand Down
36 changes: 36 additions & 0 deletions Tests/CodexBarTests/OpenAIDashboardNavigationDelegateTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import Foundation
import Testing
import WebKit
@testable import CodexBarCore

@Suite
struct OpenAIDashboardNavigationDelegateTests {
@Test("ignores NSURLErrorCancelled")
func ignoresCancelledNavigationError() {
let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorCancelled)
#expect(NavigationDelegate.shouldIgnoreNavigationError(error))
}

@Test("does not ignore non-cancelled URL errors")
func doesNotIgnoreOtherURLErrors() {
let error = NSError(domain: NSURLErrorDomain, code: NSURLErrorTimedOut)
#expect(!NavigationDelegate.shouldIgnoreNavigationError(error))
}

@MainActor
@Test("cancelled failures complete with success")
func cancelledFailureCompletesWithSuccess() {
let webView = WKWebView()
var result: Result<Void, Error>?
let delegate = NavigationDelegate { result = $0 }

delegate.webView(webView, didFail: nil, withError: NSError(domain: NSURLErrorDomain, code: NSURLErrorCancelled))

switch result {
case .success?:
#expect(Bool(true))
default:
#expect(Bool(false))
}
}
}