rastrillo / native Public

Throw CancellationError for cancelled requests; frame scripted streams like NativeHTTP

Review of #6 (Astra):
- P2: a cancelled request leaked URLSession's URLError(.cancelled), -999,
  to callers that match cancellation by type. classify now maps it to
  CancellationError; Task.isCancelled behaviour is unchanged, and the
  observer hears nothing for a cancellation.
- P3: ScriptedTransport split stream bodies on LF, so CRLF left "\r" in
  lines and an unterminated final line was delivered. It now runs the
  body through NativeStreamLines (package access), so framing, limits
  and invalid UTF-8 match NativeHTTP byte for byte.

Tests: cancellation before response headers for responseDetails, stream
and download; the existing mid-stream cancellation now asserts the type;
scripted-vs-real stream parity over LF, CRLF, CR, mixed, unterminated
and BOM bodies, and over invalid UTF-8 and an over-long line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 1 month ago 4ac16d546682d467cbe6d70e84aa102afb32d9b4 parent d9dac72

7 files changed, +161 −16

README.md +4 −2
Sources/RastrilloApple/NativeClientError.swift +6 −4
Sources/RastrilloApple/NativeStreamLines.swift +4 −3
Sources/RastrilloTestSupport/ScriptedTransport.swift +9 −6
Tests/RastrilloAppleTests/HTTPStreamCancellationTests.swift +2 −0
Tests/RastrilloAppleTests/TypedErrorTests.swift +39 −1
Tests/RastrilloTestSupportTests/StreamParityTests.swift +97 −0

CI failed — run details

Intent

full ledger: feat/k1-typed-errors-transport

diff --git a/README.md b/README.md

index df43deb..00ab329 100644

--- a/README.md

+++ b/README.md

@@ -91,7 +91,8 @@ to `.unauthorized` (401 and 303), `.forbidden`, `.notFound`, `.conflict(body)`,

`.tooLarge`, `.rateLimited(retryAfter:)` (429, or 503 with Retry-After),

`.server(status:)` and `.rejected(status:body:)`. `retryClass` sorts every case into

offline, transient, auth, permanent or conflict. `localizedDescription` keeps the

-sentences callers showed before.

+sentences callers showed before. A cancelled request throws `CancellationError`,

+never URLSession's -999.

`responseDetails` and `response` still return any status; `NativeResponse.requireSuccess()`

throws its typed case. `request` and `stream` throw typed cases, so a 401 on a

@@ -104,7 +105,8 @@ a session should take a supplier, `@MainActor () -> NativeTransport?`, rather th

capture one client, because signing in again replaces it. The

`RastrilloTestSupport` product ships `ScriptedTransport`, which answers each call

from a script: a status and body, offline, a hang until cancelled, or "commit,

-then lose the response". Use it in test targets in place of fixture servers.

+then lose the response". Scripted streams are framed by the same byte-level parser

+as `NativeHTTP`. Use it in test targets in place of fixture servers.

`tools/bundle-mac.py` includes SwiftPM resource bundles in `Contents/Resources`

and reads the minimum macOS version from the package manifest. A SwiftPM

diff --git a/Sources/RastrilloApple/NativeClientError.swift b/Sources/RastrilloApple/NativeClientError.swift

index e07f5a8..dac7cdd 100644

--- a/Sources/RastrilloApple/NativeClientError.swift

+++ b/Sources/RastrilloApple/NativeClientError.swift

@@ -80,11 +80,13 @@ public enum NativeClientError: LocalizedError, Sendable, Equatable {

.dnsLookupFailed, .timedOut, .dataNotAllowed, .internationalRoamingOff,

]

- /// Maps a URLSession failure to a typed case. Cancellation stays a

- /// `URLError(.cancelled)` so callers that check `Task.isCancelled` are

- /// unchanged; errors that are not URLErrors pass through untouched.

+ /// Maps a URLSession failure to a typed case. A cancelled task surfaces as

+ /// `CancellationError`, like every other cancelled Swift task: URLSession's

+ /// own -999 would otherwise reach callers that match cancellation by type

+ /// and read as a failure. Errors that are not URLErrors pass through untouched.

static func classify(_ error: Error) -> Error {

- guard let failure = error as? URLError, failure.code != .cancelled else { return error }

+ guard let failure = error as? URLError else { return error }

+ if failure.code == .cancelled { return CancellationError() }

return offlineCodes.contains(failure.code) ? NativeClientError.offline(failure.code) : NativeClientError.transport(failure.code)

}

diff --git a/Sources/RastrilloApple/NativeStreamLines.swift b/Sources/RastrilloApple/NativeStreamLines.swift

index 9b9217a..4ad8560 100644

--- a/Sources/RastrilloApple/NativeStreamLines.swift

+++ b/Sources/RastrilloApple/NativeStreamLines.swift

@@ -1,7 +1,8 @@

import Foundation

/// Bound before decoding: AsyncBytes.lines can allocate an unbounded line.

-struct NativeStreamLines {

+/// Package access so ScriptedTransport frames streams exactly as NativeHTTP does.

+package struct NativeStreamLines {

private var buffer = Data()

private var eventBytes = 0

private var afterCR = false

@@ -9,12 +10,12 @@ struct NativeStreamLines {

let lineLimit: Int

let eventLimit: Int

- init(lineLimit: Int = 256 * 1024, eventLimit: Int = 1024 * 1024) {

+ package init(lineLimit: Int = 256 * 1024, eventLimit: Int = 1024 * 1024) {

self.lineLimit = lineLimit

self.eventLimit = eventLimit

}

- mutating func append(_ byte: UInt8) throws -> String? {

+ package mutating func append(_ byte: UInt8) throws -> String? {

if afterCR {

afterCR = false

if byte == 10 { return nil }

diff --git a/Sources/RastrilloTestSupport/ScriptedTransport.swift b/Sources/RastrilloTestSupport/ScriptedTransport.swift

index 9911161..13766ec 100644

--- a/Sources/RastrilloTestSupport/ScriptedTransport.swift

+++ b/Sources/RastrilloTestSupport/ScriptedTransport.swift

@@ -16,8 +16,9 @@ import RastrilloApple

/// test that makes a request it did not plan for fails loudly.

public final class ScriptedTransport: NativeTransport, @unchecked Sendable {

public enum Step: Sendable {

- /// The server answers. For `stream`, a 200 body is delivered as its

- /// lines, split on LF; for `download`, a 200 body is the file.

+ /// The server answers. For `stream`, a 200 body is framed into lines

+ /// exactly as `NativeHTTP` frames the network; for `download`, a 200

+ /// body is the file.

case respond(status: Int, body: Data = Data(), headers: [String: String] = [:])

/// The request never reaches the server.

case offline(URLError.Code = .notConnectedToInternet)

@@ -121,11 +122,13 @@ public final class ScriptedTransport: NativeTransport, @unchecked Sendable {

throw NativeClientError.message("The live connection was refused. Reconnect to refresh your session.")

}

try await onOpen()

- var lines = String(decoding: response.data, as: UTF8.self).split(separator: "\n", omittingEmptySubsequences: false)

- if lines.last?.isEmpty == true { lines.removeLast() }

- for line in lines {

+ // The same byte-level framing as NativeHTTP: CR, LF and CRLF all end a

+ // line, limits and invalid UTF-8 throw, and an unterminated final line

+ // is never delivered.

+ var parser = NativeStreamLines()

+ for byte in response.data {

try Task.checkCancellation()

- try await onLine(String(line))

+ if let line = try parser.append(byte) { try await onLine(line) }

}

diff --git a/Tests/RastrilloAppleTests/HTTPStreamCancellationTests.swift b/Tests/RastrilloAppleTests/HTTPStreamCancellationTests.swift

index fc1cbe8..ebc6373 100644

--- a/Tests/RastrilloAppleTests/HTTPStreamCancellationTests.swift

+++ b/Tests/RastrilloAppleTests/HTTPStreamCancellationTests.swift

@@ -163,6 +163,8 @@ server.handle_request()

XCTFail("A cancelled stream completed successfully")

} catch {

XCTAssertTrue(Task.isCancelled)

+ // Callers match cancellation by type; URLSession's -999 must not leak.

+ XCTAssertTrue(error is CancellationError, "Expected CancellationError, got \(error)")

}

finished.fulfill()

}

diff --git a/Tests/RastrilloAppleTests/TypedErrorTests.swift b/Tests/RastrilloAppleTests/TypedErrorTests.swift

index 7064046..10f7bad 100644

--- a/Tests/RastrilloAppleTests/TypedErrorTests.swift

+++ b/Tests/RastrilloAppleTests/TypedErrorTests.swift

@@ -61,6 +61,44 @@ final class TypedErrorTests: XCTestCase {

XCTAssertEqual(outcomes.values, Array(repeating: .offline(.cannotConnectToHost), count: 4))

}

+ /// A server that accepts the connection and never answers, so the request

+ /// is cancelled while URLSession is still waiting for response headers.

+ func testCancelledRequestThrowsCancellationError() async throws {

+ let fd = socket(AF_INET, SOCK_STREAM, 0)

+ XCTAssertGreaterThanOrEqual(fd, 0)

+ defer { close(fd) }

+ var address = sockaddr_in()

+ address.sin_family = sa_family_t(AF_INET)

+ address.sin_addr.s_addr = inet_addr("127.0.0.1")

+ var length = socklen_t(MemoryLayout<sockaddr_in>.size)

+ let listening = withUnsafeMutablePointer(to: &address) {

+ $0.withMemoryRebound(to: sockaddr.self, capacity: 1) { pointer -> Int32 in

+ guard Darwin.bind(fd, pointer, length) == 0, listen(fd, 8) == 0 else { return -1 }

+ return getsockname(fd, pointer, &length)

+ }

+ XCTAssertEqual(listening, 0)

+ let outcomes = Outcomes()

+ let client = NativeHTTP(origin: URL(string: "http://127.0.0.1:\(UInt16(bigEndian: address.sin_port))")!,

+ observer: { outcomes.append($0) })

+ for call in ["responseDetails", "stream", "download"] {

+ let task = Task {

+ switch call {

+ case "responseDetails": _ = try await client.responseDetails("/silent")

+ case "stream": try await client.stream("/silent") { _ in }

+ default: try await client.download("/silent", to: FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString))

+ }

+ try await Task.sleep(nanoseconds: 200_000_000)

+ task.cancel()

+ do {

+ try await task.value

+ XCTFail("\(call): a cancelled request completed")

+ } catch { XCTAssertTrue(error is CancellationError, "\(call): expected CancellationError, got \(error)") }

+ }

+ XCTAssertEqual(outcomes.values, [], "Cancellation is not evidence about connectivity")

+ }

func testOfflineClassification() {

let offline: [URLError.Code] = [.notConnectedToInternet, .networkConnectionLost, .cannotFindHost, .cannotConnectToHost,

.dnsLookupFailed, .timedOut, .dataNotAllowed, .internationalRoamingOff]

@@ -74,7 +112,7 @@ final class TypedErrorTests: XCTestCase {

XCTAssertEqual(typed, .transport(code))

XCTAssertEqual(typed?.retryClass, .transient)

}

- XCTAssertEqual((NativeClientError.classify(URLError(.cancelled)) as? URLError)?.code, .cancelled)

+ XCTAssertTrue(NativeClientError.classify(URLError(.cancelled)) is CancellationError)

XCTAssertTrue(NativeClientError.classify(CancellationError()) is CancellationError)

}

diff --git a/Tests/RastrilloTestSupportTests/StreamParityTests.swift b/Tests/RastrilloTestSupportTests/StreamParityTests.swift

new file mode 100644

index 0000000..432a17c

--- /dev/null

+++ b/Tests/RastrilloTestSupportTests/StreamParityTests.swift

@@ -0,0 +1,97 @@

+#if os(macOS)

+import Foundation

+import XCTest

+import RastrilloApple

+import RastrilloTestSupport

+private actor Collected {

+ var lines: [String] = []

+ func append(_ line: String) { lines.append(line) }

+/// A scripted stream must deliver exactly what a real one does for the same

+/// bytes, or a test passes against framing no server produces.

+final class StreamParityTests: XCTestCase {

+ private let bodies: [String: Data] = [

+ "lf": Data("event: update\ndata: 1\n\n".utf8),

+ "crlf": Data("event: update\r\ndata: 2\r\n\r\n".utf8),

+ "cr": Data("event: update\rdata: 3\r\r".utf8),

+ "mixed": Data("a\r\nb\rc\n\r\n".utf8),

+ "unterminated": Data("data: 4\n\ndata: partial".utf8),

+ "bom": Data("\u{FEFF}data: 5\n\n".utf8),

+ ]

+ private func server(_ bodies: [String: Data]) throws -> (Process, URL) {

+ let directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)

+ try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)

+ for (name, body) in bodies { try body.write(to: directory.appendingPathComponent(name)) }

+ let process = Process()

+ process.executableURL = URL(fileURLWithPath: "/usr/bin/python3")

+ process.arguments = ["-u", "-c", #"""

+import http.server, pathlib, sys

+root = pathlib.Path(sys.argv[1])

+class Handler(http.server.BaseHTTPRequestHandler):

+ def log_message(self, *args): pass

+ def do_GET(self):

+ body = (root / self.path.strip('/')).read_bytes()

+ self.send_response(200)

+ self.send_header('Content-Type', 'text/event-stream')

+ self.send_header('Content-Length', str(len(body)))

+ self.end_headers()

+ self.wfile.write(body)

+server = http.server.ThreadingHTTPServer(('127.0.0.1', 0), Handler)

+print(server.server_port, flush=True)

+server.serve_forever()

+"""#, directory.path]

+ let output = Pipe()

+ process.standardOutput = output

+ try process.run()

+ var port = Data()

+ while port.count < 12 {

+ let byte = output.fileHandleForReading.readData(ofLength: 1)

+ if byte.isEmpty || byte == Data([10]) { break }

+ port.append(byte)

+ }

+ guard let number = String(data: port, encoding: .utf8).flatMap(Int.init),

+ let origin = URL(string: "http://127.0.0.1:\(number)") else {

+ process.terminate()

+ throw NativeClientError.message("The parity fixture could not start.")

+ }

+ return (process, origin)

+ }

+ private func lines(_ transport: NativeTransport, _ path: String) async throws -> [String] {

+ let collected = Collected()

+ try await transport.stream(path) { await collected.append($0) }

+ return await collected.lines

+ }

+ func testScriptedStreamFramesLikeNativeHTTP() async throws {

+ let (process, origin) = try server(bodies)

+ defer { process.terminate() }

+ let real = NativeHTTP(origin: origin)

+ for (name, body) in bodies.sorted(by: { $0.key < $1.key }) {

+ let expected = try await lines(real, "/\(name)")

+ let scripted = try await lines(ScriptedTransport([.respond(status: 200, body: body)]), "/\(name)")

+ XCTAssertEqual(scripted, expected, "\(name): scripted framing differs from NativeHTTP")

+ }

+ func testScriptedStreamRefusesWhatNativeHTTPRefuses() async throws {

+ let invalid = Data([0x64, 0x61, 0x74, 0x61, 0x3A, 0xFF, 0x0A])

+ let long = Data(repeating: 0x61, count: 256 * 1024 + 1) + Data("\n".utf8)

+ let (process, origin) = try server(["invalid": invalid, "long": long])

+ defer { process.terminate() }

+ let real = NativeHTTP(origin: origin)

+ for (name, body) in [("invalid", invalid), ("long", long)] {

+ var realError: String?

+ do { _ = try await lines(real, "/\(name)") } catch { realError = error.localizedDescription }

+ XCTAssertNotNil(realError, "\(name): NativeHTTP accepted it")

+ do {

+ _ = try await lines(ScriptedTransport([.respond(status: 200, body: body)]), "/\(name)")

+ XCTFail("\(name): ScriptedTransport accepted what NativeHTTP refuses")

+ } catch { XCTAssertEqual(error.localizedDescription, realError, name) }

+ }

+#endif