docs: clarify close ownership when a recovery step turns a Success into a Failure#131
Merged
Merged
Conversation
…sform When a response or recovery step is handed a ResponseOutcome.Success and deliberately returns a different outcome (e.g. a status-to-typed-exception mapping that produces a Failure, or a substitute Success), the original successful Response is discarded. The ResponsePipeline only closes the in-hand response when a step throws while holding it; on the return-a-different-outcome path it never observes the dropped response and does not close it. Document that the step performing the transform owns closing the response it discards, in the KDoc of ResponseRecoveryStep and ResponsePipeline and in docs/pipelines.md. Closes #110
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
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.
Problem
The recovery-aware
ResponsePipelinelets a step transform aResponseOutcome.Successinto aFailure(e.g. status-to-typed-exception mapping) or substitute a differentSuccess. The original successfulResponseholds an open transport connection / body stream, but until now nothing documented who is responsible for closing it on that transform path. The actual behavior was easy to misread: it is tempting to assume the pipeline drains/closes any response it stops surfacing.Actual behavior (now documented)
The
ResponsePipelinecloses the in-handSuccessresponse on exactly one path: when a step throws while holding it. Both the success-path (applyResponseSteps) and the recovery chain (invokeRecovery) close-before-propagate and attach any close error to the step's throwable as suppressed.It does not close the response when a step is handed a
Successand deliberately returns a different outcome — aSuccess → Failuretransform or a substituteSuccess. The pipeline never observes the dropped response, so the step that performs the transform owns closing the response it discards (outcome.response.close()before returning the replacement), or the connection leaks. TheFailure(t1) → Failure(t2)"Replace" path carries no response, so there is nothing to close there.Change
Documentation only:
ResponseRecoveryStepKDoc — new "Close ownership when discarding a Success" section.ResponsePipelineKDoc — new "Close ownership of a discarded Success response" section describing the throw-only close path.docs/pipelines.md— matching subsection under theResponsePipelinedescription.No code behavior changes;
:sdk-corecompile, ktlint, detekt, and apiCheck pass.Closes #110