Skip to content

Add idle body write detection to Netty HTTP client#6844

Open
zoewangg wants to merge 1 commit intomasterfrom
zoewang/WriteIdleTimeoutHandler
Open

Add idle body write detection to Netty HTTP client#6844
zoewangg wants to merge 1 commit intomasterfrom
zoewang/WriteIdleTimeoutHandler

Conversation

@zoewangg
Copy link
Copy Markdown
Contributor

@zoewangg zoewangg commented Apr 7, 2026

Motivation and Context

When using a request body publisher that is slow or unable to produce data, the connection may stay open for a long time after headers are sent. The existing WriteTimeoutHandler doesn't detect this because no write() operation is scheduled for it to time out on.

Modifications

  • Added WriteIdleTimeoutHandler that listens for WRITER_IDLE events from Netty's IdleStateHandler and proactively closes the connection with a descriptive error message
  • Added IdleStateHandler + WriteIdleTimeoutHandler to the channel pipeline in NettyRequestExecutor.writeRequest(), placed after the SSL handler (before HttpStreamsClientHandler) to avoid TLS record writes resetting the idle timer
  • Updated HandlerRemovingChannelPoolListener to clean up the new handlers on channel release

Testing

  • WriteIdleTimeoutHandlerTest — unit tests for the handler: verifies idle event fires exception and closes channel, non-writer events are ignored, and duplicate events don't fire twice
  • WriteIdleTimeoutTest — integration test with a TLS server and a stalled body publisher that never produces data, verifying the idle timeout fires within the expected window
  • HandlerRemovingChannelPoolListenerTest — updated to verify new handlers are cleaned up on channel release

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn clean install -pl :netty-nio-client succeeds
  • My code follows the code style of this project
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry

License

  • I confirm that this pull request can be released under the Apache 2 license

@zoewangg zoewangg requested a review from a team as a code owner April 7, 2026 16:31
@zoewangg zoewangg force-pushed the zoewang/WriteIdleTimeoutHandler branch 4 times, most recently from 92c38d9 to 03c9ac1 Compare April 7, 2026 20:55
@zoewangg zoewangg force-pushed the zoewang/WriteIdleTimeoutHandler branch from 03c9ac1 to 61a1147 Compare April 7, 2026 20:56
Comment on lines +296 to +298
channel.pipeline().addBefore(httpStreamsName, WRITE_IDLE_STATE_HANDLER_NAME,
new IdleStateHandler(0, context.configuration().writeTimeoutMillis(), 0,
TimeUnit.MILLISECONDS));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to give it a dedicated name because we use IdleStateHandler elsewhere and removing the handler based on class type would cause those handlers to be removed prematurely

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant