fix(netty): fail fast when InputStream body cannot be reset by arimu1 · Pull Request #2312 · AsyncHttpClient/async-http-client

arimu1 · 2026-08-15T03:25:21Z

Summary

On HTTP/1, a consumed InputStream request body used to log a warning and return, leaving NettyRequestSender.writeRequest without completing the future so the request hung until timeout (Failed stream request hangs until timeout #1973). Headers used channel.write with no flush; the peer has seen nothing.
Throw IOException instead so writeRequest aborts the future. A failed reset() on a closed markable stream (for example BufferedInputStream after WriteProgressListener closes it) is treated the same way, so retries fail fast instead of hanging or throwing only "Stream closed".
HTTP/1 100 Continue now uses the same bodyWasDeferred guard as HTTP/2, so an unsolicited 100 (RFC 9110 15.2.1) does not replay a body that was already sent.
Add regression coverage for write, writeHttp2, the writeRequest abort path, and a real request replay.

Fixes #1973

AI disclosure

Composer 2.5 on behalf of arimu1 (Cursor harness). Commit includes Co-Authored-By: Composer 2.5 per AGENTS.md.

Test plan

./mvnw -pl client -Dtest=NettyInputStreamBodyTest,Continue100InterceptorTest,NettyRequestSenderConsumedBodyRetryTest test (JDK 17)

Made with Cursor

HTTP/1 path silently returned after a warn when a consumed non-resettable InputStream body was reused, leaving a half-sent request that hung until timeout. Throw IOException so sendHttpRequest aborts the future (matches existing HTTP/2 behavior). Composer 2.5 on behalf of arimu1 Fixes AsyncHttpClient#1973 Co-Authored-By: Composer 2.5 <composer@cursor.com>

Composer 2.5 on behalf of arimu1 Co-Authored-By: Composer 2.5 <composer@cursor.com>

hyperxpro · 2026-08-15T20:40:08Z

+                // hang until it times out (the Issue #1973 silent-timeout class). A non-resettable
+                // InputStream cannot be replayed (retry / redirect / auth), so fail explicitly: the caller
+                // (sendHttpRequest) aborts the future on the IOException.
+                throw new IOException("HTTP/1 request body InputStream already consumed and cannot be reset for a retry");


Continue100Interceptor line 59 to 68 re-invokes requestSender.writeRequest(future, channel) on any interim 100, and unlike the HTTP/2 branch right below it, it does not check bodyWasDeferred.

So take a POST with a non-markable stream and no Expect: 100-continue: the body goes out on the first pass and streamConsumed is set, then the server sends an unsolicited 100 before the real 200 (allowed, RFC 9110 15.2.1). We come back through here, markSupported() is false, and now we throw instead of warning. writeRequest catches it and calls abort, which fails the future and closes the channel. That request succeeds on main today.

Simplest fix is to give the HTTP/1 branch the same bodyWasDeferred guard the HTTP/2 branch already has, so we only replay a body we actually deferred.

hyperxpro · 2026-08-15T20:40:08Z

@@ -65,8 +61,12 @@ public void write(Channel channel, NettyResponseFuture<?> future) throws IOExcep
            if (is.markSupported()) {


markSupported() isn't quite the question we want to ask. The WriteProgressListener.operationComplete override at line 79 calls closeSilently(is) after the first write, so on a retry a BufferedInputStream reports markSupported() == true, takes the reset() branch on a closed stream with no prior mark(), and dies with "Stream closed". The user never sees the nice new message.

In practice ByteArrayInputStream is about the only thing that survives that branch, since its close() is a no-op. So the fix lands on the markSupported() == false half only, which is worth calling out in the description.

hyperxpro · 2026-08-15T20:40:08Z

@@ -65,8 +61,12 @@ public void write(Channel channel, NettyResponseFuture<?> future) throws IOExcep
            if (is.markSupported()) {
                is.reset();


Not introduced by this PR, but it lives on the line you're touching so flagging it while we're here. On the same unsolicited 100 path with a resettable body, we reset() and write the whole payload plus a second LastHttpContent onto a connection where LastHttpContent was already flushed at line 83. On a keep-alive socket the peer reads those trailing bytes as a pipelined request.

The bodyWasDeferred guard suggested above closes this too, which is a nice bonus for fixing it that way.

hyperxpro · 2026-08-15T20:40:08Z

            } else {
-                LOGGER.warn("Stream has already been consumed and cannot be reset");
-                return;
+                // The request headers were already written (sendHttpRequest), so silently returning would


sendHttpRequest doesn't exist anywhere in the tree. The method that writes the HTTP/1 headers and catches this IOException is NettyRequestSender.writeRequest (headers at line 732, catch and abort at 751 to 754).

The HTTP/2 comment this was modelled on cites real methods, so this one reads like a real symbol and sends people looking for something that isn't there. Same name appears in the PR description.

hyperxpro · 2026-08-15T20:40:08Z

-                LOGGER.warn("Stream has already been consumed and cannot be reset");
-                return;
+                // The request headers were already written (sendHttpRequest), so silently returning would
+                // leave the request half-sent with no terminating LastHttpContent — the request would then


Two nits on this line.

The "request half-sent" premise isn't right for HTTP/1: writeRequest line 732 uses channel.write(...) with no flush, and the only flush on this path is the writeAndFlush(LastHttpContent...) at line 83 that we skip by throwing. There is no FlushConsolidationHandler in the pipeline either, so the peer has seen nothing at all. The actual reason the fix is needed is that the future never completes, which is a cleaner thing to say and matters if someone later asks whether the connection has to be destroyed.

Also this uses an em dash. AGENTS.md asks for ASCII only in repository content, so please use a double hyphen or a comma.

hyperxpro · 2026-08-15T20:40:08Z

                // The HEADERS frame was already written with endStream=false (sendHttp2Frames), so silently
                // returning would leave the stream half-open with no terminating DATA frame — the request
-                // would then hang until it times out (the Issue #2160 silent-timeout class). A non-resettable
+                // would then hang until it times out (the Issue #1973 silent-timeout class). A non-resettable


Could you revert this one? The HTTP/2 half-open-stream timeout class really is #2160, and NettyRequestSender still cites #2160 at lines 778 and 790 for the same thing. #1973 is the HTTP/1 consumed-body report.

After this change the three sibling comments disagree with each other, and it's an unrelated edit in an otherwise focused fix.

hyperxpro · 2026-08-15T20:40:08Z

+
+        EmbeddedChannel channel = new EmbeddedChannel();
+        try {
+            IOException ex = assertThrows(IOException.class, () -> body.write(channel, future));


The test asserts that write() throws, but the bug in #1973 is that the request hangs until the timeout. Calling body.write(...) directly skips NettyRequestSender.writeRequest entirely, so it wouldn't catch someone later swallowing this IOException, or reaching abort() without completing the future. A test that drives a real request through a retry and asserts the future fails promptly would actually guard the symptom.

Also currently uncovered: the markSupported() == true reset branch (the one that's still broken, see the other comment), the writeHttp2 sibling path whose comment this PR edits, and whether the stream gets closed. And asserting the exact message string couples us to the prose.

hyperxpro · 2026-08-15T20:40:08Z

+        }
+    }
+
+    private static final class NonResettableInputStream extends InputStream {


This helper never gets exercised. We throw at the markSupported() check before any read happens, so data, index and read() are all dead, and InputStream.markSupported() already returns false by default. On JDK 11 the whole class collapses to new NettyInputStreamBody(InputStream.nullInputStream()).

Unsolicited HTTP/1 100 Continue was replaying bodies that were already sent. Guard with bodyWasDeferred like HTTP/2. Treat a failed reset() on a closed markable stream as unreplayable so retries abort the future instead of hanging or throwing Stream closed. Comments now cite NettyRequestSender.writeRequest. Addresses review on AsyncHttpClient#2312. Composer 2.5 on behalf of arimu1 Co-Authored-By: Composer 2.5 <composer@cursor.com>

arimu1 · 2026-08-16T00:30:57Z

Addressed the CHANGES_REQUESTED review (commit da62c2fd1):

HTTP/1 unsolicited 100 -- Continue100Interceptor now has the same bodyWasDeferred guard as HTTP/2. We only schedule writeRequest when the body was actually deferred for Expect: 100-continue.
markSupported() / closed stream -- replayConsumedStream still tries reset() when mark is supported, but wraps a failed reset (closed BufferedInputStream, no prior mark()) in a clear IOException so the user does not see a bare "Stream closed".
Unsolicited 100 + resettable body / second LastHttpContent -- closed by the same bodyWasDeferred guard.
sendHttpRequest -- comments and the PR description now cite NettyRequestSender.writeRequest (headers ~732, catch/abort ~751-754).
Throw-comment nits -- HTTP/1 comment now states that headers used channel.write with no flush (peer has seen nothing) and that the real hang is the future never completing. ASCII only (-- / commas, no em dash).
HTTP/2 issue number -- restored #2160 on the half-open-stream comment. #1973 stays on the HTTP/1 consumed-body path.
Tests -- NettyRequestSender.writeRequest is driven twice (unconsumed, then consumed) and asserts the future fails within 2s. Integration replay of a real POST covers the #1973 hang. Also covered: markSupported()==true reset-fail (BufferedInputStream after close), writeHttp2 sibling, stream closed after the first write. No exact exception-message asserts. Non-markable branch uses InputStream.nullInputStream() (JDK 11).

Happy to follow up if anything still looks off.

arimu1 and others added 2 commits August 15, 2026 10:10

fix(netty): drop unused LOGGER after InputStream fail-fast

992b0a0

Composer 2.5 on behalf of arimu1 Co-Authored-By: Composer 2.5 <composer@cursor.com>

hyperxpro requested changes Aug 15, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(netty): fail fast when InputStream body cannot be reset - #2312

fix(netty): fail fast when InputStream body cannot be reset#2312
arimu1 wants to merge 3 commits into
AsyncHttpClient:mainfrom
arimu1:fix/1973-consumed-stream-http1-fail-fast

arimu1 commented Aug 15, 2026 •

edited

Loading

Uh oh!

hyperxpro Aug 15, 2026

Uh oh!

hyperxpro Aug 15, 2026

Uh oh!

hyperxpro Aug 15, 2026

Uh oh!

hyperxpro Aug 15, 2026

Uh oh!

hyperxpro Aug 15, 2026

Uh oh!

hyperxpro Aug 15, 2026

Uh oh!

hyperxpro Aug 15, 2026

Uh oh!

hyperxpro Aug 15, 2026

Uh oh!

arimu1 commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -65,8 +61,12 @@ public void write(Channel channel, NettyResponseFuture<?> future) throws IOExcep
		if (is.markSupported()) {

		@@ -65,8 +61,12 @@ public void write(Channel channel, NettyResponseFuture<?> future) throws IOExcep
		if (is.markSupported()) {
		is.reset();

Uh oh!

Conversation

arimu1 commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

AI disclosure

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arimu1 commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arimu1 commented Aug 15, 2026 •

edited

Loading