mirror of
https://github.com/swift-server/async-http-client.git
synced 2026-05-03 07:32:29 +00:00
Fabian Fett
e2ab0d176f
> ## Note: > This is a long LLM generated PR description. However it captures very well, what has been changed and has already been reduced for brevity. The PR is sadly quite complex but I think the description captures the changes quite well. This is foundational work needed to properly support HTTP trailers and scenarios where the server sends a complete response before the client finishes uploading (e.g., early rejection, 100-continue flows, or bidirectional streaming protocols). ## Changes ### State Machine Improvements - **Added `endForwarded` state** to `Transaction.StateMachine.RequestStreamState` - This new state distinguishes between "request data forwarded to the channel" and "request data written to the network" - Properly handles the race condition where response completes before the request write completes - **Renamed `succeedRequest` → `forwardResponseEnd`** in both `HTTPRequestStateMachine.Action` and `HTTP1ConnectionStateMachine.Action` - Better reflects the semantic meaning: we're forwarding the end of the response stream, not necessarily succeeding the entire request yet - More accurate naming for bidirectional streaming scenarios ### Protocol Changes - **Added `requestBodyStreamSent()` to `HTTPExecutableRequest` protocol** - Called by the channel handler when the request body stream has been fully written to the network - Allows proper coordination between request and response stream completion - Implemented in both `Transaction` and `RequestBag` ### Request State Machine Updates - **Updated `FinalSuccessfulRequestAction`** - Changed `.sendRequestEnd(EventLoopPromise<Void>?)` to simpler `.requestDone` - Added `.none` case for when response completes but request is still in-flight - Removed the need to pass promises around, simplifying the state machine - **`sendRequestEnd` action now includes `FinalSuccessfulRequestAction`** - Allows the state machine to signal what should happen after the request completes - Enables proper cleanup coordination (idle connection, close, or continue) ### Channel Handler Updates - **HTTP1ClientChannelHandler** - `sendRequestEnd` now properly handles scenarios where response has already completed - Added future callback to coordinate request completion with final actions - Properly manages connection state (idle vs close) based on both streams completing - **HTTP2ClientRequestHandler** - Updated to handle new `sendRequestEnd` signature - Properly ignores HTTP/1-specific final actions (like `.requestDone`) ### RequestBag State Machine - **Added `endReceived` state to `ResponseStreamState`** - Tracks when the response has completed while request is still ongoing - Enables proper sequencing: response end → request end → task completion - **Updated `FinishAction`** - Added `.forwardStreamFinishedAndSucceedTask` for the case where both streams complete simultaneously - Ensures delegate methods are called in the correct order ### Error Handling - **Improved failure handling in `Transaction.StateMachine`** - Now properly handles errors that occur after response completes but before request finishes - Added `cancelExecutor` action to the fail path - Executor is now passed to `failRequestStreamContinuation` for proper cleanup ## Technical Details ### The Problem Previously, when a server sent a complete response before the client finished uploading the request body, AHC would: 1. Receive the full response (head, body, end) 2. But NOT inform the user that the response was complete if the request was still streaming 3. Only succeed the request after both streams completed This made it impossible to implement proper bidirectional streaming or handle scenarios like: - Server rejecting a large upload early (e.g., 413 Payload Too Large) - 100-continue flows where the server responds before request completes - HTTP trailers sent by the server ### The Solution The new state machine properly tracks four completion states: 1. **Neither complete**: Normal request/response in flight 2. **Response complete, request ongoing**: New `endForwarded`/`endReceived` states 3. **Request complete, response ongoing**: Existing logic 4. **Both complete**: Request succeeds The key insight is the `endForwarded` state, which represents "we've given all request data to the channel, but it hasn't been written to the network yet". This allows us to: - Immediately forward response completion to the user - Wait for the write to complete before cleaning up resources - Properly sequence connection state transitions ## Future Work This PR lays the groundwork for: - Proper internal HTTP trailer support (both sending and receiving) --------- Co-authored-by: George Barnett <gbarnett@apple.com>
145 lines
5.1 KiB
Swift
145 lines
5.1 KiB
Swift
//===----------------------------------------------------------------------===//
|
|
//
|
|
// This source file is part of the AsyncHTTPClient open source project
|
|
//
|
|
// Copyright (c) 2021 Apple Inc. and the AsyncHTTPClient project authors
|
|
// Licensed under Apache License v2.0
|
|
//
|
|
// See LICENSE.txt for license information
|
|
// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors
|
|
//
|
|
// SPDX-License-Identifier: Apache-2.0
|
|
//
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
import Logging
|
|
import NIOCore
|
|
import NIOEmbedded
|
|
import NIOHTTP1
|
|
import NIOSSL
|
|
import XCTest
|
|
|
|
@testable import AsyncHTTPClient
|
|
|
|
class HTTPConnectionPool_RequestQueueTests: XCTestCase {
|
|
func testCountAndIsEmptyWorks() {
|
|
var queue = HTTPConnectionPool.RequestQueue()
|
|
XCTAssertTrue(queue.isEmpty)
|
|
XCTAssertEqual(queue.count, 0)
|
|
let req1 = MockScheduledRequest(requiredEventLoop: nil)
|
|
let req1ID = queue.push(.init(req1))
|
|
XCTAssertFalse(queue.isEmpty)
|
|
XCTAssertFalse(queue.isEmpty(for: nil))
|
|
XCTAssertEqual(queue.count, 1)
|
|
XCTAssertEqual(queue.generalPurposeCount, 1)
|
|
|
|
let req2 = MockScheduledRequest(requiredEventLoop: nil)
|
|
let req2ID = queue.push(.init(req2))
|
|
XCTAssertEqual(queue.count, 2)
|
|
|
|
XCTAssert(queue.popFirst()?.__testOnly_wrapped_request() === req1)
|
|
XCTAssertEqual(queue.count, 1)
|
|
XCTAssertFalse(queue.isEmpty)
|
|
XCTAssert(queue.remove(req2ID)?.__testOnly_wrapped_request() === req2)
|
|
XCTAssertNil(queue.remove(req1ID))
|
|
XCTAssertEqual(queue.count, 0)
|
|
XCTAssertTrue(queue.isEmpty)
|
|
|
|
let eventLoop = EmbeddedEventLoop()
|
|
|
|
XCTAssertTrue(queue.isEmpty(for: eventLoop))
|
|
XCTAssertEqual(queue.count(for: eventLoop), 0)
|
|
let req3 = MockScheduledRequest(requiredEventLoop: eventLoop)
|
|
let req3ID = queue.push(.init(req3))
|
|
XCTAssertFalse(queue.isEmpty(for: eventLoop))
|
|
XCTAssertEqual(queue.count(for: eventLoop), 1)
|
|
XCTAssertFalse(queue.isEmpty)
|
|
XCTAssertEqual(queue.count, 1)
|
|
XCTAssert(queue.popFirst(for: eventLoop)?.__testOnly_wrapped_request() === req3)
|
|
XCTAssertNil(queue.remove(req3ID))
|
|
XCTAssertTrue(queue.isEmpty(for: eventLoop))
|
|
XCTAssertEqual(queue.count(for: eventLoop), 0)
|
|
XCTAssertTrue(queue.isEmpty)
|
|
XCTAssertEqual(queue.count, 0)
|
|
|
|
let req4 = MockScheduledRequest(requiredEventLoop: eventLoop)
|
|
let req4ID = queue.push(.init(req4))
|
|
XCTAssert(queue.remove(req4ID)?.__testOnly_wrapped_request() === req4)
|
|
|
|
let req5 = MockScheduledRequest(requiredEventLoop: nil)
|
|
queue.push(.init(req5))
|
|
let req6 = MockScheduledRequest(requiredEventLoop: eventLoop)
|
|
queue.push(.init(req6))
|
|
let all = queue.removeAll()
|
|
let testSet = all.map { $0.__testOnly_wrapped_request() }
|
|
XCTAssertEqual(testSet.count, 2)
|
|
XCTAssertTrue(testSet.contains(where: { $0 === req5 }))
|
|
XCTAssertTrue(testSet.contains(where: { $0 === req6 }))
|
|
XCTAssertFalse(testSet.contains(where: { $0 === req4 }))
|
|
XCTAssertTrue(queue.isEmpty(for: eventLoop))
|
|
XCTAssertEqual(queue.count(for: eventLoop), 0)
|
|
XCTAssertTrue(queue.isEmpty)
|
|
XCTAssertEqual(queue.count, 0)
|
|
}
|
|
}
|
|
|
|
final private class MockScheduledRequest: HTTPSchedulableRequest {
|
|
let requiredEventLoop: EventLoop?
|
|
|
|
init(requiredEventLoop: EventLoop?) {
|
|
self.requiredEventLoop = requiredEventLoop
|
|
}
|
|
|
|
var poolKey: ConnectionPool.Key { preconditionFailure("Unimplemented") }
|
|
var tlsConfiguration: TLSConfiguration? { nil }
|
|
var logger: Logger { preconditionFailure("Unimplemented") }
|
|
var connectionDeadline: NIODeadline { preconditionFailure("Unimplemented") }
|
|
var preferredEventLoop: EventLoop { preconditionFailure("Unimplemented") }
|
|
|
|
func requestWasQueued(_: HTTPRequestScheduler) {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
|
|
func fail(_: Error) {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
|
|
// MARK: HTTPExecutableRequest
|
|
|
|
var requestHead: HTTPRequestHead { preconditionFailure("Unimplemented") }
|
|
var requestFramingMetadata: RequestFramingMetadata { preconditionFailure("Unimplemented") }
|
|
var requestOptions: RequestOptions { preconditionFailure("Unimplemented") }
|
|
|
|
func willExecuteRequest(_: HTTPRequestExecutor) {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
|
|
func requestHeadSent() {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
|
|
func resumeRequestBodyStream() {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
|
|
func pauseRequestBodyStream() {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
|
|
func requestBodyStreamSent() {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
|
|
func receiveResponseHead(_: HTTPResponseHead) {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
|
|
func receiveResponseBodyParts(_: CircularBuffer<ByteBuffer>) {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
|
|
func receiveResponseEnd(_ buffer: CircularBuffer<ByteBuffer>?, trailers: HTTPHeaders?) {
|
|
preconditionFailure("Unimplemented")
|
|
}
|
|
}
|