From 1d8d52145380710490f595799710ab8d3dd3946f Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Wed, 17 Jun 2026 04:41:33 +0300 Subject: [PATCH] docs: document close ownership for the Success->Failure recovery transform 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 --- docs/pipelines.md | 23 +++++++++++++++++++ .../sdk/core/pipeline/ResponsePipeline.kt | 12 ++++++++++ .../pipeline/step/ResponseRecoveryStep.kt | 13 +++++++++++ 3 files changed, 48 insertions(+) diff --git a/docs/pipelines.md b/docs/pipelines.md index 49ed0e7a..4b223147 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -232,6 +232,29 @@ fed to the next recovery step — recovery exceptions never bypass downstream re pipeline's `apply` method never throws; callers inspect the returned outcome to decide whether to surface a `Response` or rethrow. +#### Close ownership: discarding a `Success` response + +When a step is handed a `ResponseOutcome.Success`, the wrapped `Response` holds an open +transport connection / body stream that must be closed exactly once. The `ResponsePipeline` +takes that responsibility on **only one path: when a step throws while holding the response.** +Both the success-path (`applyResponseSteps`) and the recovery chain (`invokeRecovery`) +close-before-propagate — they close the in-hand response and attach any close error to the +step's throwable as suppressed, so a throwing step never strands the connection. + +The pipeline does **not** close the response on the path where a step is handed a `Success` and +*deliberately returns a different outcome*: + +- **Success → Failure transform** — e.g. a status-to-typed-exception recovery step that turns a + `Success(response)` into a `Failure(HttpException)`. The original successful `response` is + discarded. +- **Success → substitute Success** — replacing the response with a different one. + +On both of these the pipeline never observes the dropped response, so **the step that performs +the transform owns closing the response it discards**. Such a step must call +`outcome.response.close()` on the response it is dropping before returning the replacement +outcome, or the connection leaks. (The recovery "Replace" path — `Failure(t1)` → `Failure(t2)` +— operates on a `Failure`, which carries no response, so there is nothing to close there.) + ### ResponseOutcome ```kotlin diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/ResponsePipeline.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/ResponsePipeline.kt index 1f10bf24..6b253af3 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/ResponsePipeline.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/ResponsePipeline.kt @@ -49,6 +49,18 @@ import java.util.Collections * the returned outcome to decide whether to surface a [org.dexpace.sdk.core.http.response.Response] * or rethrow. * + * ## Close ownership of a discarded Success response + * The pipeline closes the in-hand [ResponseOutcome.Success] response on exactly one path: when + * a step *throws* while holding it. Both the success-path (`applyResponseSteps`) and the + * recovery chain (`invokeRecovery`) close-before-propagate, attaching any close error to the + * step's throwable as suppressed, so a throwing step never strands the open transport + * connection. The pipeline does **not** close the response on the path where a step is handed a + * [ResponseOutcome.Success] and deliberately *returns* a different outcome — a Success→Failure + * transform (e.g. status-to-typed-exception mapping) or a substitute [ResponseOutcome.Success]. + * Returning a new outcome discards the original response without the pipeline observing it, so + * the step that performs that transform owns closing the response it drops. See + * [org.dexpace.sdk.core.pipeline.step.ResponseRecoveryStep] for the per-step contract. + * * @property responseSteps Steps applied on the success path; skipped on failures. * @property recoverySteps Recovery steps applied to every outcome. */ diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/ResponseRecoveryStep.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/ResponseRecoveryStep.kt index 189f4d74..3f64cd7f 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/ResponseRecoveryStep.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/ResponseRecoveryStep.kt @@ -32,6 +32,19 @@ import org.dexpace.sdk.core.pipeline.ResponseOutcome * [ResponseOutcome.Failure] and feeds it to the next recovery step. Implementations should * still avoid this path — surface errors as [ResponseOutcome.Failure] explicitly. * + * ## Close ownership when discarding a Success + * The [org.dexpace.sdk.core.pipeline.ResponsePipeline] closes the in-hand + * [ResponseOutcome.Success] response on exactly one path: when a step *throws*. A step that is + * handed a [ResponseOutcome.Success] and instead deliberately **returns** a different outcome + * — a [ResponseOutcome.Failure] (the Success→Failure transform, e.g. status-to-typed-exception + * mapping) or a different [ResponseOutcome.Success] (response substitution) — discards the + * original response, and the pipeline does **not** close it for you. That original + * [ResponseOutcome.Success.response] holds an open transport connection / body stream, so the + * step that drops it **owns closing it**: call `outcome.response.close()` on the response you + * are discarding before returning the replacement outcome, or the connection leaks. The + * "Replace" path above operates on a [ResponseOutcome.Failure], which carries no response, so + * there is nothing to close there. + * * ## Thread-safety * Steps are shared across concurrent requests. Implementations must be safe to invoke from * multiple threads.