Skip to content

Commit

Permalink
Fixed a Task cancellation bug.
Browse files Browse the repository at this point in the history
A misleading error was being thrown if the Task performing a WebSocket handshake
was canceled. Added WebSocketError.canceled to address this case.
  • Loading branch information
rs70 committed May 27, 2022
1 parent 5ec62d4 commit a48c419
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 0 deletions.
6 changes: 6 additions & 0 deletions Sources/WebSockets/WebSocket.swift
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,12 @@ private extension WebSocket {
continue
}
}
// The `AsyncThrowingStream` used by Connection finishes the stream (yielding `nil`) if the Task is
// canceled. However, we want an error to be thrown in this instance so that it's not confused with
// the server simply dropping the connection.
if Task.isCancelled {
throw WebSocketError.canceled
}
if openingHandshakeDidExpire {
throw WebSocketError.timeout
}
Expand Down
5 changes: 5 additions & 0 deletions Sources/WebSockets/WebSocketError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ public enum WebSocketError: Error {
/// The handshake did not complete within the specified timeframe.
case timeout

/// The `Task` performing the handshake was canceled.
case canceled

/// The redirect limit was exceeded. This usually indicates a redirect loop.
case maximumRedirectsExceeded

Expand Down Expand Up @@ -99,6 +102,8 @@ extension WebSocketError: CustomDebugStringConvertible {
return "Unexpected disconnect"
case .timeout:
return "WebSocket handshake timed out"
case .canceled:
return "The task performing the WebSocket handshake was canceled"
case .maximumRedirectsExceeded:
return "Maximum number of HTTP redirects exceeded"
case .invalidRedirectLocation(let location):
Expand Down
21 changes: 21 additions & 0 deletions Tests/WebSocketsTests/WebSocketTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,27 @@ class WebSocketTests: XCTestCase {
XCTAssert(events == expected)
}

func testHandshakeCancellation() async throws {
let server = TestServer()
defer {
Task {
await server.stop()
}
}
let socket = WebSocket(url: try await server.start(path: "/test"))
let t = Task {
await socket.send(text: "Hello")
}
t.cancel()
let _ = await t.value
do {
for try await _ in socket {
}
XCTFail("Expected an exception ")
} catch WebSocketError.canceled {
}
}

func expectCloseCode(quirk: QuirkyTestServer.Quirk, code: WebSocket.CloseCode, reason: String? = nil) async throws {
let server = QuirkyTestServer(with: quirk)
defer {
Expand Down

0 comments on commit a48c419

Please sign in to comment.