Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>` (with dep-free `string()`/`empty()` handlers) and a lazy, parse-once `ParsedResponse<T>`. |
| `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<T>` (with dep-free `string()`/`empty()` handlers) and a lazy, parse-once `ParsedResponse<T>`. |
| `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`. |
Expand Down
8 changes: 4 additions & 4 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 |
Expand Down
38 changes: 27 additions & 11 deletions docs/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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`
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
8 changes: 5 additions & 3 deletions docs/pipelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/refs-comparison.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Loading
Loading