From 0d809f66f4aa8d7982ad8c17708ef76a31375b05 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Wed, 1 Jul 2026 14:48:47 -0300 Subject: [PATCH 1/2] fix: sync upstream bug fixes from swift-openapi-urlsession PRs #94 and #95 - Add `cancel(error:)` to `HTTPBodyOutputStreamBridge` so URLSession task errors can be propagated to the stream bridge (PR #94) - Fix `errorOccurred` state machine: `.initial` and `.waitingForBytes` now both close the stream; `.closed` is handled gracefully (PR #95) - Fix `wroteFinalChunk`/`endEncountered`: `.closed` state logs and returns instead of calling `preconditionFailure` (PR #95) - Propagate URLSession errors to the stream bridge in `BidirectionalStreamingURLSessionDelegate.didCompleteWithError` (PR #94) - Replace `NSLock` in `HTTPBody` with the internal pthread-based `Lock` - Add `Scripts/check-upstream.sh` to simplify future upstream syncs --- Scripts/check-upstream.sh | 98 +++++++++++++++++++ ...rectionalStreamingURLSessionDelegate.swift | 4 + .../HTTPBodyOutputStreamBridge.swift | 30 ++++-- Sources/HTTPClient/Interface/HTTPBody.swift | 7 +- 4 files changed, 126 insertions(+), 13 deletions(-) create mode 100755 Scripts/check-upstream.sh diff --git a/Scripts/check-upstream.sh b/Scripts/check-upstream.sh new file mode 100755 index 0000000..4e25b49 --- /dev/null +++ b/Scripts/check-upstream.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env bash +# Compares upstream swift-openapi-runtime and swift-openapi-urlsession files +# against their local counterparts, ignoring indentation-only differences. +# +# Usage: ./Scripts/check-upstream.sh +# Requires: gh (GitHub CLI), base64, diff + +set -euo pipefail + +TMPDIR_LOCAL=$(mktemp -d) +trap 'rm -rf "$TMPDIR_LOCAL"' EXIT + +SOURCES="Sources/HTTPClient" + +fetch() { + local repo="$1" remote_path="$2" local_path="$3" label="$4" + local upstream="$TMPDIR_LOCAL/$(basename "$remote_path")" + gh api "repos/$repo/contents/$remote_path" --jq '.content' | base64 -d > "$upstream" + + # Strip import lines that differ only due to module name (OpenAPIRuntime vs HTTPTypes) + # and do an ignore-whitespace diff to surface only logic changes + if ! diff --ignore-all-space --ignore-blank-lines "$upstream" "$local_path" > /dev/null 2>&1; then + echo "=== $label ===" + diff --ignore-all-space --ignore-blank-lines "$upstream" "$local_path" || true + echo "" + else + echo "[ok] $label" + fi +} + +echo "Checking swift-openapi-runtime files..." +fetch apple/swift-openapi-runtime \ + Sources/OpenAPIRuntime/Interface/HTTPBody.swift \ + "$SOURCES/Interface/HTTPBody.swift" \ + "HTTPBody.swift" + +fetch apple/swift-openapi-runtime \ + Sources/OpenAPIRuntime/Interface/AsyncSequenceCommon.swift \ + "$SOURCES/Interface/AsyncSequenceCommon.swift" \ + "AsyncSequenceCommon.swift" + +fetch apple/swift-openapi-runtime \ + Sources/OpenAPIRuntime/Interface/ClientTransport.swift \ + "$SOURCES/Interface/ClientTransport.swift" \ + "ClientTransport.swift" + +fetch apple/swift-openapi-runtime \ + Sources/OpenAPIRuntime/Interface/CurrencyTypes.swift \ + "$SOURCES/Interface/CurrencyTypes.swift" \ + "CurrencyTypes.swift" + +fetch apple/swift-openapi-runtime \ + Sources/OpenAPIRuntime/Errors/ClientError.swift \ + "$SOURCES/Errors/ClientError.swift" \ + "ClientError.swift" + +fetch apple/swift-openapi-runtime \ + Sources/OpenAPIRuntime/Errors/RuntimeError.swift \ + "$SOURCES/Errors/RuntimeError.swift" \ + "RuntimeError.swift" + +fetch apple/swift-openapi-runtime \ + Sources/OpenAPIRuntime/Base/PrettyStringConvertible.swift \ + "$SOURCES/Base/PrettyStringConvertible.swift" \ + "PrettyStringConvertible.swift" + +echo "" +echo "Checking swift-openapi-urlsession files..." + +fetch apple/swift-openapi-urlsession \ + Sources/OpenAPIURLSession/URLSessionTransport.swift \ + "$SOURCES/HTTPClientFoundation/URLSessionTransport.swift" \ + "URLSessionTransport.swift" + +fetch apple/swift-openapi-urlsession \ + Sources/OpenAPIURLSession/BufferedStream/BufferedStream.swift \ + "$SOURCES/HTTPClientFoundation/BufferedStream/BufferedStream.swift" \ + "BufferedStream.swift" + +fetch apple/swift-openapi-urlsession \ + Sources/OpenAPIURLSession/BufferedStream/Lock.swift \ + "$SOURCES/HTTPClientFoundation/BufferedStream/Lock.swift" \ + "Lock.swift" + +fetch apple/swift-openapi-urlsession \ + Sources/OpenAPIURLSession/URLSessionBidirectionalStreaming/BidirectionalStreamingURLSessionDelegate.swift \ + "$SOURCES/HTTPClientFoundation/URLSessionBidirectionalStreaming/BidirectionalStreamingURLSessionDelegate.swift" \ + "BidirectionalStreamingURLSessionDelegate.swift" + +fetch apple/swift-openapi-urlsession \ + Sources/OpenAPIURLSession/URLSessionBidirectionalStreaming/HTTPBodyOutputStreamBridge.swift \ + "$SOURCES/HTTPClientFoundation/URLSessionBidirectionalStreaming/HTTPBodyOutputStreamBridge.swift" \ + "HTTPBodyOutputStreamBridge.swift" + +fetch apple/swift-openapi-urlsession \ + Sources/OpenAPIURLSession/URLSessionBidirectionalStreaming/URLSession+Extensions.swift \ + "$SOURCES/HTTPClientFoundation/URLSessionBidirectionalStreaming/URLSession+Extensions.swift" \ + "URLSession+Extensions.swift" diff --git a/Sources/HTTPClient/HTTPClientFoundation/URLSessionBidirectionalStreaming/BidirectionalStreamingURLSessionDelegate.swift b/Sources/HTTPClient/HTTPClientFoundation/URLSessionBidirectionalStreaming/BidirectionalStreamingURLSessionDelegate.swift index a17a96b..18bf9e7 100644 --- a/Sources/HTTPClient/HTTPClientFoundation/URLSessionBidirectionalStreaming/BidirectionalStreamingURLSessionDelegate.swift +++ b/Sources/HTTPClient/HTTPClientFoundation/URLSessionBidirectionalStreaming/BidirectionalStreamingURLSessionDelegate.swift @@ -199,6 +199,10 @@ import HTTPTypes responseContinuation?.resume(throwing: error) responseContinuation = nil } + + // Propagate the error to the stream bridge. + requestStream?.cancel(error: error) + requestStream = nil } } } diff --git a/Sources/HTTPClient/HTTPClientFoundation/URLSessionBidirectionalStreaming/HTTPBodyOutputStreamBridge.swift b/Sources/HTTPClient/HTTPClientFoundation/URLSessionBidirectionalStreaming/HTTPBodyOutputStreamBridge.swift index 7e0c776..762f2a2 100644 --- a/Sources/HTTPClient/HTTPClientFoundation/URLSessionBidirectionalStreaming/HTTPBodyOutputStreamBridge.swift +++ b/Sources/HTTPClient/HTTPClientFoundation/URLSessionBidirectionalStreaming/HTTPBodyOutputStreamBridge.swift @@ -132,6 +132,17 @@ import HTTPTypes break } } + + func cancel(error: (any Error)? = nil) { + debug("Output stream received cancellation request.") + Self.streamQueue.async { [self] in + switch state { + case .initial, .waitingForBytes, .haveBytes, .needBytes: + self.performAction(state.errorOccurred(error ?? CancellationError())) + case .closed: break + } + } + } } extension HTTPBodyOutputStreamBridge { @@ -206,10 +217,7 @@ import HTTPTypes mutating func errorOccurred(_ error: any Error) -> Action { switch self { - case .initial: - self = .closed(error) - return .none - case .waitingForBytes(_): + case .initial, .waitingForBytes(_): self = .closed(error) return .closeStream case .haveBytes(_, _, let producerContinuation): @@ -218,7 +226,9 @@ import HTTPTypes case .needBytes(_, let producerContinuation): self = .closed(error) return .cancelProducerAndCloseStream(producerContinuation) - case .closed: preconditionFailure("\(#function) called in invalid state: \(self)") + case .closed: + debug("Ignoring \(#function) event in closed state") + return .none } } @@ -227,7 +237,10 @@ import HTTPTypes case .waitingForBytes(_): self = .closed(nil) return .closeStream - case .initial, .haveBytes, .needBytes, .closed: + case .closed: + debug("Ignoring \(#function) event in closed state") + return .none + case .initial, .haveBytes, .needBytes: preconditionFailure("\(#function) called in invalid state: \(self)") } } @@ -243,7 +256,10 @@ import HTTPTypes case .needBytes(_, let producerContinuation): self = .closed(nil) return .cancelProducerAndCloseStream(producerContinuation) - case .initial, .closed: preconditionFailure("\(#function) called in invalid state: \(self)") + case .closed: + debug("Ignoring \(#function) event in closed state") + return .none + case .initial: preconditionFailure("\(#function) called in invalid state: \(self)") } } diff --git a/Sources/HTTPClient/Interface/HTTPBody.swift b/Sources/HTTPClient/Interface/HTTPBody.swift index 640237a..01132bd 100644 --- a/Sources/HTTPClient/Interface/HTTPBody.swift +++ b/Sources/HTTPClient/Interface/HTTPBody.swift @@ -14,7 +14,6 @@ import struct Foundation.Data // only for convenience initializers import protocol Foundation.LocalizedError -import class Foundation.NSLock /// A body of an HTTP request or HTTP response. /// @@ -142,11 +141,7 @@ public final class HTTPBody: @unchecked Sendable { private let sequence: AnySequence /// A lock for shared mutable state. - private let lock: NSLock = { - let lock = NSLock() - lock.name = "com.apple.swift-openapi-generator.runtime.body" - return lock - }() + private let lock = Lock() /// A flag indicating whether an iterator has already been created. private var locked_iteratorCreated: Bool = false From d7d6c1bc21b89da77ee3982d68204ce3063958df Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Wed, 1 Jul 2026 14:57:57 -0300 Subject: [PATCH 2/2] test: fix flaky LoggingMiddlewareTests by replacing racy sleep with deterministic wait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `log()` handler fires `Task { await collector.append(entry) }`, so entries arrive asynchronously. Using a fixed 100ms sleep was racy — in `loggingMiddlewareLogsDifferentMethods` the response Task could arrive before the request Task, causing an order-sensitive `entries[0]` check to fail. - Add `waitForEntries(count:)` to `LogCollector` that polls via `Task.yield()` until the expected number of entries arrive - Replace all `Task.sleep(for: .milliseconds(100))` calls with it - Find the request log by content ("⬆️") rather than assuming index 0 - Use `waitForEntries(count: 1)` in the error test (only request is logged) --- .../LoggingMiddlewareTests.swift | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/Tests/HTTPClientTests/LoggingMiddlewareTests.swift b/Tests/HTTPClientTests/LoggingMiddlewareTests.swift index d035727..b046364 100644 --- a/Tests/HTTPClientTests/LoggingMiddlewareTests.swift +++ b/Tests/HTTPClientTests/LoggingMiddlewareTests.swift @@ -34,6 +34,12 @@ struct LoggingMiddlewareTests { func clear() { entries.removeAll() } + + func waitForEntries(count: Int) async { + while entries.count < count { + await Task.yield() + } + } } let collector: LogCollector @@ -98,7 +104,7 @@ struct LoggingMiddlewareTests { _ = try await client.send(request) // Wait a bit for async logging to complete - try await Task.sleep(for: .milliseconds(100)) + await collector.waitForEntries(count: 2) let entries = await collector.getEntries() @@ -135,7 +141,7 @@ struct LoggingMiddlewareTests { let request = HTTPRequest(method: .post, url: serverURL.appending(path: "users")) _ = try await client.send(request) - try await Task.sleep(for: .milliseconds(100)) + await collector.waitForEntries(count: 2) let entries = await collector.getEntries() #expect(entries.count == 2) @@ -162,7 +168,7 @@ struct LoggingMiddlewareTests { let request = HTTPRequest(method: .get, url: serverURL.appending(path: "test")) _ = try await client.send(request) - try await Task.sleep(for: .milliseconds(100)) + await collector.waitForEntries(count: 2) let entries = await collector.getEntries() #expect(entries.count == 2) @@ -190,7 +196,7 @@ struct LoggingMiddlewareTests { let request = HTTPRequest(method: .get, url: serverURL.appending(path: "test")) _ = try await client.send(request) - try await Task.sleep(for: .milliseconds(100)) + await collector.waitForEntries(count: 2) let entries = await collector.getEntries() #expect(entries.count == 2) @@ -215,7 +221,7 @@ struct LoggingMiddlewareTests { let request = HTTPRequest(method: .get, url: serverURL.appending(path: "test")) _ = try await client.send(request) - try await Task.sleep(for: .milliseconds(100)) + await collector.waitForEntries(count: 2) // Logs should be created let entries = await collector.getEntries() @@ -245,12 +251,12 @@ struct LoggingMiddlewareTests { let request = HTTPRequest(method: method, url: serverURL.appending(path: "test")) _ = try await client.send(request) - try await Task.sleep(for: .milliseconds(100)) + await collector.waitForEntries(count: 2) let entries = await collector.getEntries() #expect(entries.count == 2) - let requestLog = entries[0] + let requestLog = try #require(entries.first(where: { $0.message.contains("⬆️") })) #expect(requestLog.message.contains(method.rawValue)) } } @@ -278,7 +284,7 @@ struct LoggingMiddlewareTests { let request = HTTPRequest(method: .get, url: serverURL.appending(path: "test")) _ = try await client.send(request) - try await Task.sleep(for: .milliseconds(100)) + await collector.waitForEntries(count: 2) let entries = await collector.getEntries() #expect(entries.count == 2) @@ -320,7 +326,7 @@ struct LoggingMiddlewareTests { let request = HTTPRequest(method: .get, url: serverURL.appending(path: "test")) _ = try await client.send(request) - try await Task.sleep(for: .milliseconds(100)) + await collector.waitForEntries(count: 2) // Logging middleware should have logged let entries = await collector.getEntries() @@ -363,7 +369,7 @@ struct LoggingMiddlewareTests { // Expected to throw } - try await Task.sleep(for: .milliseconds(100)) + await collector.waitForEntries(count: 1) // Request should still be logged let entries = await collector.getEntries()