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.