fix(interceptor/dump): fix handler lifecycle violation #4634
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.
Summary
Fixes a handler lifecycle violation in the dump interceptor that was causing it to call
super.onResponseEnd()prematurely from withinonResponseData().Problem
The original implementation called
super.onResponseEnd(controller, {})from withinonResponseData()whensize >= maxSize. This violates the handler lifecycle because:onResponseEnd()hasn't been called yet by the HTTP parserChanges
#dumped = truewhen size limit is reached, but don't callsuper.onResponseEnd()prematurely. Continue consuming data by returningtrue.super.onResponseEnd()when the actual HTTP response ends, checking the#dumpedflag to determine if we should pass empty trailers.this.#controller.aborted(consistent with original implementation)Known Issue
The tests still timeout due to a separate architectural issue:
When
Content-Lengthheader is set but the dump interceptor doesn't pass data to the body stream (because we're discarding it), the body stream inlib/api/api-request.jshangs waiting for data that will never arrive.This issue exists in the original implementation too (verified by testing the original code - it also times out).
Recommended Follow-up
The body stream implementation needs to be updated to handle the case where:
Content-Lengthheader is setonComplete()is called without receiving the expected amount of data viaonData()Or alternatively, update test expectations to not expect both
Content-Lengthpreservation AND empty body.Testing
Analyzed the issue using:
NODE_DEBUG=undici npx borp -p "test/interceptors/dump-interceptor.js" --timeout 5000The lifecycle fix is correct, but exposes the pre-existing body stream limitation.
🤖 Generated with Claude Code