diff --git a/README.md b/README.md index 57e1fe7f..806a97c8 100644 --- a/README.md +++ b/README.md @@ -247,8 +247,8 @@ See [docs/pipelines.md](docs/pipelines.md) for the step-author walkthrough. |---|---| | `client` | `HttpClient`, `AsyncHttpClient` — the two transport SPIs (sync and async). | | `http.request` | `Request`, `RequestBody`, `FileRequestBody`, `LoggableRequestBody`, `Method`. | -| `http.response` | `Response`, `ResponseBody`, `LoggableResponseBody`, `Status` (a value-carrying class with a total `fromCode`), `HttpResponseException`, plus the raw-vs-parsed seam: `ResponseHandler` (with dep-free `string()`/`empty()` handlers) and a lazy, parse-once `ParsedResponse`. | -| `http.response.exception` | Typed `HttpException` hierarchy (`BadRequestException`, `RequestTimeoutException`, `TooManyRequestsException`, `ServiceUnavailableException`, …) with `retryable` derived from `RetryUtils.isRetryable`, plus `NetworkException` and `HttpExceptionFactory`. | +| `http.response` | `Response`, `ResponseBody`, `LoggableResponseBody`, `Status` (a value-carrying class with a total `fromCode`), plus the raw-vs-parsed seam: `ResponseHandler` (with dep-free `string()`/`empty()` handlers) and a lazy, parse-once `ParsedResponse`. | +| `http.response.exception` | Typed `HttpException` hierarchy (`BadRequestException`, `RequestTimeoutException`, `TooManyRequestsException`, `ServiceUnavailableException`, …) with `isRetryable` derived from `RetryUtils.isRetryable` and exposed via the `Retryable` interface, plus `NetworkException` and `HttpExceptionFactory`. | | `http.common` | `Headers`, `HttpHeaderName` (interned), `MediaType`, `Protocol`, `HttpRange`, `ETag`, `RequestConditions`. | | `http.context` | `CallContext` → `DispatchContext` → `RequestContext` → `ExchangeContext` chain, `ContextStore`. | | `http.pipeline` | Sync (`HttpStep` / `HttpPipeline` / `HttpPipelineBuilder` / `PipelineNext` / `Stage`) and async (`AsyncHttpStep` / `AsyncHttpPipeline` / `AsyncHttpPipelineBuilder` / `AsyncPipelineNext`) pipeline machinery, plus `AsyncPipelineBridges`. | diff --git a/docs/architecture.md b/docs/architecture.md index 42cbe61a..73a923f1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -582,8 +582,8 @@ responds in a uniform way: subsequent blocking call also surfaces it. 3. Throws `InterruptedIOException` (or the operation's natural failure exception with `InterruptedException` added as a suppressed cause). -4. Classifies the interruption as **non-retryable** — `HttpResponseException.isRetryable` - returns `false` for an interrupt-driven failure. +4. Classifies the interruption as **non-retryable** — an interrupt-driven failure is not a + `Retryable` condition, so the retry step never re-issues it. Loops bounded by user input (retry attempts, paged iteration, server-sent-event consumption, drain loops in body logging) check `Thread.currentThread().isInterrupted` at @@ -660,8 +660,8 @@ they should construct a fresh one. |----------------------|-------------------------------------------------------------------------------------------------| | `io` | Source, Sink, BufferedSource, BufferedSink, Buffer, IoProvider, Io, TeeSink (internal) | | `http.request` | Request, RequestBody, FileRequestBody, LoggableRequestBody, Method | -| `http.response` | Response, ResponseBody, LoggableResponseBody, Status, HttpResponseException | -| `http.response.exception` | HttpException, HttpExceptionFactory, RequestTimeoutException (and siblings), NetworkException | +| `http.response` | Response, ResponseBody, LoggableResponseBody, Status | +| `http.response.exception` | HttpException, HttpExceptionFactory, Retryable, RequestTimeoutException (and siblings), NetworkException | | `http.common` | Headers, MediaType, CommonMediaTypes, Protocol, ETag, HttpRange, RequestConditions | | `http.auth` | Credential, KeyCredential, BearerToken, ChallengeHandler, Basic/Digest/CompositeChallengeHandler, AuthChallengeParser | | `http.context` | CallContext, DispatchContext, RequestContext, ExchangeContext, ContextStore | diff --git a/docs/http.md b/docs/http.md index 6c0c933d..18ae9fe9 100644 --- a/docs/http.md +++ b/docs/http.md @@ -286,8 +286,9 @@ on whether a code is one the SDK recognizes. The SDK ships a typed exception hierarchy under `org.dexpace.sdk.core.http.response.exception`. The shape mirrors `gax`'s `ApiException` taxonomy translated to HTTP terms: one base class plus one concrete subclass per canonical status code. The base class derives its retryable flag -from a single source of truth, so a downstream retry policy can read `exception.retryable` -instead of maintaining a parallel predicate map. +from a single source of truth and exposes it through the `Retryable` interface, so a downstream +retry policy can read `(t as? Retryable)?.isRetryable` instead of maintaining a parallel +predicate map or matching concrete exception types. ### HttpException Hierarchy @@ -302,16 +303,21 @@ abstract class HttpException( val body: ResponseBody?, // lazy — NOT eagerly buffered message: String? = null, cause: Throwable? = null, -) : RuntimeException(message, cause) { - val retryable: Boolean = RetryUtils.isRetryable(status.code) + val value: Any? = null, // slot for a deserialized error payload (generated layer) +) : RuntimeException(message, cause), Retryable { + override val isRetryable: Boolean = RetryUtils.isRetryable(status.code) fun bodySnapshot(maxBytes: Int = DEFAULT_SNAPSHOT_BYTES): ByteArray? } ``` -`retryable` is a `val` **derived** at construction from `RetryUtils.isRetryable(status.code)`, +`isRetryable` is a `val` **derived** at construction from `RetryUtils.isRetryable(status.code)`, not hardcoded per subclass and not a constructor parameter. This guarantees the baked flag can never disagree with the live retry policy: 408 / 429 and the 5xx range except 501 and 505 -are retryable, everything else is not. +are retryable, everything else is not. It satisfies the `Retryable` interface (shared with +`NetworkException`), which is what the retry step keys off. + +`value` is a slot for a deserialized error payload, left `null` here — `sdk-core` does not +parse bodies. The generated service layer populates it on a per-operation typed subclass. `bodySnapshot()` returns a non-consuming preview of the body bytes — it reads from a fresh `source().peek()` view so the primary read path is undisturbed — capped at `maxBytes` @@ -355,19 +361,21 @@ subclass: it extends `java.io.IOException` so existing `catch (IOException)` cal working, and it carries no status/headers/body because none arrived. ```kotlin -open class NetworkException(message: String? = null, cause: Throwable? = null) : IOException(message, cause) { - val retryable: Boolean = true // always retryable at the SDK level +open class NetworkException(message: String? = null, cause: Throwable? = null) : + IOException(message, cause), Retryable { + override val isRetryable: Boolean = true // always retryable at the SDK level } ``` -The `retryable` flag is always `true`: nothing reached the server, so the SDK can safely +The `isRetryable` flag is always `true`: nothing reached the server, so the SDK can safely attempt the request again. Whether the request itself is *safe* to retry (HTTP method idempotency, replayable body) is the retry policy's call, not this class's. ### HttpExceptionFactory -`HttpExceptionFactory.fromResponse(response)` maps a non-2xx `Response` to the right -subclass: +`HttpExceptionFactory.fromResponse(response)` maps a 4xx/5xx `Response` to the right subclass. +It is the one seam that turns an error response into a typed exception, so the family is +produced consistently rather than re-derived at each call site: ```kotlin val response = httpClient.execute(request) @@ -379,6 +387,14 @@ if (!response.status.isSuccess) { The factory throws `IllegalArgumentException` if called with a status outside 400..599 — 1xx/2xx/3xx outcomes are not exceptions and should not be funneled through this path. +For the recovery-aware pipeline, `ThrowOnHttpErrorStep` packages this mapping as a +`ResponsePipelineStep`: drop it into a `ResponsePipeline.responseSteps` list and it calls +`fromResponse` on a 4xx/5xx response and throws the result. `ResponsePipeline` converts that +throw into a `ResponseOutcome.Failure`, which then flows through the recovery chain (e.g. +`RetryStep`) exactly like a transport failure — and because the thrown `HttpException` is +`Retryable`, retry classification keys off it uniformly. The step is a building block; no +default pipeline in `sdk-core` assembles it for you. + --- ## Common Types diff --git a/docs/pipelines.md b/docs/pipelines.md index d066731e..49ed0e7a 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -356,11 +356,13 @@ public class RetryStep( ) ``` -It retries only when the outcome is a `Failure` whose throwable is classified retryable: +It retries only when the outcome is a `Failure` whose throwable is classified retryable. The +classifier keys off the `Retryable` interface, not concrete exception types: -- An `HttpException` with `retryable == true` whose status code is in +- An `HttpException` (which is `Retryable`) with `isRetryable == true` whose status code is in `RetrySettings.retryableStatuses`. -- A `NetworkException` (a transport failure with no response on the wire — always retryable). +- A `NetworkException` (also `Retryable`, always `true` — a transport failure with no response + on the wire). Idempotency is enforced independently of classification, keyed off whether the request carries a body. A request **with a body** is eligible only when its body is replayable (a non-replayable diff --git a/docs/refs-comparison.md b/docs/refs-comparison.md index d33df5ad..aea2b3b7 100644 --- a/docs/refs-comparison.md +++ b/docs/refs-comparison.md @@ -223,7 +223,7 @@ below records where each scheme's design was sourced from: **What shipped, and what's left:** 1. `HttpException` is the base, with status-code-keyed subclasses (`NotFoundException`, `UnauthorizedException`, `TooManyRequestsException`, `InternalServerErrorException`, etc.) plus `ClientErrorException`/`ServerErrorException` fallbacks for unmapped 4xx/5xx. `NetworkException` is a sibling for transport failures. The set mirrors gax's taxonomy scaled to HTTP statuses. -2. `retryable: Boolean` is a `val` derived once at construction from `RetryUtils.isRetryable(status.code)` — not a per-subclass constant. This is the single source of truth, so it can never disagree with the live retry policy (408 retryable; 501/505 not). A retry predicate is just `(t as? HttpException)?.retryable == true`. +2. `isRetryable: Boolean` (from the `Retryable` interface) is a `val` derived once at construction from `RetryUtils.isRetryable(status.code)` — not a per-subclass constant. This is the single source of truth, so it can never disagree with the live retry policy (408 retryable; 501/505 not). `NetworkException` implements the same interface (always `true`), so a retry predicate keys off the interface: `(t as? Retryable)?.isRetryable == true`. 3. The base exposes `status` + `headers` + a **lazy** `body: ResponseBody?` (not eagerly buffered), plus a non-consuming `bodySnapshot()` that reads from a `peek()` view so the primary read path is undisturbed — large 5xx bodies don't OOM. Per-operation per-status subclasses carrying typed bodies (Expedia pattern) are still codegen's job. 4. Tolerant error-body parsing (Square's `SquareApiException.parseErrors`) remains future work — never throw inside an exception constructor; pass through the raw body on parse failure. diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index b4ec3744..d8b4eae4 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -1129,16 +1129,6 @@ public final class org/dexpace/sdk/core/http/request/RequestBody$Companion { public static synthetic fun create$default (Lorg/dexpace/sdk/core/http/request/RequestBody$Companion;[BLorg/dexpace/sdk/core/http/common/MediaType;ILjava/lang/Object;)Lorg/dexpace/sdk/core/http/request/RequestBody; } -public class org/dexpace/sdk/core/http/response/HttpResponseException : java/io/IOException { - public fun (Ljava/lang/String;Lorg/dexpace/sdk/core/http/response/Response;)V - public fun (Ljava/lang/String;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/Object;)V - public fun (Ljava/lang/String;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/Object;Ljava/lang/Throwable;)V - public synthetic fun (Ljava/lang/String;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/Object;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public final fun getResponse ()Lorg/dexpace/sdk/core/http/response/Response; - public final fun getValue ()Ljava/lang/Object; - public final fun isRetryable ()Z -} - public final class org/dexpace/sdk/core/http/response/LoggableResponseBody : org/dexpace/sdk/core/http/response/ResponseBody { public fun (Lorg/dexpace/sdk/core/http/response/ResponseBody;)V public fun (Lorg/dexpace/sdk/core/http/response/ResponseBody;Lorg/dexpace/sdk/core/io/IoProvider;)V @@ -1336,65 +1326,74 @@ public class org/dexpace/sdk/core/http/response/exception/BadGatewayException : public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/BadRequestException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/ClientErrorException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/ConflictException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/ForbiddenException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/GatewayTimeoutException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/GoneException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } -public abstract class org/dexpace/sdk/core/http/response/exception/HttpException : java/lang/RuntimeException { +public abstract class org/dexpace/sdk/core/http/response/exception/HttpException : java/lang/RuntimeException, org/dexpace/sdk/core/http/response/exception/Retryable { public static final field Companion Lorg/dexpace/sdk/core/http/response/exception/HttpException$Companion; public static final field DEFAULT_SNAPSHOT_BYTES I public fun (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;)V public fun (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun bodySnapshot ()[B public final fun bodySnapshot (I)[B public static synthetic fun bodySnapshot$default (Lorg/dexpace/sdk/core/http/response/exception/HttpException;IILjava/lang/Object;)[B public final fun getBody ()Lorg/dexpace/sdk/core/http/response/ResponseBody; public final fun getHeaders ()Lorg/dexpace/sdk/core/http/common/Headers; - public final fun getRetryable ()Z public final fun getStatus ()Lorg/dexpace/sdk/core/http/response/Status; + public final fun getValue ()Ljava/lang/Object; + public fun isRetryable ()Z } public final class org/dexpace/sdk/core/http/response/exception/HttpException$Companion { @@ -1403,91 +1402,108 @@ public final class org/dexpace/sdk/core/http/response/exception/HttpException$Co public final class org/dexpace/sdk/core/http/response/exception/HttpExceptionFactory { public static final field INSTANCE Lorg/dexpace/sdk/core/http/response/exception/HttpExceptionFactory; public static final fun fromResponse (Lorg/dexpace/sdk/core/http/response/Response;)Lorg/dexpace/sdk/core/http/response/exception/HttpException; + public static final fun fromResponseOrNull (Lorg/dexpace/sdk/core/http/response/Response;)Lorg/dexpace/sdk/core/http/response/exception/HttpException; + public static final fun isErrorStatus (I)Z } public class org/dexpace/sdk/core/http/response/exception/InternalServerErrorException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/MethodNotAllowedException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } -public class org/dexpace/sdk/core/http/response/exception/NetworkException : java/io/IOException { +public class org/dexpace/sdk/core/http/response/exception/NetworkException : java/io/IOException, org/dexpace/sdk/core/http/response/exception/Retryable { public fun ()V public fun (Ljava/lang/String;)V public fun (Ljava/lang/String;Ljava/lang/Throwable;)V public synthetic fun (Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public final fun getRetryable ()Z + public fun isRetryable ()Z } public class org/dexpace/sdk/core/http/response/exception/NotFoundException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/PayloadTooLargeException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/RequestTimeoutException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V +} + +public abstract interface class org/dexpace/sdk/core/http/response/exception/Retryable { + public abstract fun isRetryable ()Z } public class org/dexpace/sdk/core/http/response/exception/ServerErrorException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/ServiceUnavailableException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/TooManyRequestsException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/UnauthorizedException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/UnprocessableEntityException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public class org/dexpace/sdk/core/http/response/exception/UnsupportedMediaTypeException : org/dexpace/sdk/core/http/response/exception/HttpException { public fun (Lorg/dexpace/sdk/core/http/response/Response;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;)V - public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;)V + public synthetic fun (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V } public final class org/dexpace/sdk/core/http/sse/ServerSentEvent { @@ -2159,6 +2175,12 @@ public abstract interface class org/dexpace/sdk/core/pipeline/step/ResponseRecov public abstract fun invoke (Lorg/dexpace/sdk/core/pipeline/ResponseOutcome;)Lorg/dexpace/sdk/core/pipeline/ResponseOutcome; } +public final class org/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStep : org/dexpace/sdk/core/pipeline/step/ResponsePipelineStep { + public static final field INSTANCE Lorg/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStep; + public synthetic fun execute (Ljava/lang/Object;Lorg/dexpace/sdk/core/http/context/DispatchContext;)Ljava/lang/Object; + public fun execute (Lorg/dexpace/sdk/core/http/response/Response;Lorg/dexpace/sdk/core/http/context/DispatchContext;)Lorg/dexpace/sdk/core/http/response/Response; +} + public final class org/dexpace/sdk/core/pipeline/step/retry/BackoffCalculator { public static final field INSTANCE Lorg/dexpace/sdk/core/pipeline/step/retry/BackoffCalculator; public static final fun computeDelay (ILorg/dexpace/sdk/core/pipeline/step/retry/RetrySettings;)Ljava/time/Duration; diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/HttpResponseException.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/HttpResponseException.kt deleted file mode 100644 index 7e4d7102..00000000 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/HttpResponseException.kt +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright (c) 2026 dexpace and Omar Aljarrah - * - * Licensed under the MIT License. See LICENSE in the project root. - * SPDX-License-Identifier: MIT - */ - -package org.dexpace.sdk.core.http.response - -import org.dexpace.sdk.core.util.RetryUtils -import java.io.IOException - -/** - * An [IOException] that carries an HTTP [Response] (when available) and an optional - * deserialized error [value]. The exception classifies its own retryability at - * construction time so the retry policy can query a precomputed field rather than - * maintaining a parallel predicate. - * - * Retryability rules (see [RetryUtils]): - * - If [response] is non-null, [isRetryable] reflects [RetryUtils.isRetryable] for the - * response status code: 408, 429, or 5xx excluding 501 and 505. - * - Otherwise, if [cause] is non-null, [isRetryable] is `true` when the cause chain - * contains an [IOException] or [java.util.concurrent.TimeoutException]. - * - If both are null, [isRetryable] is `false`. - * - * Extends [IOException] so it propagates cleanly through APIs declaring - * `@Throws(IOException::class)`. Marked `open` so service clients can introduce typed - * subclasses; the [isRetryable] field is `final` and cannot be overridden. - * - * @param message Human-readable error description. - * @property response Response that triggered the error, or `null` when the failure happened - * before a response was received. - * @property value Optional deserialized payload describing the error (e.g. a JSON error body). - * @param cause Underlying cause to chain into the [IOException], or `null`. - */ -public open class HttpResponseException - @JvmOverloads - constructor( - message: String, - public val response: Response?, - public val value: Any? = null, - cause: Throwable? = null, - ) : IOException(message, cause) { - /** - * Whether this exception represents a retryable condition. Computed once at - * construction; querying is free. - */ - public val isRetryable: Boolean = computeIsRetryable(response, cause) - - private companion object { - private fun computeIsRetryable( - response: Response?, - cause: Throwable?, - ): Boolean { - response?.let { return RetryUtils.isRetryable(it.status.code) } - cause?.let { return RetryUtils.isRetryable(it) } - return false - } - } - } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpException.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpException.kt index afea41aa..121976a1 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpException.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpException.kt @@ -25,14 +25,18 @@ import java.io.IOException * so service-client codegen can introduce per-operation typed subclasses by deriving from a * concrete one and stamping a typed deserialized payload. * + * This is the single response-carrying exception base. It is a [RuntimeException], so callers + * are not forced to declare `throws` for protocol-level failures; transport-level failures use + * [NetworkException] (an `IOException` sibling) instead. + * * ## Retryability * - * [retryable] is a `val` derived at construction time from [RetryUtils.isRetryable] applied - * to [status]'s code — it is *not* hardcoded per subclass. This guarantees the baked flag can - * never disagree with the live retry policy: 408 / 429 and the 5xx range (except 501 and 505) - * are retryable, every other status is not. A downstream retry policy is expected to query - * this field rather than maintaining a parallel predicate map. Transport-level - * [NetworkException] is a sibling type and always retryable. + * [HttpException] implements [Retryable]. [isRetryable] is a `val` derived at construction time + * from [RetryUtils.isRetryable] applied to [status]'s code — it is *not* hardcoded per subclass. + * This guarantees the baked flag can never disagree with the live retry policy: 408 / 429 and + * the 5xx range (except 501 and 505) are retryable, every other status is not. A downstream + * retry policy queries the [Retryable] interface rather than maintaining a parallel predicate + * map. Transport-level [NetworkException] is a sibling type and always retryable. * * ## Body access * @@ -41,6 +45,10 @@ import java.io.IOException * non-consuming preview suitable for log lines. The snapshot uses * [org.dexpace.sdk.core.io.BufferedSource.peek] so the primary read path is undisturbed. * + * [value] is a slot for a deserialized error payload (e.g. a parsed JSON error object). It is + * `null` here; the generated service layer is expected to populate it on a per-operation typed + * subclass once it has deserialized [body]. + * * ## Why `RuntimeException` and not `IOException` * * A successful transport produced a response — the failure is at the protocol level, not at @@ -51,7 +59,9 @@ import java.io.IOException * @property status Parsed HTTP status from the originating response. * @property headers Response headers as received on the wire. * @property body Lazy response body, or `null` when the response carries no payload. - * @property retryable Whether the exception represents a retryable condition. Derived once at + * @property value Optional deserialized payload describing the error, or `null` when none was + * parsed. Reserved for the generated layer to stamp a typed error object. + * @property isRetryable Whether the exception represents a retryable condition. Derived once at * construction from [RetryUtils.isRetryable] over [status]'s code, so it always mirrors the * live retry policy rather than a per-subclass constant. */ @@ -63,13 +73,14 @@ public abstract class HttpException public val body: ResponseBody?, message: String? = null, cause: Throwable? = null, - ) : RuntimeException(message ?: defaultMessage(status), cause) { + public val value: Any? = null, + ) : RuntimeException(message ?: defaultMessage(status), cause), Retryable { /** * Whether this exception represents a retryable condition. Derived from * [RetryUtils.isRetryable] over [status]'s code so it can never disagree with the * live retry policy. */ - public val retryable: Boolean = RetryUtils.isRetryable(status.code) + override val isRetryable: Boolean = RetryUtils.isRetryable(status.code) /** * Returns a non-consuming preview of the body bytes, capped at [maxBytes]. diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptionFactory.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptionFactory.kt index 3b58869e..4c912bd4 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptionFactory.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptionFactory.kt @@ -65,6 +65,26 @@ public object HttpExceptionFactory { private const val SC_SERVER_ERROR_MIN = 500 private const val SC_SERVER_ERROR_MAX = 599 + /** + * Whether [code] is in the 400..599 error range this factory maps to an [HttpException]. + * + * This is the single source of truth for the error-status boundary; callers that want to + * pre-check before mapping (e.g. a pipeline step that only acts on error responses) should + * use this rather than re-declaring the range. + */ + @JvmStatic + public fun isErrorStatus(code: Int): Boolean = code in SC_CLIENT_ERROR_MIN..SC_SERVER_ERROR_MAX + + /** + * Maps [response] to its [HttpException] subclass, or returns `null` when the status is not + * an error (1xx / 2xx / 3xx). Unlike [fromResponse], a non-error status is a no-op rather + * than an [IllegalArgumentException], so this is the convenient form for "map it if it is + * an error" call sites. + */ + @JvmStatic + public fun fromResponseOrNull(response: Response): HttpException? = + if (isErrorStatus(response.status.code)) fromResponse(response) else null + /** * Returns the [HttpException] subclass that corresponds to [response]'s status code. * diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptions.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptions.kt index 7587ee1f..4db3194c 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptions.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptions.kt @@ -18,7 +18,7 @@ import org.dexpace.sdk.core.http.response.Response // that stamps a deserialized error payload — see `docs/refs-comparison.md` Error Model §, // action item 3. // -// None of these subclasses sets its own retryable flag: `HttpException.retryable` is derived +// None of these subclasses sets its own retryable flag: `HttpException.isRetryable` is derived // from `RetryUtils.isRetryable(status.code)` (the single source of truth), so the baked flag // always mirrors the live retry policy — 408 / 429 and the 5xx range except 501 / 505 are // retryable, everything else is not. Fallback subclasses (`ClientErrorException`, @@ -35,12 +35,14 @@ public open class BadRequestException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -54,12 +56,14 @@ public open class UnauthorizedException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -73,12 +77,14 @@ public open class ForbiddenException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -91,12 +97,14 @@ public open class NotFoundException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -109,12 +117,14 @@ public open class MethodNotAllowedException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -128,12 +138,14 @@ public open class RequestTimeoutException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -147,12 +159,14 @@ public open class ConflictException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -165,12 +179,14 @@ public open class GoneException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -183,12 +199,14 @@ public open class PayloadTooLargeException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -201,12 +219,14 @@ public open class UnsupportedMediaTypeException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -220,12 +240,14 @@ public open class UnprocessableEntityException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -239,12 +261,14 @@ public open class TooManyRequestsException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -258,12 +282,14 @@ public open class InternalServerErrorException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -276,12 +302,14 @@ public open class BadGatewayException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -295,12 +323,14 @@ public open class ServiceUnavailableException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -313,12 +343,14 @@ public open class GatewayTimeoutException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -334,12 +366,14 @@ public open class ClientErrorException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) /** @@ -355,10 +389,12 @@ public open class ServerErrorException response: Response, message: String? = null, cause: Throwable? = null, + value: Any? = null, ) : HttpException( status = response.status, headers = response.headers, body = response.body, message = message, cause = cause, + value = value, ) diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/NetworkException.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/NetworkException.kt index 3fc8182a..b1a259e0 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/NetworkException.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/NetworkException.kt @@ -23,24 +23,25 @@ import java.io.IOException * * ## Retryability * - * Always [retryable] = `true`. A transport-level failure with no response on the wire is by - * definition transient from the SDK's standpoint: nothing was processed by the server, so a - * retry can attempt the request from scratch. Whether the operation is *safe* to retry - * still depends on idempotency (HTTP method + replayable body) — that decision lives at the - * retry step, not here. + * Implements [Retryable] with [isRetryable] always `true`. A transport-level failure with no + * response on the wire is by definition transient from the SDK's standpoint: nothing was + * processed by the server, so a retry can attempt the request from scratch. Whether the + * operation is *safe* to retry still depends on idempotency (HTTP method + replayable body) — + * that decision lives at the retry step, not here. * - * @property retryable Always `true`; exposed as a `val` for API symmetry with [HttpException]. + * @property isRetryable Always `true`; satisfies [Retryable] so the retry classifier treats + * transport failures uniformly with retryable [HttpException]s. */ public open class NetworkException @JvmOverloads constructor( message: String? = null, cause: Throwable? = null, - ) : IOException(message, cause) { + ) : IOException(message, cause), Retryable { /** * Whether this exception represents a retryable condition. Always `true` for * `NetworkException` — transport failures with no response are by definition * transient at the SDK level. */ - public val retryable: Boolean = true + override val isRetryable: Boolean = true } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/Retryable.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/Retryable.kt new file mode 100644 index 00000000..d1811d34 --- /dev/null +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/exception/Retryable.kt @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2026 dexpace and Omar Aljarrah + * + * Licensed under the MIT License. See LICENSE in the project root. + * SPDX-License-Identifier: MIT + */ + +package org.dexpace.sdk.core.http.response.exception + +/** + * Marks an exception that knows whether the condition it represents is worth retrying. + * + * The retry machinery keys off this interface rather than concrete exception types: a + * [org.dexpace.sdk.core.pipeline.step.retry.RetryStep] asks `error is Retryable` and reads + * [isRetryable] instead of matching every subclass it might encounter. New retryable + * exception types participate automatically by implementing this interface — no change to the + * classifier is required. + * + * Two SDK exceptions implement it today: + * - [HttpException] — a response was received; [isRetryable] is derived once at construction + * from the status code via [org.dexpace.sdk.core.util.RetryUtils.isRetryable]. + * - [NetworkException] — the transport failed before any response arrived; [isRetryable] is + * always `true` (nothing reached the server, so a fresh attempt is safe at the SDK level). + * + * A `true` flag means the *condition* is transient. Whether a specific request is *safe* to + * replay (HTTP-method idempotency, replayable body) is an orthogonal decision owned by the + * retry step, not by this flag. + */ +public interface Retryable { + /** Whether the condition this exception represents is a retryable one. */ + public val isRetryable: Boolean +} diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStep.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStep.kt new file mode 100644 index 00000000..21326999 --- /dev/null +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStep.kt @@ -0,0 +1,137 @@ +/* + * Copyright (c) 2026 dexpace and Omar Aljarrah + * + * Licensed under the MIT License. See LICENSE in the project root. + * SPDX-License-Identifier: MIT + */ + +package org.dexpace.sdk.core.pipeline.step + +import org.dexpace.sdk.core.http.context.DispatchContext +import org.dexpace.sdk.core.http.response.Response +import org.dexpace.sdk.core.http.response.ResponseBody +import org.dexpace.sdk.core.http.response.exception.HttpException +import org.dexpace.sdk.core.http.response.exception.HttpExceptionFactory +import org.dexpace.sdk.core.io.BufferedSource +import org.dexpace.sdk.core.io.Io + +/** + * Response step that turns an HTTP error response into the matching typed [HttpException]. + * + * This is the error path for the recovery-aware pipeline: a transport hands back a `Response` + * for every completed exchange — including 4xx and 5xx — so a 4xx/5xx is a [Response], not a + * thrown exception, until something maps it. Placed in a + * [org.dexpace.sdk.core.pipeline.ResponsePipeline.responseSteps] list, this step performs that + * mapping via [HttpExceptionFactory.fromResponse], so the typed [HttpException] family is + * produced consistently from one seam rather than re-derived ad hoc at each call site. + * + * ## Behaviour + * + * - **2xx (and 1xx/3xx) response** — returned unchanged; the success chain continues. + * - **4xx / 5xx response** — [HttpExceptionFactory.fromResponse] is invoked and the resulting + * [HttpException] is thrown. [org.dexpace.sdk.core.pipeline.ResponsePipeline] catches it, + * closes the in-hand response, and wraps it into a + * [org.dexpace.sdk.core.pipeline.ResponseOutcome.Failure] — which then flows through the + * recovery chain (e.g. [org.dexpace.sdk.core.pipeline.step.retry.RetryStep]) exactly like a + * transport failure. Because the thrown [HttpException] implements + * [org.dexpace.sdk.core.http.response.exception.Retryable], retry classification keys off it + * uniformly. + * + * Range classification is single-sourced from [HttpExceptionFactory.isErrorStatus]: only + * 400..599 maps to an exception, so the success / informational / redirection ranges fall + * through untouched. + * + * ## Error-body buffering + * + * The pipeline closes the in-hand response as soon as this step throws (close-before-propagate), + * which would also close the live [ResponseBody]. To keep the error body readable on the + * resulting [org.dexpace.sdk.core.pipeline.ResponseOutcome.Failure] — for a log-line + * [HttpException.bodySnapshot] or downstream typed-error deserialization — this step buffers the + * error body into a small, replayable in-memory body before constructing the exception. The + * buffer is bounded at [HttpException.DEFAULT_SNAPSHOT_BYTES], consistent with the + * `bodySnapshot` cap, so a multi-megabyte 5xx body cannot OOM the process. Mapping the buffered + * body into a typed error value is still left to the generated layer (see [HttpException.value]). + * + * The cap is a hard truncation, not just an OOM guard: an error body larger than + * [HttpException.DEFAULT_SNAPSHOT_BYTES] is preserved only up to that many bytes, and the excess + * is discarded with no marker. This is harmless for a log-line snapshot, but a downstream + * consumer that deserializes the buffered body (e.g. the generated layer parsing it into + * [HttpException.value]) must tolerate a structurally incomplete payload — a JSON object cut + * mid-token will not parse. Callers that need the full error body must read it from the live + * response before this step runs rather than relying on the buffered copy. + * + * ## Thread-safety + * + * Stateless; safe to share across concurrent requests and reuse as a singleton. + */ +public object ThrowOnHttpErrorStep : ResponsePipelineStep { + /** + * Returns [input] unchanged for a non-error status; throws the + * [HttpExceptionFactory.fromResponse] mapping for a 4xx / 5xx status. + * + * @throws HttpException when `input.status.code` is in 400..599. + */ + override fun execute( + input: Response, + context: DispatchContext, + ): Response { + if (!HttpExceptionFactory.isErrorStatus(input.status.code)) { + return input + } + throw HttpExceptionFactory.fromResponse(bufferErrorBody(input)) + } + + /** + * Returns a copy of [response] whose body has been drained into a bounded, replayable + * in-memory body, so it survives the pipeline closing the original response. Bodies that are + * absent are left as-is. + */ + private fun bufferErrorBody(response: Response): Response { + val body = response.body ?: return response + val buffered = replayableBody(body) + return response.newBuilder().body(buffered).build() + } + + /** + * Reads up to [HttpException.DEFAULT_SNAPSHOT_BYTES] from [body] into memory and returns a + * [ResponseBody] that can be read repeatedly from those bytes. The original [body] is read + * here (the byte budget is bounded), so the caller must not also stream it. + */ + private fun replayableBody(body: ResponseBody): ResponseBody { + val cap = HttpException.DEFAULT_SNAPSHOT_BYTES + val mediaType = body.mediaType() + val bytes = + body.source().use { source -> + readUpTo(source, cap) + } + return object : ResponseBody() { + override fun mediaType() = mediaType + + override fun contentLength(): Long = bytes.size.toLong() + + override fun source(): BufferedSource = Io.provider.source(bytes) + + override fun close() { + // Bytes are held in memory; nothing transport-owned to release. + } + } + } + + /** Reads at most [cap] bytes from [source], returning however many were available. */ + private fun readUpTo( + source: BufferedSource, + cap: Int, + ): ByteArray { + if (cap == 0) return ByteArray(0) + val out = ByteArray(cap) + var read = 0 + source.inputStream().use { stream -> + while (read < cap) { + val n = stream.read(out, read, cap - read) + if (n < 0) break + read += n + } + } + return if (read == cap) out else out.copyOf(read) + } +} diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetrySettings.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetrySettings.kt index 26c1e124..9d29be75 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetrySettings.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetrySettings.kt @@ -274,7 +274,7 @@ public class RetrySettings * timeout). * * This is the recovery-aware step's allow-list; [RetryStep] intersects it with - * [HttpException.retryable][org.dexpace.sdk.core.http.response.exception.HttpException.retryable] + * [HttpException.isRetryable][org.dexpace.sdk.core.http.response.exception.HttpException.isRetryable] * (which is itself derived from [RetryUtils.isRetryable][org.dexpace.sdk.core.util.RetryUtils.isRetryable]). * With this default set the recovery-aware step retries the same common statuses the * stage-based [DefaultRetryStep][org.dexpace.sdk.core.http.pipeline.steps.DefaultRetryStep] diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetryStep.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetryStep.kt index e5e42cfc..2bae727b 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetryStep.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetryStep.kt @@ -12,6 +12,7 @@ import org.dexpace.sdk.core.http.request.Request import org.dexpace.sdk.core.http.response.Response import org.dexpace.sdk.core.http.response.exception.HttpException import org.dexpace.sdk.core.http.response.exception.NetworkException +import org.dexpace.sdk.core.http.response.exception.Retryable import org.dexpace.sdk.core.pipeline.ResponseOutcome import org.dexpace.sdk.core.pipeline.step.ResponseRecoveryStep import java.io.InterruptedIOException @@ -31,14 +32,16 @@ import java.util.concurrent.TimeUnit * * ## Semantics * - * - On a [ResponseOutcome.Failure] whose throwable is an [HttpException] with `retryable = + * Retry eligibility is classified off the [Retryable] interface, not concrete exception types: + * - On a [ResponseOutcome.Failure] whose throwable is an [HttpException] with `isRetryable = * true` AND whose status is in [RetrySettings.retryableStatuses], the step schedules a * retry — provided the request is idempotency-safe (see [canRetry]). - * - On a [ResponseOutcome.Failure] whose throwable is a [NetworkException], the step always - * schedules a retry (network-level failures had no response, so the server never saw the - * request — replay is safe modulo the body-replayability check). - * - On any other outcome (other throwable types or a [ResponseOutcome.Success]), the step - * is a pass-through. + * - On a [ResponseOutcome.Failure] whose throwable is a [NetworkException] (always + * [Retryable.isRetryable]), the step always schedules a retry (network-level failures had no + * response, so the server never saw the request — replay is safe modulo the body- + * replayability check). + * - On any other outcome (a non-[Retryable] throwable, a [Retryable] whose flag is `false`, or + * a [ResponseOutcome.Success]), the step is a pass-through. * * ## Idempotency * @@ -366,15 +369,18 @@ public class RetryStep } /** - * Returns true when [error] is an SDK-classified retryable condition AND, for - * [HttpException], the status is in [RetrySettings.retryableStatuses]. + * Returns true when [error] is an SDK-classified retryable condition. Classification + * keys off the [Retryable] interface rather than concrete exception types: a non- + * [Retryable] throwable is never retried, and a [Retryable] is retried only when its + * [Retryable.isRetryable] flag is set. An [HttpException] carries an additional gate — + * its status must also be in [RetrySettings.retryableStatuses] — so the caller can + * narrow which retryable statuses this step actually re-issues. A [NetworkException] + * (no response, hence no status) passes once its flag is set. */ - private fun isClassifiedRetryable(error: Throwable): Boolean = - when (error) { - is NetworkException -> error.retryable - is HttpException -> error.retryable && settings.retryableStatuses.contains(error.status.code) - else -> false - } + private fun isClassifiedRetryable(error: Throwable): Boolean { + if (error !is Retryable || !error.isRetryable) return false + return error !is HttpException || settings.retryableStatuses.contains(error.status.code) + } /** * Returns true when [request] is safe to retry. A body-less request is retry-safe only diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/util/RetryUtils.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/util/RetryUtils.kt index 51bfdf0e..b31008e5 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/util/RetryUtils.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/util/RetryUtils.kt @@ -11,9 +11,10 @@ import java.io.IOException import java.util.concurrent.TimeoutException /** - * Shared retryability classifier used by both [org.dexpace.sdk.core.http.response.HttpResponseException] - * (to precompute its `isRetryable` flag at construction time) and the default retry - * policy (Rank 4). + * Shared retryability classifier used by both + * [org.dexpace.sdk.core.http.response.exception.HttpException] (to derive its + * [org.dexpace.sdk.core.http.response.exception.Retryable.isRetryable] flag at construction + * time) and the default retry policy (Rank 4). * * Retryable conditions: * - **Status codes**: 408 (Request Timeout), 429 (Too Many Requests), all 5xx except diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/response/HttpResponseExceptionTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/response/HttpResponseExceptionTest.kt deleted file mode 100644 index 32377a48..00000000 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/response/HttpResponseExceptionTest.kt +++ /dev/null @@ -1,163 +0,0 @@ -/* - * Copyright (c) 2026 dexpace and Omar Aljarrah - * - * Licensed under the MIT License. See LICENSE in the project root. - * SPDX-License-Identifier: MIT - */ - -package org.dexpace.sdk.core.http.response - -import org.dexpace.sdk.core.http.common.Protocol -import org.dexpace.sdk.core.http.request.Method -import org.dexpace.sdk.core.http.request.Request -import java.io.IOException -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertFalse -import kotlin.test.assertNull -import kotlin.test.assertSame -import kotlin.test.assertTrue - -class HttpResponseExceptionTest { - @Test - fun `null response and null cause means not retryable`() { - val ex = HttpResponseException("boom", response = null) - assertFalse(ex.isRetryable) - assertNull(ex.response) - assertNull(ex.cause) - assertNull(ex.value) - } - - @Test - fun `200 OK response is not retryable`() { - val ex = HttpResponseException("ok-but-thrown", response = response(Status.OK)) - assertFalse(ex.isRetryable) - } - - @Test - fun `503 Service Unavailable response is retryable`() { - val ex = HttpResponseException("unavailable", response = response(Status.SERVICE_UNAVAILABLE)) - assertTrue(ex.isRetryable) - } - - @Test - fun `501 Not Implemented response is NOT retryable`() { - // 501 is explicitly excluded from the retryable 5xx range. - val ex = HttpResponseException("not impl", response = response(Status.NOT_IMPLEMENTED)) - assertFalse(ex.isRetryable) - } - - @Test - fun `505 HTTP Version Not Supported response is NOT retryable`() { - val ex = HttpResponseException("bad version", response = response(Status.HTTP_VERSION_NOT_SUPPORTED)) - assertFalse(ex.isRetryable) - } - - @Test - fun `cause is IOException with null response is retryable`() { - val cause = IOException("network") - val ex = HttpResponseException("io failed", response = null, cause = cause) - assertTrue(ex.isRetryable) - assertSame(cause, ex.cause) - } - - @Test - fun `cause chain wraps IOException is retryable`() { - val root = IOException("root") - val middle = RuntimeException("middle", root) - val outer = IllegalStateException("outer", middle) - val ex = HttpResponseException("nested", response = null, cause = outer) - assertTrue(ex.isRetryable) - } - - @Test - fun `response takes precedence over cause when response status is non-retryable`() { - // Response is 200 (not retryable) but cause is IOException (would be retryable). - // Response wins because it is non-null — matches the spec's response-first rule. - val ex = - HttpResponseException( - "ok with io cause", - response = response(Status.OK), - cause = IOException("io"), - ) - assertFalse(ex.isRetryable) - } - - @Test - fun `self-referential cause chain does not infinite-loop`() { - val selfCausing = SelfReferentialException("loop") - selfCausing.setSelfCause() - // Construction must terminate; isRetryable must be false (no IOException in the chain). - val ex = HttpResponseException("self loop", response = null, cause = selfCausing) - assertFalse(ex.isRetryable) - } - - @Test - fun `value parameter is preserved`() { - val payload = mapOf("error" to "rate-limited") - val ex = - HttpResponseException( - "payload", - response = response(Status.TOO_MANY_REQUESTS), - value = payload, - ) - assertSame(payload, ex.value) - assertTrue(ex.isRetryable) // 429 is retryable. - } - - @Test - fun `extends IOException`() { - val ex = HttpResponseException("io", response = null) - // Catchable by IOException — the whole point of inheriting from it. - val asIo: IOException = ex - assertEquals("io", asIo.message) - } - - @Test - fun `message and cause are propagated to IOException constructor`() { - val cause = RuntimeException("root") - val ex = HttpResponseException("oops", response = null, cause = cause) - assertEquals("oops", ex.message) - assertSame(cause, ex.cause) - } - - @Test - fun `subclasses can be defined and inherit isRetryable behavior`() { - class TypedHttpResponseException(message: String, response: Response?, cause: Throwable?) : - HttpResponseException(message, response, value = null, cause = cause) - - val ex = - TypedHttpResponseException( - "typed", - response = response(Status.SERVICE_UNAVAILABLE), - cause = null, - ) - assertTrue(ex.isRetryable) - } - - // ---- Helpers ----------------------------------------------------------------------- - - private fun response(status: Status): Response { - val req = - Request.builder() - .method(Method.GET) - .url("https://api.example.com/") - .build() - return Response.builder() - .request(req) - .protocol(Protocol.HTTP_1_1) - .status(status) - .build() - } - - private class SelfReferentialException(message: String) : RuntimeException(message) { - private var selfCause: Boolean = false - - fun setSelfCause() { - selfCause = true - } - - override val cause: Throwable? - get() = if (selfCause) this else null - } -} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptionFactoryTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptionFactoryTest.kt index 2e5dfd1b..d05ef86e 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptionFactoryTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/response/exception/HttpExceptionFactoryTest.kt @@ -27,6 +27,7 @@ import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertNotNull import kotlin.test.assertNull +import kotlin.test.assertSame import kotlin.test.assertTrue class HttpExceptionFactoryTest { @@ -51,7 +52,7 @@ class HttpExceptionFactoryTest { ex.javaClass, "factory must dispatch status ${status.code} (${status.statusName}) to ${expectedClass.simpleName}", ) - assertEquals(expectedRetryable, ex.retryable, "${status.code} retryable flag mismatch") + assertEquals(expectedRetryable, ex.isRetryable, "${status.code} retryable flag mismatch") assertEquals(status, ex.status) } } @@ -64,11 +65,11 @@ class HttpExceptionFactoryTest { // should fall through to the 4xx generic. Same for the auth-handshake-style 407. val teapot = HttpExceptionFactory.fromResponse(response(Status.IM_A_TEAPOT)) assertTrue(teapot is ClientErrorException, "418 should be ClientErrorException") - assertEquals(false, teapot.retryable) + assertEquals(false, teapot.isRetryable) val proxyAuth = HttpExceptionFactory.fromResponse(response(Status.PROXY_AUTHENTICATION_REQUIRED)) assertTrue(proxyAuth is ClientErrorException, "407 should be ClientErrorException") - assertEquals(false, proxyAuth.retryable) + assertEquals(false, proxyAuth.isRetryable) } @Test @@ -77,21 +78,21 @@ class HttpExceptionFactoryTest { // RetryUtils (which excludes 501/505) — NOT the old blanket "5xx is retryable" rule. val notImpl = HttpExceptionFactory.fromResponse(response(Status.NOT_IMPLEMENTED)) assertTrue(notImpl is ServerErrorException, "501 should be ServerErrorException") - assertEquals(false, notImpl.retryable, "501 must not be retryable (RetryUtils excludes it)") - assertEquals(RetryUtils.isRetryable(Status.NOT_IMPLEMENTED.code), notImpl.retryable) + assertEquals(false, notImpl.isRetryable, "501 must not be retryable (RetryUtils excludes it)") + assertEquals(RetryUtils.isRetryable(Status.NOT_IMPLEMENTED.code), notImpl.isRetryable) val versionBad = HttpExceptionFactory.fromResponse(response(Status.HTTP_VERSION_NOT_SUPPORTED)) assertTrue(versionBad is ServerErrorException, "505 should be ServerErrorException") - assertEquals(false, versionBad.retryable, "505 must not be retryable (RetryUtils excludes it)") - assertEquals(RetryUtils.isRetryable(Status.HTTP_VERSION_NOT_SUPPORTED.code), versionBad.retryable) + assertEquals(false, versionBad.isRetryable, "505 must not be retryable (RetryUtils excludes it)") + assertEquals(RetryUtils.isRetryable(Status.HTTP_VERSION_NOT_SUPPORTED.code), versionBad.isRetryable) } @Test fun `408 maps to RequestTimeoutException and is retryable`() { val ex = HttpExceptionFactory.fromResponse(response(Status.REQUEST_TIMEOUT)) assertTrue(ex is RequestTimeoutException, "408 should map to RequestTimeoutException, not the 4xx fallback") - assertEquals(true, ex.retryable, "408 must be retryable per RetryUtils") - assertEquals(RetryUtils.isRetryable(Status.REQUEST_TIMEOUT.code), ex.retryable) + assertEquals(true, ex.isRetryable, "408 must be retryable per RetryUtils") + assertEquals(RetryUtils.isRetryable(Status.REQUEST_TIMEOUT.code), ex.isRetryable) assertEquals(Status.REQUEST_TIMEOUT, ex.status) } @@ -130,7 +131,7 @@ class HttpExceptionFactoryTest { val ex = HttpExceptionFactory.fromResponse(response(status)) assertEquals( RetryUtils.isRetryable(status.code), - ex.retryable, + ex.isRetryable, "baked retryable for ${status.code} (${status.statusName}) must equal RetryUtils.isRetryable", ) } @@ -160,36 +161,72 @@ class HttpExceptionFactoryTest { @Test fun `retryable flag matches the canonical retryable status set`() { // 4xx subclasses except 429 — not retryable. - assertEquals(false, BadRequestException(response(Status.BAD_REQUEST)).retryable) - assertEquals(false, UnauthorizedException(response(Status.UNAUTHORIZED)).retryable) - assertEquals(false, ForbiddenException(response(Status.FORBIDDEN)).retryable) - assertEquals(false, NotFoundException(response(Status.NOT_FOUND)).retryable) - assertEquals(false, MethodNotAllowedException(response(Status.METHOD_NOT_ALLOWED)).retryable) - assertEquals(false, ConflictException(response(Status.CONFLICT)).retryable) - assertEquals(false, GoneException(response(Status.GONE)).retryable) - assertEquals(false, PayloadTooLargeException(response(Status.PAYLOAD_TOO_LARGE)).retryable) - assertEquals(false, UnsupportedMediaTypeException(response(Status.UNSUPPORTED_MEDIA_TYPE)).retryable) - assertEquals(false, UnprocessableEntityException(response(Status.UNPROCESSABLE_ENTITY)).retryable) - assertEquals(false, ClientErrorException(response(Status.IM_A_TEAPOT)).retryable) + assertEquals(false, BadRequestException(response(Status.BAD_REQUEST)).isRetryable) + assertEquals(false, UnauthorizedException(response(Status.UNAUTHORIZED)).isRetryable) + assertEquals(false, ForbiddenException(response(Status.FORBIDDEN)).isRetryable) + assertEquals(false, NotFoundException(response(Status.NOT_FOUND)).isRetryable) + assertEquals(false, MethodNotAllowedException(response(Status.METHOD_NOT_ALLOWED)).isRetryable) + assertEquals(false, ConflictException(response(Status.CONFLICT)).isRetryable) + assertEquals(false, GoneException(response(Status.GONE)).isRetryable) + assertEquals(false, PayloadTooLargeException(response(Status.PAYLOAD_TOO_LARGE)).isRetryable) + assertEquals(false, UnsupportedMediaTypeException(response(Status.UNSUPPORTED_MEDIA_TYPE)).isRetryable) + assertEquals(false, UnprocessableEntityException(response(Status.UNPROCESSABLE_ENTITY)).isRetryable) + assertEquals(false, ClientErrorException(response(Status.IM_A_TEAPOT)).isRetryable) // 408 + 429 + the retryable 5xx codes — retryable. - assertEquals(true, RequestTimeoutException(response(Status.REQUEST_TIMEOUT)).retryable) - assertEquals(true, TooManyRequestsException(response(Status.TOO_MANY_REQUESTS)).retryable) - assertEquals(true, InternalServerErrorException(response(Status.INTERNAL_SERVER_ERROR)).retryable) - assertEquals(true, BadGatewayException(response(Status.BAD_GATEWAY)).retryable) - assertEquals(true, ServiceUnavailableException(response(Status.SERVICE_UNAVAILABLE)).retryable) - assertEquals(true, GatewayTimeoutException(response(Status.GATEWAY_TIMEOUT)).retryable) + assertEquals(true, RequestTimeoutException(response(Status.REQUEST_TIMEOUT)).isRetryable) + assertEquals(true, TooManyRequestsException(response(Status.TOO_MANY_REQUESTS)).isRetryable) + assertEquals(true, InternalServerErrorException(response(Status.INTERNAL_SERVER_ERROR)).isRetryable) + assertEquals(true, BadGatewayException(response(Status.BAD_GATEWAY)).isRetryable) + assertEquals(true, ServiceUnavailableException(response(Status.SERVICE_UNAVAILABLE)).isRetryable) + assertEquals(true, GatewayTimeoutException(response(Status.GATEWAY_TIMEOUT)).isRetryable) // 501 routes through the 5xx fallback but is NOT retryable — the baked flag now // mirrors RetryUtils instead of the old blanket 5xx rule. - assertEquals(false, ServerErrorException(response(Status.NOT_IMPLEMENTED)).retryable) - assertEquals(false, ServerErrorException(response(Status.HTTP_VERSION_NOT_SUPPORTED)).retryable) + assertEquals(false, ServerErrorException(response(Status.NOT_IMPLEMENTED)).isRetryable) + assertEquals(false, ServerErrorException(response(Status.HTTP_VERSION_NOT_SUPPORTED)).isRetryable) } @Test fun `NetworkException is always retryable`() { val ex = NetworkException("connect refused") - assertEquals(true, ex.retryable) + assertEquals(true, ex.isRetryable) + } + + // ---- 4b. Retryable interface + error-body value slot -------------------------------- + + @Test + fun `HttpException and NetworkException both implement Retryable`() { + val http: Throwable = HttpExceptionFactory.fromResponse(response(Status.SERVICE_UNAVAILABLE)) + val net: Throwable = NetworkException("connect failed") + // Both surface their retry classification through the shared Retryable seam, so the + // retry classifier can key off the interface instead of matching concrete types. + assertTrue(Retryable::class.java.isInstance(http), "HttpException must implement Retryable") + assertTrue(Retryable::class.java.isInstance(net), "NetworkException must implement Retryable") + assertEquals(true, (http as Retryable).isRetryable) + assertEquals(true, (net as Retryable).isRetryable) + } + + @Test + fun `factory-produced exception carries a null value slot by default`() { + // The deserialized error-body slot exists for the generated layer to populate; the + // factory leaves it null because sdk-core does not parse bodies. + val ex = HttpExceptionFactory.fromResponse(response(Status.BAD_REQUEST)) + assertNull(ex.value, "value slot must default to null") + } + + @Test + fun `value slot round-trips a deserialized error payload`() { + val payload = mapOf("code" to "rate_limited", "message" to "slow down") + val ex = + object : HttpException( + status = Status.TOO_MANY_REQUESTS, + headers = Headers.Builder().build(), + body = null, + value = payload, + ) {} + assertSame(payload, ex.value, "value must be carried through as the same instance") + assertEquals(true, ex.isRetryable, "429 is retryable") } // ---- 5. bodySnapshot() behavior ----------------------------------------------------- diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStepTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStepTest.kt new file mode 100644 index 00000000..5938923f --- /dev/null +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStepTest.kt @@ -0,0 +1,207 @@ +/* + * Copyright (c) 2026 dexpace and Omar Aljarrah + * + * Licensed under the MIT License. See LICENSE in the project root. + * SPDX-License-Identifier: MIT + */ + +package org.dexpace.sdk.core.pipeline.step + +import org.dexpace.sdk.core.http.common.Headers +import org.dexpace.sdk.core.http.common.Protocol +import org.dexpace.sdk.core.http.context.DispatchContext +import org.dexpace.sdk.core.http.request.Method +import org.dexpace.sdk.core.http.request.Request +import org.dexpace.sdk.core.http.response.Response +import org.dexpace.sdk.core.http.response.ResponseBody +import org.dexpace.sdk.core.http.response.Status +import org.dexpace.sdk.core.http.response.exception.HttpException +import org.dexpace.sdk.core.http.response.exception.NotFoundException +import org.dexpace.sdk.core.http.response.exception.Retryable +import org.dexpace.sdk.core.http.response.exception.ServiceUnavailableException +import org.dexpace.sdk.core.http.response.exception.TooManyRequestsException +import org.dexpace.sdk.core.io.BufferedSource +import org.dexpace.sdk.core.io.Io +import org.dexpace.sdk.core.pipeline.ResponseOutcome +import org.dexpace.sdk.core.pipeline.ResponsePipeline +import org.dexpace.sdk.io.OkioIoProvider +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertIs +import kotlin.test.assertNotNull +import kotlin.test.assertSame +import kotlin.test.assertTrue + +/** + * Verifies that [ThrowOnHttpErrorStep] is the live error path: an error response is routed + * through [org.dexpace.sdk.core.http.response.exception.HttpExceptionFactory.fromResponse] and + * surfaces as the matching typed [HttpException], while a success response passes through. + */ +class ThrowOnHttpErrorStepTest { + @BeforeTest + fun installProvider() { + Io.installProvider(OkioIoProvider) + } + + // ---- the step in isolation ---------------------------------------------------------- + + @Test + fun `2xx response passes through unchanged`() { + val ok = response(Status.OK) + val out = ThrowOnHttpErrorStep.execute(ok, ctx()) + assertSame(ok, out, "a success response must pass through untouched") + } + + @Test + fun `3xx response passes through unchanged`() { + // The factory rejects non-4xx/5xx; the step must not even call it for a 3xx. + val redirect = response(Status.MOVED_PERMANENTLY) + assertSame(redirect, ThrowOnHttpErrorStep.execute(redirect, ctx())) + } + + @Test + fun `404 maps to NotFoundException via the factory`() { + val ex = + assertFailsWith { + ThrowOnHttpErrorStep.execute(response(Status.NOT_FOUND), ctx()) + } + assertIs(ex, "404 must produce the factory's NotFoundException") + assertEquals(Status.NOT_FOUND, ex.status) + } + + @Test + fun `503 maps to a retryable ServiceUnavailableException`() { + val ex = + assertFailsWith { + ThrowOnHttpErrorStep.execute(response(Status.SERVICE_UNAVAILABLE), ctx()) + } + assertIs(ex) + assertTrue(ex.isRetryable, "503 must surface as retryable") + } + + @Test + fun `error exception carries the response headers and status`() { + val headers = Headers.Builder().add("Retry-After", "7").build() + val ex = + assertFailsWith { + ThrowOnHttpErrorStep.execute(response(Status.TOO_MANY_REQUESTS, headers), ctx()) + } + assertIs(ex) + assertEquals("7", ex.headers.get("Retry-After")) + assertEquals(Status.TOO_MANY_REQUESTS, ex.status) + } + + // ---- wired through the recovery-aware ResponsePipeline ------------------------------ + + @Test + fun `pipeline turns a 500 success-outcome into a Failure carrying the factory exception`() { + val pipeline = ResponsePipeline(responseSteps = listOf(ThrowOnHttpErrorStep)) + val outcome = ResponseOutcome.Success(response(Status.INTERNAL_SERVER_ERROR)) + + val result = pipeline.apply(outcome, ctx()) + + val failure = assertIs(result, "a 5xx must become a Failure") + val error = assertIs(failure.error) + assertEquals(Status.INTERNAL_SERVER_ERROR, error.status) + } + + @Test + fun `pipeline leaves a 2xx success-outcome as a Success`() { + val pipeline = ResponsePipeline(responseSteps = listOf(ThrowOnHttpErrorStep)) + val ok = response(Status.OK) + + val result = pipeline.apply(ResponseOutcome.Success(ok), ctx()) + + val success = assertIs(result) + assertSame(ok, success.response) + } + + @Test + fun `the mapped failure is Retryable so the retry classifier can key off it`() { + // The error path emits an HttpException that is Retryable, which is exactly what + // RetryStep.isClassifiedRetryable matches on. + val pipeline = ResponsePipeline(responseSteps = listOf(ThrowOnHttpErrorStep)) + val result = pipeline.apply(ResponseOutcome.Success(response(Status.BAD_GATEWAY)), ctx()) + + val failure = assertIs(result) + val retryable = assertIs(failure.error) + assertTrue(retryable.isRetryable, "502 must classify as retryable through the interface") + } + + @Test + fun `error body stays readable on the Failure after the pipeline closes the response`() { + // The pipeline closes the in-hand response as soon as the step throws. The step buffers + // the error body first, so bodySnapshot()/string() still work on the thrown exception + // even though the original transport body is closed by close-before-propagate. + val payload = """{"error":"boom"}""" + val original = closeTrackingBody(payload) + val pipeline = ResponsePipeline(responseSteps = listOf(ThrowOnHttpErrorStep)) + + val result = + pipeline.apply( + ResponseOutcome.Success(response(Status.INTERNAL_SERVER_ERROR, body = original)), + ctx(), + ) + + assertTrue(original.closed, "the pipeline must close the original transport body") + val failure = assertIs(result) + val error = assertIs(failure.error) + + val snapshot = assertNotNull(error.bodySnapshot(), "buffered body must be snapshot-able") + assertEquals(payload, String(snapshot, Charsets.UTF_8)) + // Replayable: the buffered body can be read again as the primary stream. + val streamed = error.body!!.source().use { it.readUtf8() } + assertEquals(payload, streamed) + } + + // ---- helpers ------------------------------------------------------------------------ + + private fun ctx(): DispatchContext = DispatchContext.default() + + /** + * A response body that records when it is closed, so a test can prove the pipeline closed + * the original transport body while the buffered copy on the exception remains readable. + */ + private class CloseTrackingBody( + private val bytes: ByteArray, + ) : ResponseBody() { + var closed: Boolean = false + private set + + override fun mediaType() = null + + override fun contentLength(): Long = bytes.size.toLong() + + override fun source(): BufferedSource = Io.provider.source(bytes) + + override fun close() { + closed = true + } + } + + private fun closeTrackingBody(content: String): CloseTrackingBody = + CloseTrackingBody(content.toByteArray(Charsets.UTF_8)) + + private fun request(): Request = + Request.builder() + .url("https://api.example.com/resource") + .method(Method.GET) + .build() + + private fun response( + status: Status, + headers: Headers = Headers.Builder().build(), + body: ResponseBody? = null, + ): Response { + val builder = + Response.builder() + .request(request()) + .protocol(Protocol.HTTP_1_1) + .status(status) + .headers(headers) + if (body != null) builder.body(body) + return builder.build() + } +} diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetrySettingsTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetrySettingsTest.kt index 574167b8..458fbfdc 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetrySettingsTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/pipeline/step/retry/RetrySettingsTest.kt @@ -82,7 +82,7 @@ class RetrySettingsTest { @Test fun `default retryable statuses include 408 and the common retryable codes`() { - // The reconciled stance: 408 IS retryable, matching RetryUtils / HttpException.retryable + // The reconciled stance: 408 IS retryable, matching RetryUtils / HttpException.isRetryable // and the stage-based DefaultRetryStep. The two stacks must agree on the 408 question. val statuses = RetrySettings.DEFAULT_RETRYABLE_STATUSES assertEquals(setOf(408, 429, 500, 502, 503, 504), statuses.toSet())