fix(teeny-request): avoid piping into destroyed streams#8671
Open
rockwotj wants to merge 1 commit into
Open
Conversation
Stream-mode requests defer pipeline setup until both the consumer starts reading and the fetch response arrives. A response listener can destroy the returned request stream before teeny-request's later response listener runs. retry-request follows this sequence when it discards a retryable response. Passing that destroyed destination to pipeline() rejects with ERR_STREAM_UNABLE_TO_PIPE outside the caller's stream error handling. The unused fetch response body also remains open. Centralize response pipeline setup and recheck the request stream immediately before constructing the pipeline. When the destination is already destroyed, destroy the response body instead so its underlying HTTP resources are released without surfacing an unhandled rejection. Add a regression that reproduces the response-listener ordering and waits for the discarded body to close. The full teeny-request suite passes on Node 18, 22, 24, and 26 with 116 passing tests and 2 existing pending tests. The gts lint check and a retry-request 500-to-200 integration exercise also pass. Fixes googleapis#8670
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the stream piping logic in teeny-request by introducing a helper function pipeResponseStream and adds a check to destroy the response stream if the request stream is already destroyed. A test case is also added to verify this behavior. A review comment suggests using optional chaining when calling responseStream.destroy() to prevent potential runtime errors if responseStream is undefined.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #8670
Summary
Stream-mode requests defer pipeline setup until both the consumer starts reading and the fetch response arrives. A
responselistener can destroy the returned request stream before the later teeny-request listener constructs the pipeline.retry-requestfollows this sequence when it discards a retryable response before starting another attempt.Passing the destroyed destination to
pipeline()produces an unhandledERR_STREAM_UNABLE_TO_PIPE. The unused fetch response body also remains open.This change centralizes response pipeline setup and checks the destination immediately before constructing the pipeline. If the request stream is already destroyed, teeny-request destroys the unused response body instead.
A regression test reproduces the real listener ordering and verifies that the discarded body closes.
Testing
ERR_STREAM_UNABLE_TO_PIPE; response body remained open.npm test: 116 passing, 2 existing pending.npm run lintpassed.retry-requestintegration passed: a 500 response was discarded, the request retried, and the 200 response body was returned without an unhandled rejection.