Skip to content

Commit ff44640

Browse files
committed
refactor!: unify the HTTP error path on a single exception base and a Retryable interface
The response-carrying exception hierarchy had two overlapping bases and the factory that produces the typed family was never wired into the error path. - Collapse `HttpResponseException` (an `IOException` carrying a `Response` + optional error value) into `HttpException`. `HttpException` becomes the single response-carrying base; it gains an optional `value` slot for a deserialized error payload so the generated layer can stamp a typed body later. Transport failures keep using the `NetworkException` (`IOException`) sibling, so existing `catch (IOException)` sites for transport errors are unaffected. - Introduce a `Retryable` interface (`val isRetryable: Boolean`) and implement it on both `HttpException` and `NetworkException`. The two types previously disagreed on the accessor name (`retryable` vs `isRetryable`); they now expose the same member through the interface. Retry classification in `RetryStep` keys off `Retryable` instead of matching concrete types, so a new retryable exception type participates automatically. The set of retryable statuses is unchanged (408/429 and 5xx except 501/505, still derived from `RetryUtils`). - Wire `HttpExceptionFactory.fromResponse` into the live error path via a new `ThrowOnHttpErrorStep` response step: on a 4xx/5xx response it throws the factory's typed exception, which `ResponsePipeline` turns into a failure outcome that flows through the recovery chain (including retry) like a transport failure. Previously the factory had no callers and 4xx/5xx responses never produced the typed subclass family. This is a binary-incompatible change to the public exception API: the `HttpResponseException` type is removed, the `retryable` getter on `HttpException` and `NetworkException` is renamed to `isRetryable`, and `HttpException` gains a `value` property and a corresponding constructor parameter. API snapshot regenerated; docs updated.
1 parent ea0cc81 commit ff44640

17 files changed

Lines changed: 415 additions & 324 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,8 @@ See [docs/pipelines.md](docs/pipelines.md) for the step-author walkthrough.
246246
|---|---|
247247
| `client` | `HttpClient`, `AsyncHttpClient` — the two transport SPIs (sync and async). |
248248
| `http.request` | `Request`, `RequestBody`, `FileRequestBody`, `LoggableRequestBody`, `Method`. |
249-
| `http.response` | `Response`, `ResponseBody`, `LoggableResponseBody`, `Status` (a value-carrying class with a total `fromCode`), `HttpResponseException`. |
250-
| `http.response.exception` | Typed `HttpException` hierarchy (`BadRequestException`, `RequestTimeoutException`, `TooManyRequestsException`, `ServiceUnavailableException`, …) with `retryable` derived from `RetryUtils.isRetryable`, plus `NetworkException` and `HttpExceptionFactory`. |
249+
| `http.response` | `Response`, `ResponseBody`, `LoggableResponseBody`, `Status` (a value-carrying class with a total `fromCode`). |
250+
| `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`. |
251251
| `http.common` | `Headers`, `HttpHeaderName` (interned), `MediaType`, `Protocol`, `HttpRange`, `ETag`, `RequestConditions`. |
252252
| `http.context` | `CallContext``DispatchContext``RequestContext``ExchangeContext` chain, `ContextStore`. |
253253
| `http.pipeline` | Sync (`HttpStep` / `HttpPipeline` / `HttpPipelineBuilder` / `PipelineNext` / `Stage`) and async (`AsyncHttpStep` / `AsyncHttpPipeline` / `AsyncHttpPipelineBuilder` / `AsyncPipelineNext`) pipeline machinery, plus `AsyncPipelineBridges`. |

docs/architecture.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -575,8 +575,8 @@ responds in a uniform way:
575575
subsequent blocking call also surfaces it.
576576
3. Throws `InterruptedIOException` (or the operation's natural failure exception with
577577
`InterruptedException` added as a suppressed cause).
578-
4. Classifies the interruption as **non-retryable**`HttpResponseException.isRetryable`
579-
returns `false` for an interrupt-driven failure.
578+
4. Classifies the interruption as **non-retryable**an interrupt-driven failure is not a
579+
`Retryable` condition, so the retry step never re-issues it.
580580

581581
Loops bounded by user input (retry attempts, paged iteration, server-sent-event
582582
consumption, drain loops in body logging) check `Thread.currentThread().isInterrupted` at
@@ -653,8 +653,8 @@ they should construct a fresh one.
653653
|----------------------|-------------------------------------------------------------------------------------------------|
654654
| `io` | Source, Sink, BufferedSource, BufferedSink, Buffer, IoProvider, Io, TeeSink (internal) |
655655
| `http.request` | Request, RequestBody, FileRequestBody, LoggableRequestBody, Method |
656-
| `http.response` | Response, ResponseBody, LoggableResponseBody, Status, HttpResponseException |
657-
| `http.response.exception` | HttpException, HttpExceptionFactory, RequestTimeoutException (and siblings), NetworkException |
656+
| `http.response` | Response, ResponseBody, LoggableResponseBody, Status |
657+
| `http.response.exception` | HttpException, HttpExceptionFactory, Retryable, RequestTimeoutException (and siblings), NetworkException |
658658
| `http.common` | Headers, MediaType, CommonMediaTypes, Protocol, ETag, HttpRange, RequestConditions |
659659
| `http.auth` | Credential, KeyCredential, BearerToken, ChallengeHandler, Basic/Digest/CompositeChallengeHandler, AuthChallengeParser |
660660
| `http.context` | CallContext, DispatchContext, RequestContext, ExchangeContext, ContextStore |

docs/http.md

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,9 @@ on whether a code is one the SDK recognizes.
286286
The SDK ships a typed exception hierarchy under `org.dexpace.sdk.core.http.response.exception`.
287287
The shape mirrors `gax`'s `ApiException` taxonomy translated to HTTP terms: one base class plus
288288
one concrete subclass per canonical status code. The base class derives its retryable flag
289-
from a single source of truth, so a downstream retry policy can read `exception.retryable`
290-
instead of maintaining a parallel predicate map.
289+
from a single source of truth and exposes it through the `Retryable` interface, so a downstream
290+
retry policy can read `(t as? Retryable)?.isRetryable` instead of maintaining a parallel
291+
predicate map or matching concrete exception types.
291292

292293
### HttpException Hierarchy
293294

@@ -302,16 +303,21 @@ abstract class HttpException(
302303
val body: ResponseBody?, // lazy — NOT eagerly buffered
303304
message: String? = null,
304305
cause: Throwable? = null,
305-
) : RuntimeException(message, cause) {
306-
val retryable: Boolean = RetryUtils.isRetryable(status.code)
306+
val value: Any? = null, // slot for a deserialized error payload (generated layer)
307+
) : RuntimeException(message, cause), Retryable {
308+
override val isRetryable: Boolean = RetryUtils.isRetryable(status.code)
307309
fun bodySnapshot(maxBytes: Int = DEFAULT_SNAPSHOT_BYTES): ByteArray?
308310
}
309311
```
310312

311-
`retryable` is a `val` **derived** at construction from `RetryUtils.isRetryable(status.code)`,
313+
`isRetryable` is a `val` **derived** at construction from `RetryUtils.isRetryable(status.code)`,
312314
not hardcoded per subclass and not a constructor parameter. This guarantees the baked flag
313315
can never disagree with the live retry policy: 408 / 429 and the 5xx range except 501 and 505
314-
are retryable, everything else is not.
316+
are retryable, everything else is not. It satisfies the `Retryable` interface (shared with
317+
`NetworkException`), which is what the retry step keys off.
318+
319+
`value` is a slot for a deserialized error payload, left `null` here — `sdk-core` does not
320+
parse bodies. The generated service layer populates it on a per-operation typed subclass.
315321

316322
`bodySnapshot()` returns a non-consuming preview of the body bytes — it reads from a fresh
317323
`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
355361
working, and it carries no status/headers/body because none arrived.
356362

357363
```kotlin
358-
open class NetworkException(message: String? = null, cause: Throwable? = null) : IOException(message, cause) {
359-
val retryable: Boolean = true // always retryable at the SDK level
364+
open class NetworkException(message: String? = null, cause: Throwable? = null) :
365+
IOException(message, cause), Retryable {
366+
override val isRetryable: Boolean = true // always retryable at the SDK level
360367
}
361368
```
362369

363-
The `retryable` flag is always `true`: nothing reached the server, so the SDK can safely
370+
The `isRetryable` flag is always `true`: nothing reached the server, so the SDK can safely
364371
attempt the request again. Whether the request itself is *safe* to retry (HTTP method
365372
idempotency, replayable body) is the retry policy's call, not this class's.
366373

367374
### HttpExceptionFactory
368375

369-
`HttpExceptionFactory.fromResponse(response)` maps a non-2xx `Response` to the right
370-
subclass:
376+
`HttpExceptionFactory.fromResponse(response)` maps a 4xx/5xx `Response` to the right subclass.
377+
It is the one seam that turns an error response into a typed exception, so the family is
378+
produced consistently rather than re-derived at each call site:
371379

372380
```kotlin
373381
val response = httpClient.execute(request)
@@ -379,6 +387,12 @@ if (!response.status.isSuccess) {
379387
The factory throws `IllegalArgumentException` if called with a status outside 400..599 —
380388
1xx/2xx/3xx outcomes are not exceptions and should not be funneled through this path.
381389

390+
In the recovery-aware pipeline this is wired through `ThrowOnHttpErrorStep`, a
391+
`ResponsePipelineStep` that calls `fromResponse` on a 4xx/5xx response and throws the result.
392+
`ResponsePipeline` converts that throw into a `ResponseOutcome.Failure`, which then flows
393+
through the recovery chain (e.g. `RetryStep`) exactly like a transport failure — and because
394+
the thrown `HttpException` is `Retryable`, retry classification keys off it uniformly.
395+
382396
---
383397

384398
## Common Types

docs/pipelines.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,11 +355,13 @@ public class RetryStep(
355355
)
356356
```
357357

358-
It retries only when the outcome is a `Failure` whose throwable is classified retryable:
358+
It retries only when the outcome is a `Failure` whose throwable is classified retryable. The
359+
classifier keys off the `Retryable` interface, not concrete exception types:
359360

360-
- An `HttpException` with `retryable == true` whose status code is in
361+
- An `HttpException` (which is `Retryable`) with `isRetryable == true` whose status code is in
361362
`RetrySettings.retryableStatuses`.
362-
- A `NetworkException` (a transport failure with no response on the wire — always retryable).
363+
- A `NetworkException` (also `Retryable`, always `true` — a transport failure with no response
364+
on the wire).
363365

364366
Idempotency is enforced independently of classification: a request is eligible only when its
365367
method is in `RetrySettings.retryableMethods` **or** its body is replayable. Non-idempotent

docs/refs-comparison.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ below records where each scheme's design was sourced from:
223223
**What shipped, and what's left:**
224224

225225
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.
226-
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`.
226+
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`.
227227
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.
228228
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.
229229

sdk-core/api/sdk-core.api

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,16 +1100,6 @@ public final class org/dexpace/sdk/core/http/request/RequestBody$Companion {
11001100
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;
11011101
}
11021102

1103-
public class org/dexpace/sdk/core/http/response/HttpResponseException : java/io/IOException {
1104-
public fun <init> (Ljava/lang/String;Lorg/dexpace/sdk/core/http/response/Response;)V
1105-
public fun <init> (Ljava/lang/String;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/Object;)V
1106-
public fun <init> (Ljava/lang/String;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/Object;Ljava/lang/Throwable;)V
1107-
public synthetic fun <init> (Ljava/lang/String;Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/Object;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
1108-
public final fun getResponse ()Lorg/dexpace/sdk/core/http/response/Response;
1109-
public final fun getValue ()Ljava/lang/Object;
1110-
public final fun isRetryable ()Z
1111-
}
1112-
11131103
public final class org/dexpace/sdk/core/http/response/LoggableResponseBody : org/dexpace/sdk/core/http/response/ResponseBody {
11141104
public fun <init> (Lorg/dexpace/sdk/core/http/response/ResponseBody;)V
11151105
public fun <init> (Lorg/dexpace/sdk/core/http/response/ResponseBody;Lorg/dexpace/sdk/core/io/IoProvider;)V
@@ -1319,20 +1309,22 @@ public class org/dexpace/sdk/core/http/response/exception/GoneException : org/de
13191309
public synthetic fun <init> (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
13201310
}
13211311

1322-
public abstract class org/dexpace/sdk/core/http/response/exception/HttpException : java/lang/RuntimeException {
1312+
public abstract class org/dexpace/sdk/core/http/response/exception/HttpException : java/lang/RuntimeException, org/dexpace/sdk/core/http/response/exception/Retryable {
13231313
public static final field Companion Lorg/dexpace/sdk/core/http/response/exception/HttpException$Companion;
13241314
public static final field DEFAULT_SNAPSHOT_BYTES I
13251315
public fun <init> (Lorg/dexpace/sdk/core/http/response/Status;Lorg/dexpace/sdk/core/http/common/Headers;Lorg/dexpace/sdk/core/http/response/ResponseBody;)V
13261316
public fun <init> (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
13271317
public fun <init> (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
1328-
public synthetic fun <init> (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
1318+
public fun <init> (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
1319+
public synthetic fun <init> (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
13291320
public final fun bodySnapshot ()[B
13301321
public final fun bodySnapshot (I)[B
13311322
public static synthetic fun bodySnapshot$default (Lorg/dexpace/sdk/core/http/response/exception/HttpException;IILjava/lang/Object;)[B
13321323
public final fun getBody ()Lorg/dexpace/sdk/core/http/response/ResponseBody;
13331324
public final fun getHeaders ()Lorg/dexpace/sdk/core/http/common/Headers;
1334-
public final fun getRetryable ()Z
13351325
public final fun getStatus ()Lorg/dexpace/sdk/core/http/response/Status;
1326+
public final fun getValue ()Ljava/lang/Object;
1327+
public fun isRetryable ()Z
13361328
}
13371329

13381330
public final class org/dexpace/sdk/core/http/response/exception/HttpException$Companion {
@@ -1357,12 +1349,12 @@ public class org/dexpace/sdk/core/http/response/exception/MethodNotAllowedExcept
13571349
public synthetic fun <init> (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
13581350
}
13591351

1360-
public class org/dexpace/sdk/core/http/response/exception/NetworkException : java/io/IOException {
1352+
public class org/dexpace/sdk/core/http/response/exception/NetworkException : java/io/IOException, org/dexpace/sdk/core/http/response/exception/Retryable {
13611353
public fun <init> ()V
13621354
public fun <init> (Ljava/lang/String;)V
13631355
public fun <init> (Ljava/lang/String;Ljava/lang/Throwable;)V
13641356
public synthetic fun <init> (Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
1365-
public final fun getRetryable ()Z
1357+
public fun isRetryable ()Z
13661358
}
13671359

13681360
public class org/dexpace/sdk/core/http/response/exception/NotFoundException : org/dexpace/sdk/core/http/response/exception/HttpException {
@@ -1386,6 +1378,10 @@ public class org/dexpace/sdk/core/http/response/exception/RequestTimeoutExceptio
13861378
public synthetic fun <init> (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
13871379
}
13881380

1381+
public abstract interface class org/dexpace/sdk/core/http/response/exception/Retryable {
1382+
public abstract fun isRetryable ()Z
1383+
}
1384+
13891385
public class org/dexpace/sdk/core/http/response/exception/ServerErrorException : org/dexpace/sdk/core/http/response/exception/HttpException {
13901386
public fun <init> (Lorg/dexpace/sdk/core/http/response/Response;)V
13911387
public fun <init> (Lorg/dexpace/sdk/core/http/response/Response;Ljava/lang/String;)V
@@ -2091,6 +2087,12 @@ public abstract interface class org/dexpace/sdk/core/pipeline/step/ResponseRecov
20912087
public abstract fun invoke (Lorg/dexpace/sdk/core/pipeline/ResponseOutcome;)Lorg/dexpace/sdk/core/pipeline/ResponseOutcome;
20922088
}
20932089

2090+
public final class org/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStep : org/dexpace/sdk/core/pipeline/step/ResponsePipelineStep {
2091+
public static final field INSTANCE Lorg/dexpace/sdk/core/pipeline/step/ThrowOnHttpErrorStep;
2092+
public synthetic fun execute (Ljava/lang/Object;Lorg/dexpace/sdk/core/http/context/DispatchContext;)Ljava/lang/Object;
2093+
public fun execute (Lorg/dexpace/sdk/core/http/response/Response;Lorg/dexpace/sdk/core/http/context/DispatchContext;)Lorg/dexpace/sdk/core/http/response/Response;
2094+
}
2095+
20942096
public final class org/dexpace/sdk/core/pipeline/step/retry/BackoffCalculator {
20952097
public static final field INSTANCE Lorg/dexpace/sdk/core/pipeline/step/retry/BackoffCalculator;
20962098
public static final fun computeDelay (ILorg/dexpace/sdk/core/pipeline/step/retry/RetrySettings;)Ljava/time/Duration;

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/response/HttpResponseException.kt

Lines changed: 0 additions & 60 deletions
This file was deleted.

0 commit comments

Comments
 (0)