21 Commits

Author SHA1 Message Date
Fabian Fett 67ac92dc76 Support sending and receiving trailers in HTTPExecutableRequest (#882)
This pr adds all the internal wiring in `HTTP1ClientChannelHandler` and
`HTTP2ClientRequestHandler` to send and receive HTTP trailers.
2026-02-10 11:53:56 +01:00
Fabian Fett e2ab0d176f Full support for bidirectional streaming (#879)
> ## 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>
2026-02-03 12:21:31 +01:00
Fabian Fett 4b99975677 Rename succeedRequest to receiveResponseEnd (#877) 2026-01-08 11:36:36 +00:00
George Barnett 086524fd8a Fix sendability issues in the connection pool (#833)
Motivation:

The connection pool holds much of the low level logic in AHC. We should
fix its sendability issues before moving to higher levels.

Modifications:

- Make HTTP1ConnectionDelegate and HTTP2Delegate sendable, this requires
passing IDs rather than connections to their methods
- Make HTTPConnectionRequester sendable and have its methods take
Sendable views of the HTTP1Connection and HTTP2Connection types
- Add sendable views to HTTP1Connection and HTTP2Connection
- Mark HTTP1Connection and HTTP2Connection as not sendable
- Make HTTPRequestExecutor and HTTPExecutableRequest sendable
- Update tests

Result:

Connection pool has stricter sendability requirements
2025-04-28 15:33:38 +01:00
Rick Newton-Rogers f38c2fea86 Avoid precondition failure in write timeout (#803)
### Motivation:

In some cases we can crash because of a precondition failure when the
write timeout fires and we aren't in the running state. This can happen
for example if the connection is closed whilst the write timer is
active.

### Modifications:

* Remove the precondition and instead take no action if the timeout
fires outside of the running state. Instead we take a new `Action`,
`.noAction` when the timer fires.
* Clear write timeouts upon request completion. When a request completes
we have no use for the idle write timer, we clear the read timer and we
should clear the write one too.

### Result:

Fewer crashes.

The supplied tests fails without these changes and passes with either of them.
2025-01-28 17:50:38 +00:00
Johannes Weiss 2119f0d9cc fix 784: dont crash on huge in-memory bodies (#785)
fixes #784 

`writeChunks` had 3 bugs:
1. An actually wrong `UnsafeMutableTransferBox` -> removed that type
which should never be created
2. A loooong future chain (instead of one final promise) -> implemented
3. Potentially infinite recursion which lead to the crash in #784) ->
fixed too
2024-11-26 14:52:39 +00:00
Rick Newton-Rogers c621142327 Adopt GitHub actions (#780)
Migrate CI to use GitHub Actions.

### Motivation:

To migrate to GitHub actions and centralised infrastructure.

### Modifications:

Changes of note:
* Adopt swift-format using rules from SwiftNIO.
* Remove scripts and docker files which are no longer needed.
* Disabled warnings-as-errors on Swift 6.0 CI pipelines for now.

### Result:

Feature parity with old CI.
2024-10-29 15:01:46 +00:00
Gustavo Cairo ffe36fcf54 Fix potential race conditions when cancelling read/write idle timers (#720) 2023-12-18 10:16:27 -03:00
Gustavo Cairo d2d35663a2 Add an idle write timeout (#718) 2023-12-18 09:06:06 -03:00
Fabian Fett e26459902c Fix HTTP2StreamChannel leak (#657)
* Fix HTTP2StreamChannel leak

* Update code comments.
2023-02-14 10:04:27 +01:00
David Nadoba 1d24271fee Fix crash for large HTTP request headers (#661)
* Reproducer

* Refactor test case

* Refactor tests

* Remove debugging artefacts

* Fix typo

* Fix formatting

* Remove `promise?.succeed(())`

* Add test for HTTP2 request with large header

Motivation

We currently don't handle large headers well which trigger a channel writability change event.

Modification

Add failing (but currently skipped) tests which reproduces the issue
Result

We can reliably reproduce the large request header issue in an integration and unit test.
Note that the actual fix is not included to make reviewing easier and will come in a follow up PR.

* Remove logging

* Fix crash for large HTTP request headers

Fix crash for when sending HTTP request headers result in a channel writability change event

* Formatting and linux tests

* Formatting and linux tests

* Generate linux tests

* Use previous default max concurrent streams value of 10

* Fix crash if request is canceled after request header is send

* generate linux tests and run swift format

---------

Co-authored-by: Cory Benfield <lukasa@apple.com>
2023-02-10 15:41:26 +01:00
Franz Busch 24425989da Call didSendRequestPart after the write has hit the socket (#566)
### Motivation

Today `didSendRequestPart` is called after a request body part has been passed to the executor. However, this does not mean that the write hit the socket. Users may depend on this behavior to implement back-pressure. For this reason, we should only call this `didSendRequestPart` once the write was successful.

### Modification

Pass a promise to the actual channel write and only call the delegate once that promise succeeds.

### Result

The delegate method `didSendRequestPart` is only called after the write was successful. Fixes #565.

Co-authored-by: Fabian Fett <fabianfett@apple.com>
2022-04-26 16:04:54 +02:00
Fabian Fett 71af9c7fb7 Crash fix: HTTPClientRequestHandler can deal with failing writes (#558) 2022-02-22 13:59:54 +01:00
Fabian Fett 5844a6b4ee Crash fix: HTTP2 can handle requests are cancelled (#555)
Co-authored-by: George Barnett <gbarnett@apple.com>
2022-02-10 10:42:27 +01:00
Fabian Fett 474b23d677 Typo: Replace HTTPExecutingRequest with HTTPExecutableRequest (#514) 2021-12-03 17:10:55 +01:00
Fabian Fett 7617c35db3 Handle NIOSSLError.uncleanShutdown correctly (#472)
### Motivation

Fixes #238 and #231.

### Changes

- Extracted the unclean shutdown test from `HTTPClientTests` into their own file `HTTPClientUncleanSSLConnectionShutdownTests`
- Copy and pasted @weissi great explanation from #238 into the test file
- Removed property `ignoreUncleanSSLShutdown` everywhere

### Result

`ignoreUncleanSSLShutdown` on `HTTPClient.Configuration` is deprecated and ignored.

Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
2021-11-11 11:02:54 +01:00
David Nadoba d5bd8d6526 Always clear read idle timeout at the end of a request (#455) 2021-10-08 16:29:28 +02:00
Fabian Fett 64fbfdaeda [HTTP1Connection] Option to ignore unclean ssl shutdown errors (#432)
- a new `RequestOptions` struct was created, that can be used to set request specific options. It is required by the `HTTPExecutableRequest`
- Added support for `ignoreUncleanSSLShutdown` in the `HTTPRequestStateMachine` and the `HTTP1ConnectionStateMachine`. In http/2 `ignoreUncleanSSLShutdown` is always off.
2021-09-21 10:04:17 +02:00
Fabian Fett eab2a84b1c Use explicit NIO imports (#407)
* Use explicit NIO imports for `NIOCore`, `NIOPosix` and `NIOEmbedded`
* Updated dependencies
2021-08-19 21:11:49 +02:00
Fabian Fett 44efb9481b Cleanup: Connection cancel -> shutdown (#404)
* Cleanup

* Code review
2021-07-23 11:52:46 +01:00
Fabian Fett 388b8dc14a Add HTTP2Connection (#401)
Co-authored-by: George Barnett <gbarnett@apple.com>
2021-07-22 18:30:37 +02:00