Skip to content

Conversation

JoshMock
Copy link
Member

@JoshMock JoshMock commented Jun 3, 2025

In HttpConnection, the 'close' handler for a response was being cleaned up implicitly, since all its listeners should be dropped when the object is garbage collected at the end of a request/response cycle. However, as @pmuellr noted here, explicitly cleaning it up removes ambiguity and the potential for unexpected edge case errors when a system is under heavy load.

This also updates the error message provided to ConnectionError in a 'close' event to distinguish it from another similar circumstance that can also raise an EPIPE error.

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM, if we continue to see the "Response aborted ..." messages that we've been seeing for ~2 weeks, we'll at least know which flavor it is :-)

My understanding is that close is not just emitted for errors, but when the underlying "thing" closes. Hopefully if our issues were edge cases because the close events happened to slip in before the response was finished processing, and then signaled an error, this won't happen any more (or as much) since we are now removing the listener explicitly when finishing processing the response.

@JoshMock JoshMock merged commit 0da9511 into main Jun 4, 2025
17 checks passed
@JoshMock JoshMock deleted the close-listener branch June 4, 2025 18:13
Copy link

github-actions bot commented Jun 4, 2025

The backport to 8.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-8.x 8.x
# Navigate to the new working tree
cd .worktrees/backport-8.x
# Create a new branch
git switch --create backport-266-to-8.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 0da9511e119f900ead92bdcc07d35bd1d9b43c48
# Push it to GitHub
git push --set-upstream origin backport-266-to-8.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.x

Then, create a pull request where the base branch is 8.x and the compare/head branch is backport-266-to-8.x.

JoshMock added a commit that referenced this pull request Jun 4, 2025
* Clean up response 'close' event

* Update exception messages for EPIPE

* Ensure both EPIPE error messages are handled by test

* D'oh
JoshMock added a commit that referenced this pull request Jun 4, 2025
* Clean up response 'close' event (#266)

* Clean up response 'close' event

* Update exception messages for EPIPE

* Ensure both EPIPE error messages are handled by test

* D'oh

* Patch unit tests
@rudolf rudolf added the bug Something isn't working label Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 8.x bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants