From e8553c17be1225b58663ec416ef3ff46619621eb Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Tue, 25 Aug 2026 18:16:18 +0100 Subject: [PATCH 1/9] proposal: express error-response API in terms of error codes Proposal to re-express RequestFilterResultBuilder.errorResponse and RouterContext.respondWithError in terms of Errors codes rather than kafka-clients ApiException, removing the last exception types from the public API surface. Relates to kroxylicious#4756 and complements proposal 116. Assisted-by: Claude Opus 4.8 Signed-off-by: Keith Wall --- ...or-response-api-in-terms-of-error-codes.md | 206 ++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 proposals/000-error-response-api-in-terms-of-error-codes.md diff --git a/proposals/000-error-response-api-in-terms-of-error-codes.md b/proposals/000-error-response-api-in-terms-of-error-codes.md new file mode 100644 index 0000000..712ffd9 --- /dev/null +++ b/proposals/000-error-response-api-in-terms-of-error-codes.md @@ -0,0 +1,206 @@ + + +# 000 - Express the error-response API in terms of error codes, not client exceptions + +> [!NOTE] +> This proposal is based on [kroxylicious#4756](https://github.com/kroxylicious/kroxylicious/issues/4756) and complements +> [proposal 116 - Own the Kafka Protocol API Surface](./116-kafka-api-migration.md). + +The short-circuit error-response entry points on the public Filter and Router APIs take a Kafka +*client exception* (`org.apache.kafka.common.errors.ApiException`). This proposal re-expresses them +in terms of an error *code* (`Errors`) plus an optional message, removing the last places where the +`kafka-clients` exception hierarchy leaks into the Kroxylicious public API. + +## Current situation + +Two methods on the public API let a filter or router short-circuit a request with an error response: + +```java +// io.kroxylicious.proxy.filter.RequestFilterResultBuilder +CloseOrTerminalStage errorResponse( + RequestHeaderData header, ApiMessage requestMessage, ApiException apiException); + +// io.kroxylicious.proxy.router.RouterContext +RouterResult respondWithError( + RequestHeaderData header, ApiMessage requestMessage, ApiException apiException); +``` + +Both take `org.apache.kafka.common.errors.ApiException`. A caller who simply wants to reply "this is +`INVALID_REQUEST`" must first find, then instantiate, a matching exception subclass: + +```java +context.requestFilterResultBuilder() + .errorResponse(header, request, new InvalidRequestException("no topic id tag")) + .completed(); +``` + +Internally the runtime immediately reverses that: `KafkaProxyExceptionMapper` derives an error +**code** (`Errors.forException`) and a **message** (`Throwable.getMessage()`) from the exception, and +feeds them to Kafka's `AbstractRequest.getErrorResponse(Throwable)`. So the caller constructs an +exception purely so the runtime can map it back to the code the caller already had in mind. + +This is the last of the concerns identified in proposal 116: the `*Data` message classes, protocol +infrastructure, record classes and scattered `common.*` types are all addressed there, but the +`ApiException` hierarchy on these two entry points is a distinct API-shape problem — it is not a +namespace move, it is the wrong abstraction — and is called out separately in #4756. + +## Motivation + +- **Wrong abstraction.** The concept a caller wants to express is an error *code* (optionally with a + human-readable message). Requiring an exception forces the caller to pick a subclass from Kafka's + ~150-strong `ApiException` hierarchy and trust that `Errors.forException` maps it back to the code + they intended. The round-trip is lossy and non-obvious: two different exception subclasses can map + to the same code, and constructing the "wrong" exception silently yields a different code. +- **Keeps `kafka-clients` on the API surface.** Proposal 116 removes the generated `*Data` classes + and protocol infrastructure from the API. If these two methods keep taking `ApiException`, the + `kafka-clients` exception classes remain a compile-time dependency of every filter that + short-circuits, undermining the goal of a self-contained, Kroxylicious-owned API surface for 1.0. +- **Enables the owned-`Errors` payoff.** Once the API speaks in `Errors` codes rather than exception + instances, the `Errors` type itself can later be swapped for a Kroxylicious-owned enum (the follow + on to #4752/#4755). That swap is what ultimately allows the ~150 vendored `ApiException` subclasses + to be dropped from the owned surface entirely — the real payoff described in #4756. It is only + reachable once the *shape* of the API no longer demands an exception. + +## Proposal + +Introduce `Errors`-based overloads, deprecate the exception-based overloads, and widen the deprecated +overloads' parameter from `ApiException` to `java.lang.Throwable` so the `kafka-clients` reference +leaves the API *signature* immediately. + +### New overloads + +On both `RequestFilterResultBuilder.errorResponse` and `RouterContext.respondWithError`: + +```java +// code only — uses the Errors default message +errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors error); + +// code plus an explicit message +errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors error, @Nullable String message); +``` + +`Errors` is `org.apache.kafka.common.protocol.Errors` — the same enum the runtime already uses +internally, and consistent with the rest of the API surface on `main` today. When the owned `Errors` +enum lands, this single type is swapped for the owned one; call sites are otherwise unchanged. + +### Deprecate and widen the exception-based overloads + +```java +@Deprecated(since = "0.24.0", forRemoval = true) +errorResponse(RequestHeaderData header, ApiMessage requestMessage, Throwable apiException); +``` + +The parameter is widened from `ApiException` to `java.lang.Throwable` (a JDK type), so no +`kafka-clients` exception type appears in the public API signature. The former **compile-time** +constraint becomes a **runtime** one: the deprecated overload throws `IllegalArgumentException` if the +throwable is not an `org.apache.kafka.common.errors.ApiException` (referenced via FQN in the Javadoc +only, so the source imports no `kafka-clients` exception type). + +Existing callers keep compiling unchanged — every `ApiException` *is* a `Throwable`, so a call passing +an exception now binds to the deprecated `Throwable` overload. Overload resolution is unambiguous: +`Errors` is not a `Throwable`, so a call passing an `Errors` binds to the new overloads and a call +passing an exception binds to the deprecated one. + +### The runtime is unchanged + +Because both new paths ultimately construct `error.exception(message)` — a `kafka-clients` +`ApiException`; `Errors.exception(String)` returns the default-message instance when `message` is +`null` — they hand `KafkaProxyExceptionMapper` exactly what it consumes today. That the proxy still +materialises an `ApiException` internally to shape the response is an implementation detail: +`KafkaProxyExceptionMapper`, the `RouterResponseImpl.RespondWithError` record, `RouterDispatchHandler`, +and all the existing special-casing (`LIST_OFFSETS`, `END_TXN`, `LEAVE_GROUP`, the api-key-match +invariant, etc.) are preserved untouched. + +- The deprecated `Throwable` overload validates `throwable instanceof ApiException` (throwing + `IllegalArgumentException` otherwise), casts, and calls the existing mapper. +- The `Errors` overloads construct `error.exception(message)` and call the same mapper. + +### Migration + +- Existing call sites keep compiling; they simply bind to the (now deprecated) `Throwable` overload + and surface a deprecation warning, making the migration path visible to filter authors. +- Internal filters and the runtime are migrated to the `Errors` overloads as the reference example, + e.g. `errorResponse(header, request, Errors.SASL_AUTHENTICATION_FAILED)` and + `errorResponse(header, request, Errors.UNSUPPORTED_VERSION, reason)`. +- The `Errors` overloads should be preferred in documentation and examples. +- Removal of the deprecated `Throwable` overloads follows the project deprecation policy (no earlier + than the third minor release after the announcement, and at least three months later). + +## Affected/not affected projects + +**Affected:** + +- `kroxylicious-api` — the two entry points gain `Errors` overloads and their exception overloads are + deprecated and widened to `Throwable`. This is the public-API change this proposal exists to cover. +- `kroxylicious-runtime` — `RequestFilterResultBuilderImpl` and `RouterContextImpl` gain the new + overrides; the deprecated override adds the `instanceof` guard. `KafkaProxyExceptionMapper` and the + routing engine are **not** changed. +- `kroxylicious-filter-test-support` and the per-filter mock `MockFilterContext` implementations — + must implement the new abstract methods. +- `kroxylicious-filters` and test plugins — migrated to the new overloads as demonstration. + +**Not affected:** + +- The wire protocol and interoperability — the generated response bytes are identical; the change is + purely how the caller expresses the intended error. +- `kroxylicious-operator`, KMS, authorizer APIs, CRDs, and YAML configuration. + +## Compatibility + +- **Source compatibility:** preserved. Every existing caller passes an `ApiException`, which is a + `Throwable`, so existing filter source keeps compiling (against the deprecated overload). +- **Binary compatibility:** the exception-typed overloads are *removed* at the bytecode level (the + parameter type changes from `ApiException` to `Throwable`, which is a different method descriptor). + A pre-compiled plugin that was linked against `errorResponse(..., ApiException)` would fail at link + time (`NoSuchMethodError`) until recompiled. This is an accepted, deliberate break and is recorded + as an explicit `japicmp` exclusion. Recompilation against the new API is transparent. +- **Behavioural parity:** for an equivalent exception the deprecated overload produces the identical + response (same error code, same message) as before; unit tests assert this parity, and assert the + new `IllegalArgumentException` runtime contract for non-`ApiException` throwables. +- **Runtime contract change:** the deprecated overload now throws `IllegalArgumentException` at call + time if handed a non-`ApiException` throwable. Previously this was impossible to express (the + compiler rejected it), so no existing correct caller is affected. +- **Forward compatibility:** the `Errors` type in the new signatures is the single point that will be + swapped for the Kroxylicious-owned `Errors` enum in a later change, at which point the vendored + `ApiException` subclasses can be dropped from the owned surface. + +## Rejected alternatives + +### Replace the exception overloads outright (no deprecation window) + +Deleting `errorResponse(..., ApiException)` and shipping only the `Errors` overloads. This is a hard +source break for every filter that short-circuits, with no migration window. Rejected in favour of +the deprecate-and-widen path, which keeps existing source compiling and gives filter authors a +release cycle to migrate. + +### Keep `ApiException`, add `Errors` overloads alongside (no widening) + +Leave the exception overloads exactly as they are and just add the `Errors` overloads. This achieves +the ergonomic win but leaves `org.apache.kafka.common.errors.ApiException` on the public API +signature indefinitely, so `kafka-clients` never fully leaves the API surface — defeating the primary +motivation and the 1.0 goal from proposal 116. Widening to `Throwable` removes the type from the +signature now while preserving source compatibility. + +### Accept `String` code names or `int` codes instead of the `Errors` enum + +Expressing the error as a raw error-code `int`, or the `Errors` name as a `String`. Both discard type +safety and discoverability: an `int` or `String` invites invalid values and gives no IDE completion, +whereas the `Errors` enum is exhaustive, self-documenting, and already the runtime's own vocabulary. +Rejected. + +### Introduce a Kroxylicious-owned error-code type now + +Define a new Kroxylicious error-code abstraction as part of this change rather than reusing +`org.apache.kafka.common.protocol.Errors`. This couples this focused API-shape change to the larger +owned-protocol effort (#4752/#4755, proposal 116) and would land an owned type on `main` ahead of +that work. Reusing the existing `Errors` enum keeps this change small and consistent with the current +surface; the swap to an owned enum is a clean, mechanical follow-up once the owned protocol lands. From 1e3595911104261267d79c2a7ca6a10168d2e31b Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Tue, 25 Aug 2026 18:17:10 +0100 Subject: [PATCH 2/9] proposal: rename to PR number 131 and update heading Assisted-by: Claude Opus 4.8 Signed-off-by: Keith Wall --- ...1-error-response-api-in-terms-of-error-codes.md} | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) rename proposals/{000-error-response-api-in-terms-of-error-codes.md => 131-error-response-api-in-terms-of-error-codes.md} (95%) diff --git a/proposals/000-error-response-api-in-terms-of-error-codes.md b/proposals/131-error-response-api-in-terms-of-error-codes.md similarity index 95% rename from proposals/000-error-response-api-in-terms-of-error-codes.md rename to proposals/131-error-response-api-in-terms-of-error-codes.md index 712ffd9..f43a4cf 100644 --- a/proposals/000-error-response-api-in-terms-of-error-codes.md +++ b/proposals/131-error-response-api-in-terms-of-error-codes.md @@ -1,15 +1,4 @@ - - -# 000 - Express the error-response API in terms of error codes, not client exceptions +# 131 - Express the error-response API in terms of error codes, not client exceptions > [!NOTE] > This proposal is based on [kroxylicious#4756](https://github.com/kroxylicious/kroxylicious/issues/4756) and complements From 20467d11eb685d737fc0cf4cabd6f282256a1079 Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Tue, 25 Aug 2026 18:26:37 +0100 Subject: [PATCH 3/9] proposal: focus on public API inconsistency, trim internal detail The concern is that ApiException on the public API is inconsistent with the error-code vocabulary used elsewhere in the API; how the exception is used internally is not the point. Assisted-by: Claude Opus 4.8 Signed-off-by: Keith Wall --- ...or-response-api-in-terms-of-error-codes.md | 44 ++++++++----------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/proposals/131-error-response-api-in-terms-of-error-codes.md b/proposals/131-error-response-api-in-terms-of-error-codes.md index f43a4cf..7f2398d 100644 --- a/proposals/131-error-response-api-in-terms-of-error-codes.md +++ b/proposals/131-error-response-api-in-terms-of-error-codes.md @@ -32,23 +32,23 @@ context.requestFilterResultBuilder() .completed(); ``` -Internally the runtime immediately reverses that: `KafkaProxyExceptionMapper` derives an error -**code** (`Errors.forException`) and a **message** (`Throwable.getMessage()`) from the exception, and -feeds them to Kafka's `AbstractRequest.getErrorResponse(Throwable)`. So the caller constructs an -exception purely so the runtime can map it back to the code the caller already had in mind. - -This is the last of the concerns identified in proposal 116: the `*Data` message classes, protocol -infrastructure, record classes and scattered `common.*` types are all addressed there, but the -`ApiException` hierarchy on these two entry points is a distinct API-shape problem — it is not a +This is inconsistent with the rest of the API, which already speaks in terms of error **codes**: +everywhere else an error is conveyed — including the error codes a filter reads off a response — the +vocabulary is the `Errors` code, not a client exception. These two entry points are the odd ones out, +requiring the caller to reach for the `kafka-clients` exception hierarchy to say something the API +otherwise expresses as a code. + +They are also the last of the concerns identified in proposal 116: the `*Data` message classes, +protocol infrastructure, record classes and scattered `common.*` types are all addressed there, but +the `ApiException` hierarchy on these two entry points is a distinct API-shape problem — it is not a namespace move, it is the wrong abstraction — and is called out separately in #4756. ## Motivation -- **Wrong abstraction.** The concept a caller wants to express is an error *code* (optionally with a - human-readable message). Requiring an exception forces the caller to pick a subclass from Kafka's - ~150-strong `ApiException` hierarchy and trust that `Errors.forException` maps it back to the code - they intended. The round-trip is lossy and non-obvious: two different exception subclasses can map - to the same code, and constructing the "wrong" exception silently yields a different code. +- **Inconsistent with the rest of the API.** Errors are conveyed as `Errors` codes everywhere else + in the API. These two methods are the exception — literally — forcing the caller to pick a subclass + from Kafka's ~150-strong `ApiException` hierarchy to express what the API elsewhere expresses as a + code. That inconsistency is a papercut for filter authors and an obstacle to a coherent 1.0 API. - **Keeps `kafka-clients` on the API surface.** Proposal 116 removes the generated `*Data` classes and protocol infrastructure from the API. If these two methods keep taking `ApiException`, the `kafka-clients` exception classes remain a compile-time dependency of every filter that @@ -99,19 +99,13 @@ an exception now binds to the deprecated `Throwable` overload. Overload resoluti `Errors` is not a `Throwable`, so a call passing an `Errors` binds to the new overloads and a call passing an exception binds to the deprecated one. -### The runtime is unchanged +### No runtime churn -Because both new paths ultimately construct `error.exception(message)` — a `kafka-clients` -`ApiException`; `Errors.exception(String)` returns the default-message instance when `message` is -`null` — they hand `KafkaProxyExceptionMapper` exactly what it consumes today. That the proxy still -materialises an `ApiException` internally to shape the response is an implementation detail: -`KafkaProxyExceptionMapper`, the `RouterResponseImpl.RespondWithError` record, `RouterDispatchHandler`, -and all the existing special-casing (`LIST_OFFSETS`, `END_TXN`, `LEAVE_GROUP`, the api-key-match -invariant, etc.) are preserved untouched. - -- The deprecated `Throwable` overload validates `throwable instanceof ApiException` (throwing - `IllegalArgumentException` otherwise), casts, and calls the existing mapper. -- The `Errors` overloads construct `error.exception(message)` and call the same mapper. +How the runtime turns the request into an error response is an implementation detail and is +unchanged: the new `Errors` overloads feed the existing response-shaping engine exactly what it +consumes today, so no downstream signatures or behaviour change. The deprecated `Throwable` overload +performs the `instanceof ApiException` check that the compiler used to enforce, and otherwise behaves +as before. ### Migration From 7980e1c99553c42cefc8e3be4a9fc25e06edd48d Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Wed, 26 Aug 2026 12:08:42 +0100 Subject: [PATCH 4/9] proposal: clean break, move deprecate-and-widen to rejected alternatives Following review, drop the transitional Throwable overload: remove the ApiException overloads outright rather than deprecating and widening them. Filter authors already edit source in 0.24.0 to migrate off Kafka's *Data classes (proposal 116), so the source break rides along with that change. Also address review feedback: reframe the current-situation around API inconsistency rather than difficulty, add a Non-goals section covering the runtime's continued kafka-clients dependency and the Filter error contract, and note the same-release timing minimises inconvenience. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Keith Wall --- ...or-response-api-in-terms-of-error-codes.md | 159 +++++++++++------- 1 file changed, 96 insertions(+), 63 deletions(-) diff --git a/proposals/131-error-response-api-in-terms-of-error-codes.md b/proposals/131-error-response-api-in-terms-of-error-codes.md index 7f2398d..6fcdea2 100644 --- a/proposals/131-error-response-api-in-terms-of-error-codes.md +++ b/proposals/131-error-response-api-in-terms-of-error-codes.md @@ -24,7 +24,7 @@ RouterResult respondWithError( ``` Both take `org.apache.kafka.common.errors.ApiException`. A caller who simply wants to reply "this is -`INVALID_REQUEST`" must first find, then instantiate, a matching exception subclass: +`INVALID_REQUEST`" must round-trip through the exception hierarchy: ```java context.requestFilterResultBuilder() @@ -32,11 +32,13 @@ context.requestFilterResultBuilder() .completed(); ``` -This is inconsistent with the rest of the API, which already speaks in terms of error **codes**: -everywhere else an error is conveyed — including the error codes a filter reads off a response — the -vocabulary is the `Errors` code, not a client exception. These two entry points are the odd ones out, -requiring the caller to reach for the `kafka-clients` exception hierarchy to say something the API -otherwise expresses as a code. +This is not especially *hard* — `Errors` even offers a shortcut, +`Errors.INVALID_REQUEST.exception("no topic id tag")`, so the caller need not pick the subclass by +hand — but it is *inconsistent*. Everywhere else the API conveys an error as an `Errors` code, +including the error codes a filter reads off a response. These two entry points are the odd ones out, +forcing the caller to materialise a `kafka-clients` exception instance to say something the API +otherwise expresses as a code, only for the runtime to unwrap that exception straight back to the code +it started from. They are also the last of the concerns identified in proposal 116: the `*Data` message classes, protocol infrastructure, record classes and scattered `common.*` types are all addressed there, but @@ -46,9 +48,9 @@ namespace move, it is the wrong abstraction — and is called out separately in ## Motivation - **Inconsistent with the rest of the API.** Errors are conveyed as `Errors` codes everywhere else - in the API. These two methods are the exception — literally — forcing the caller to pick a subclass - from Kafka's ~150-strong `ApiException` hierarchy to express what the API elsewhere expresses as a - code. That inconsistency is a papercut for filter authors and an obstacle to a coherent 1.0 API. + in the API. These two methods are the exception — literally — forcing the caller to route through + Kafka's `ApiException` hierarchy to express what the API elsewhere expresses as a code. That + inconsistency is a papercut for filter authors and an obstacle to a coherent 1.0 API. - **Keeps `kafka-clients` on the API surface.** Proposal 116 removes the generated `*Data` classes and protocol infrastructure from the API. If these two methods keep taking `ApiException`, the `kafka-clients` exception classes remain a compile-time dependency of every filter that @@ -61,9 +63,15 @@ namespace move, it is the wrong abstraction — and is called out separately in ## Proposal -Introduce `Errors`-based overloads, deprecate the exception-based overloads, and widen the deprecated -overloads' parameter from `ApiException` to `java.lang.Throwable` so the `kafka-clients` reference -leaves the API *signature* immediately. +Add `Errors`-based overloads and **remove** the exception-based overloads outright. There is no +deprecation window and no transitional `Throwable` signature: the `org.apache.kafka.common.errors.ApiException` +reference leaves the public API in one step. + +The clean break is chosen deliberately. Filter authors already have to make source changes for +proposal 116 — moving off Kafka's `*Data` classes onto Kroxylicious's own, in the same 0.24.0 release +— so this edit rides along with a migration they are already performing; it costs them no *additional* +migration event, and the alternative deprecate-and-widen machinery buys little in return (see +[Rejected alternatives](#deprecate-and-widen-the-exception-overloads-to-throwable)). ### New overloads @@ -81,54 +89,64 @@ errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors error, internally, and consistent with the rest of the API surface on `main` today. When the owned `Errors` enum lands, this single type is swapped for the owned one; call sites are otherwise unchanged. -### Deprecate and widen the exception-based overloads +### Removed overloads ```java -@Deprecated(since = "0.24.0", forRemoval = true) -errorResponse(RequestHeaderData header, ApiMessage requestMessage, Throwable apiException); +// removed — no deprecated replacement +errorResponse(RequestHeaderData header, ApiMessage requestMessage, ApiException apiException); +respondWithError(RequestHeaderData header, ApiMessage requestMessage, ApiException apiException); ``` -The parameter is widened from `ApiException` to `java.lang.Throwable` (a JDK type), so no -`kafka-clients` exception type appears in the public API signature. The former **compile-time** -constraint becomes a **runtime** one: the deprecated overload throws `IllegalArgumentException` if the -throwable is not an `org.apache.kafka.common.errors.ApiException` (referenced via FQN in the Javadoc -only, so the source imports no `kafka-clients` exception type). - -Existing callers keep compiling unchanged — every `ApiException` *is* a `Throwable`, so a call passing -an exception now binds to the deprecated `Throwable` overload. Overload resolution is unambiguous: -`Errors` is not a `Throwable`, so a call passing an `Errors` binds to the new overloads and a call -passing an exception binds to the deprecated one. +With these gone, no `kafka-clients` exception type appears anywhere in the public API signature — no +deprecated overload, no `Throwable` widening, no runtime type-check to maintain. ### No runtime churn How the runtime turns the request into an error response is an implementation detail and is unchanged: the new `Errors` overloads feed the existing response-shaping engine exactly what it -consumes today, so no downstream signatures or behaviour change. The deprecated `Throwable` overload -performs the `instanceof ApiException` check that the compiler used to enforce, and otherwise behaves -as before. - -### Migration - -- Existing call sites keep compiling; they simply bind to the (now deprecated) `Throwable` overload - and surface a deprecation warning, making the migration path visible to filter authors. -- Internal filters and the runtime are migrated to the `Errors` overloads as the reference example, - e.g. `errorResponse(header, request, Errors.SASL_AUTHENTICATION_FAILED)` and +consumes today, so no downstream signatures or behaviour change. Internally the code the caller now +passes is the code the engine already worked with; the exception was only ever an envelope for it. + +## Non-goals + +- **Removing `kafka-clients` from the runtime.** The `kroxylicious-runtime` continues to depend on + `kafka-clients`; that dependency's eventual removal is part of the wider own-the-protocol work + (proposal 116, #4752/#4755), not this proposal. This change adjusts the *public API* shape only and + leaves the runtime free to keep using `Errors`/`ApiException` internally. +- **Redefining how thrown exceptions are mapped to responses.** Today a filter that *throws* an + `ApiException` from a filter method has it mapped back to an error response by + `KafkaProxyExceptionMapper`; that behaviour is untouched here. It is worth being explicit about the + contract, though: the supported way to short-circuit with a protocol error is the `Errors`-based + `errorResponse`/`respondWithError`. Relying on throwing a `kafka-clients` exception and having the + runtime recover the code is not a guarantee this proposal strengthens — and it cannot survive + `kafka-clients` eventually leaving the runtime (a reflective code-recovery shim could bridge that + transition, but that is future work under 116). Firming up the `Filter` error contract in full is + out of scope here and tracked with the own-the-protocol effort. + +## Migration + +- Existing call sites must be updated — mechanically — from an exception to the equivalent code, e.g. + `errorResponse(header, request, Errors.GROUP_AUTHORIZATION_FAILED.exception())` becomes + `errorResponse(header, request, Errors.GROUP_AUTHORIZATION_FAILED)`, and + `Errors.UNSUPPORTED_VERSION.exception(reason)` becomes `errorResponse(header, request, Errors.UNSUPPORTED_VERSION, reason)`. -- The `Errors` overloads should be preferred in documentation and examples. -- Removal of the deprecated `Throwable` overloads follows the project deprecation policy (no earlier - than the third minor release after the announcement, and at least three months later). +- Because 0.24.0 already forces filter authors to make source changes (proposal 116's move off the + `*Data` classes), this edit rides along with changes the author is making regardless; there is no + separate migration release to track. +- Internal filters, the runtime and the test plugins are migrated to the `Errors` overloads as the + reference examples, and the `Errors` overloads are the only form shown in documentation. ## Affected/not affected projects **Affected:** - `kroxylicious-api` — the two entry points gain `Errors` overloads and their exception overloads are - deprecated and widened to `Throwable`. This is the public-API change this proposal exists to cover. + removed. This is the public-API change this proposal exists to cover. - `kroxylicious-runtime` — `RequestFilterResultBuilderImpl` and `RouterContextImpl` gain the new - overrides; the deprecated override adds the `instanceof` guard. `KafkaProxyExceptionMapper` and the - routing engine are **not** changed. + overrides and drop the removed ones. `KafkaProxyExceptionMapper` and the routing engine are **not** + changed. - `kroxylicious-filter-test-support` and the per-filter mock `MockFilterContext` implementations — - must implement the new abstract methods. + must implement the new abstract methods and drop the removed ones. - `kroxylicious-filters` and test plugins — migrated to the new overloads as demonstration. **Not affected:** @@ -139,39 +157,54 @@ as before. ## Compatibility -- **Source compatibility:** preserved. Every existing caller passes an `ApiException`, which is a - `Throwable`, so existing filter source keeps compiling (against the deprecated overload). -- **Binary compatibility:** the exception-typed overloads are *removed* at the bytecode level (the - parameter type changes from `ApiException` to `Throwable`, which is a different method descriptor). - A pre-compiled plugin that was linked against `errorResponse(..., ApiException)` would fail at link - time (`NoSuchMethodError`) until recompiled. This is an accepted, deliberate break and is recorded - as an explicit `japicmp` exclusion. Recompilation against the new API is transparent. -- **Behavioural parity:** for an equivalent exception the deprecated overload produces the identical - response (same error code, same message) as before; unit tests assert this parity, and assert the - new `IllegalArgumentException` runtime contract for non-`ApiException` throwables. -- **Runtime contract change:** the deprecated overload now throws `IllegalArgumentException` at call - time if handed a non-`ApiException` throwable. Previously this was impossible to express (the - compiler rejected it), so no existing correct caller is affected. +- **Source compatibility:** deliberately broken. Every existing caller passes an `ApiException`, + which no longer resolves to any overload, so filter source that short-circuits must be edited (the + mechanical change in [Migration](#migration)). This break is scheduled for the same 0.24.0 release + as proposal 116, where filter authors are already editing source to move off Kafka's `*Data` + classes onto Kroxylicious's own — so the change is folded into a migration they must perform anyway, + minimising the inconvenience. +- **Binary compatibility:** the exception-typed overloads are removed at the bytecode level. A + pre-compiled plugin linked against `errorResponse(..., ApiException)` would fail at link time + (`NoSuchMethodError`) until recompiled. This is an accepted, deliberate break and is recorded as an + explicit `japicmp` exclusion. Recompilation against the new API is transparent. (A future runtime + enhancement could catch `LinkageError`/`NoSuchMethodError` in the safe invoker and emit a targeted + "compiled against a different API version" diagnostic; that is out of scope here and belongs with + the API-versioning work.) +- **Behavioural parity:** for an equivalent input the new `Errors` overload produces the identical + response (same error code, same message) the exception overload produced before; unit tests assert + this parity. - **Forward compatibility:** the `Errors` type in the new signatures is the single point that will be swapped for the Kroxylicious-owned `Errors` enum in a later change, at which point the vendored `ApiException` subclasses can be dropped from the owned surface. ## Rejected alternatives -### Replace the exception overloads outright (no deprecation window) +### Deprecate and widen the exception overloads to `Throwable` + +Rather than removing the exception overloads, keep them but deprecate them and widen their parameter +from `ApiException` to `java.lang.Throwable`, adding the `Errors` overloads alongside. The +`kafka-clients` type would leave the *signature* immediately (`Throwable` is a JDK type), the former +compile-time `ApiException` constraint would become a runtime check throwing `IllegalArgumentException`, +and existing source would keep compiling through a deprecation window. -Deleting `errorResponse(..., ApiException)` and shipping only the `Errors` overloads. This is a hard -source break for every filter that short-circuits, with no migration window. Rejected in favour of -the deprecate-and-widen path, which keeps existing source compiling and gives filter authors a -release cycle to migrate. +Rejected. The machinery buys source compatibility that is largely moot. Filter authors are already +making source changes in 0.24.0 to migrate off Kafka's `*Data` classes onto Kroxylicious's own +(proposal 116), so preserving compilation of *unchanged* source protects a case that does not really +occur: a filter that short-circuits will be edited in this release regardless. The runtime check is +also more awkward than it first appears — since the direction of travel is to remove `kafka-clients` +from the runtime entirely, the guard could not be a plain `instanceof ApiException`; it would +ultimately have to be a *reflective* class-name check, keeping a `kafka-clients` coupling alive by the +back door. Add the deprecation window, the `japicmp` bookkeeping, and the risk of a caller passing a +non-`ApiException` `Throwable` and only finding out at runtime, and reviewers rightly questioned +whether the transitional signature was worth its complexity. The clean break carries the same one-time +source edit while leaving nothing behind to remove later. -### Keep `ApiException`, add `Errors` overloads alongside (no widening) +### Keep `ApiException`, add `Errors` overloads alongside (no removal) Leave the exception overloads exactly as they are and just add the `Errors` overloads. This achieves the ergonomic win but leaves `org.apache.kafka.common.errors.ApiException` on the public API signature indefinitely, so `kafka-clients` never fully leaves the API surface — defeating the primary -motivation and the 1.0 goal from proposal 116. Widening to `Throwable` removes the type from the -signature now while preserving source compatibility. +motivation and the 1.0 goal from proposal 116. ### Accept `String` code names or `int` codes instead of the `Errors` enum From 010f0d1d6f2b43749ccd746f87be2a1c610d2208 Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Wed, 26 Aug 2026 12:45:54 +0100 Subject: [PATCH 5/9] proposal: clarity fixes and non-goal on Errors vendoring Proofread the current-situation and non-goals sections: fix a comma splice and Errors/short wording, tidy the proposal-116 vendoring paragraph, and correct the mapper description to use *RequestData/*Request terminology. Add an explicit non-goal that vendoring an owned Errors class is delivered separately; this proposal's API uses Kafka's Errors enum, to be swapped for the Kroxylicious-owned one later. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Keith Wall --- ...or-response-api-in-terms-of-error-codes.md | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/proposals/131-error-response-api-in-terms-of-error-codes.md b/proposals/131-error-response-api-in-terms-of-error-codes.md index 6fcdea2..fe4b6b4 100644 --- a/proposals/131-error-response-api-in-terms-of-error-codes.md +++ b/proposals/131-error-response-api-in-terms-of-error-codes.md @@ -34,20 +34,20 @@ context.requestFilterResultBuilder() This is not especially *hard* — `Errors` even offers a shortcut, `Errors.INVALID_REQUEST.exception("no topic id tag")`, so the caller need not pick the subclass by -hand — but it is *inconsistent*. Everywhere else the API conveys an error as an `Errors` code, -including the error codes a filter reads off a response. These two entry points are the odd ones out, -forcing the caller to materialise a `kafka-clients` exception instance to say something the API -otherwise expresses as a code, only for the runtime to unwrap that exception straight back to the code -it started from. +hand — but it is *inconsistent*. Everywhere else the API conveys an error as an error code (a `short` +or the `Errors` enum). These two entry points are the odd ones out, forcing the caller to materialise +a `kafka-clients` exception instance to say something the API otherwise expresses as a code and +optional message. -They are also the last of the concerns identified in proposal 116: the `*Data` message classes, -protocol infrastructure, record classes and scattered `common.*` types are all addressed there, but -the `ApiException` hierarchy on these two entry points is a distinct API-shape problem — it is not a -namespace move, it is the wrong abstraction — and is called out separately in #4756. +[Proposal 116](https://github.com/kroxylicious/design/blob/main/proposals/116-kafka-api-migration.md) had agreed that +the `ApiException` class would be vendored into the `kroxylicious-api` source tree, but did not fully +appreciate the gravity of that decision: +* it would also entail vendoring more than one hundred `ApiException` subclasses; and +* Kroxylicious would inherit Kafka's large exception model into its public API. ## Motivation -- **Inconsistent with the rest of the API.** Errors are conveyed as `Errors` codes everywhere else +- **Inconsistent with the rest of the API.** Errors are conveyed as `Errors` codes (or shorts) everywhere else in the API. These two methods are the exception — literally — forcing the caller to route through Kafka's `ApiException` hierarchy to express what the API elsewhere expresses as a code. That inconsistency is a papercut for filter authors and an obstacle to a coherent 1.0 API. @@ -73,7 +73,7 @@ proposal 116 — moving off Kafka's `*Data` classes onto Kroxylicious's own, in migration event, and the alternative deprecate-and-widen machinery buys little in return (see [Rejected alternatives](#deprecate-and-widen-the-exception-overloads-to-throwable)). -### New overloads +### New API On both `RequestFilterResultBuilder.errorResponse` and `RouterContext.respondWithError`: @@ -89,7 +89,7 @@ errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors error, internally, and consistent with the rest of the API surface on `main` today. When the owned `Errors` enum lands, this single type is swapped for the owned one; call sites are otherwise unchanged. -### Removed overloads +### Removed API ```java // removed — no deprecated replacement @@ -110,18 +110,21 @@ passes is the code the engine already worked with; the exception was only ever a ## Non-goals - **Removing `kafka-clients` from the runtime.** The `kroxylicious-runtime` continues to depend on - `kafka-clients`; that dependency's eventual removal is part of the wider own-the-protocol work - (proposal 116, #4752/#4755), not this proposal. This change adjusts the *public API* shape only and - leaves the runtime free to keep using `Errors`/`ApiException` internally. -- **Redefining how thrown exceptions are mapped to responses.** Today a filter that *throws* an - `ApiException` from a filter method has it mapped back to an error response by - `KafkaProxyExceptionMapper`; that behaviour is untouched here. It is worth being explicit about the - contract, though: the supported way to short-circuit with a protocol error is the `Errors`-based - `errorResponse`/`respondWithError`. Relying on throwing a `kafka-clients` exception and having the - runtime recover the code is not a guarantee this proposal strengthens — and it cannot survive - `kafka-clients` eventually leaving the runtime (a reflective code-recovery shim could bridge that - transition, but that is future work under 116). Firming up the `Filter` error contract in full is - out of scope here and tracked with the own-the-protocol effort. + `kafka-clients` for now; that dependency's eventual removal is part of the wider own-the-protocol work + (proposal 116, #4752/#4755), not this proposal. This change adjusts the *public API* shape only. +- **Vendoring the `Errors` class.** The new API delivered by this proposal is expressed in terms of + Kafka's `org.apache.kafka.common.protocol.Errors` — the enum the runtime and the rest of the API + surface already use today. Vendoring an owned `Errors` class into `kroxylicious-api` is delivered + separately (the follow-on to #4752/#4755, proposal 116); at that point this single type is swapped + for the Kroxylicious-owned one, and call sites are otherwise unchanged. +- **Redefining how error responses are created.** Today the `KafkaProxyExceptionMapper` uses the + `ApiException` to generate an error response. The public API is expressed in terms of the + `*RequestData`/`*ResponseData` message classes, but internally the mapper reconstructs the + corresponding `*Request` object from the `*RequestData` and calls + `AbstractRequest#getErrorResponse(java.lang.Throwable)` on it to produce a correctly shaped error + response. For the scope of this proposal, this behaviour is unchanged. Separate work (being + delivered by [kroxylicious#4748](https://github.com/kroxylicious/kroxylicious/issues/4748)) will + eliminate the dependency on the `*Request` object. ## Migration From f8be63d60fb21850bbdf917fc41384de45bfe482 Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Wed, 26 Aug 2026 13:13:18 +0100 Subject: [PATCH 6/9] proposal: proofreading pass for consistency and clarity Resolve internal inconsistencies found in review: - drop the "No runtime churn" section (contradicted the mapper non-goal) - clarify why removing ApiException from the signatures does not yet drop the ~150 subclasses (Kafka's Errors enum still references them) - describe the future owned type consistently as the Kroxylicious-owned Errors enum (a vendored copy), not a new abstraction - align the subclass count to ~150 and soften the proposal-116 framing - note respondWithError gains the same overloads - remove the redundant forward-compatibility bullet Co-Authored-By: Claude Opus 4.8 Signed-off-by: Keith Wall --- ...or-response-api-in-terms-of-error-codes.md | 46 +++++++++---------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/proposals/131-error-response-api-in-terms-of-error-codes.md b/proposals/131-error-response-api-in-terms-of-error-codes.md index fe4b6b4..6dea992 100644 --- a/proposals/131-error-response-api-in-terms-of-error-codes.md +++ b/proposals/131-error-response-api-in-terms-of-error-codes.md @@ -39,10 +39,10 @@ or the `Errors` enum). These two entry points are the odd ones out, forcing the a `kafka-clients` exception instance to say something the API otherwise expresses as a code and optional message. -[Proposal 116](https://github.com/kroxylicious/design/blob/main/proposals/116-kafka-api-migration.md) had agreed that -the `ApiException` class would be vendored into the `kroxylicious-api` source tree, but did not fully -appreciate the gravity of that decision: -* it would also entail vendoring more than one hundred `ApiException` subclasses; and +[Proposal 116](https://github.com/kroxylicious/design/blob/main/proposals/116-kafka-api-migration.md) had agreed to +vendor the `ApiException` class into the `kroxylicious-api` source tree. The full weight of that +decision only became apparent later: +* it would also entail vendoring the ~150 `ApiException` subclasses; and * Kroxylicious would inherit Kafka's large exception model into its public API. ## Motivation @@ -55,11 +55,14 @@ appreciate the gravity of that decision: and protocol infrastructure from the API. If these two methods keep taking `ApiException`, the `kafka-clients` exception classes remain a compile-time dependency of every filter that short-circuits, undermining the goal of a self-contained, Kroxylicious-owned API surface for 1.0. -- **Enables the owned-`Errors` payoff.** Once the API speaks in `Errors` codes rather than exception - instances, the `Errors` type itself can later be swapped for a Kroxylicious-owned enum (the follow - on to #4752/#4755). That swap is what ultimately allows the ~150 vendored `ApiException` subclasses - to be dropped from the owned surface entirely — the real payoff described in #4756. It is only - reachable once the *shape* of the API no longer demands an exception. +- **Enables the owned-`Errors` payoff.** Removing `ApiException` from these two signatures takes the + exception hierarchy off the public API *shape*, but it does not by itself drop the ~150 subclasses + from the owned surface: the API now speaks in Kafka's `Errors` enum, and that enum still references + the exception subclasses (each constant can instantiate its exception via `Errors.exception()`), so + they are pulled in transitively. Dropping them entirely needs the further step of swapping `Errors` + for a Kroxylicious-owned enum that does not reference the Kafka exceptions (the follow-on to + #4752/#4755) — the real payoff described in #4756. That swap only becomes *reachable* once the shape + of the API no longer demands an exception, which is what this proposal delivers. ## Proposal @@ -85,6 +88,8 @@ errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors error) errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors error, @Nullable String message); ``` +`RouterContext.respondWithError` gains the same two overloads. + `Errors` is `org.apache.kafka.common.protocol.Errors` — the same enum the runtime already uses internally, and consistent with the rest of the API surface on `main` today. When the owned `Errors` enum lands, this single type is swapped for the owned one; call sites are otherwise unchanged. @@ -100,13 +105,6 @@ respondWithError(RequestHeaderData header, ApiMessage requestMessage, ApiExcepti With these gone, no `kafka-clients` exception type appears anywhere in the public API signature — no deprecated overload, no `Throwable` widening, no runtime type-check to maintain. -### No runtime churn - -How the runtime turns the request into an error response is an implementation detail and is -unchanged: the new `Errors` overloads feed the existing response-shaping engine exactly what it -consumes today, so no downstream signatures or behaviour change. Internally the code the caller now -passes is the code the engine already worked with; the exception was only ever an envelope for it. - ## Non-goals - **Removing `kafka-clients` from the runtime.** The `kroxylicious-runtime` continues to depend on @@ -176,9 +174,6 @@ passes is the code the engine already worked with; the exception was only ever a - **Behavioural parity:** for an equivalent input the new `Errors` overload produces the identical response (same error code, same message) the exception overload produced before; unit tests assert this parity. -- **Forward compatibility:** the `Errors` type in the new signatures is the single point that will be - swapped for the Kroxylicious-owned `Errors` enum in a later change, at which point the vendored - `ApiException` subclasses can be dropped from the owned surface. ## Rejected alternatives @@ -216,10 +211,11 @@ safety and discoverability: an `int` or `String` invites invalid values and give whereas the `Errors` enum is exhaustive, self-documenting, and already the runtime's own vocabulary. Rejected. -### Introduce a Kroxylicious-owned error-code type now +### Introduce the Kroxylicious-owned `Errors` enum now -Define a new Kroxylicious error-code abstraction as part of this change rather than reusing -`org.apache.kafka.common.protocol.Errors`. This couples this focused API-shape change to the larger -owned-protocol effort (#4752/#4755, proposal 116) and would land an owned type on `main` ahead of -that work. Reusing the existing `Errors` enum keeps this change small and consistent with the current -surface; the swap to an owned enum is a clean, mechanical follow-up once the owned protocol lands. +Vendor the Kroxylicious-owned `Errors` enum as part of this change, rather than reusing Kafka's +`org.apache.kafka.common.protocol.Errors` for now. This couples this focused API-shape change to the +larger owned-protocol effort (#4752/#4755, proposal 116) and would land the owned enum on `main` ahead +of that work. Reusing Kafka's `Errors` enum keeps this change small and consistent with the current +surface; vendoring the owned `Errors` enum is a clean, mechanical follow-up once the owned protocol +lands. From 255836b73d9d5552edc568057063c852d6ed160c Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Wed, 26 Aug 2026 13:17:06 +0100 Subject: [PATCH 7/9] proposal: reframe motivation around vendoring cost, not kafka-clients dep Under proposal 116 ApiException would be vendored into kroxylicious-api, so keeping these methods on ApiException forces the owned API to vendor the ~150-strong exception hierarchy rather than leaving a lingering kafka-clients compile-time dependency. Reword the second motivation bullet accordingly. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Keith Wall --- .../131-error-response-api-in-terms-of-error-codes.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/proposals/131-error-response-api-in-terms-of-error-codes.md b/proposals/131-error-response-api-in-terms-of-error-codes.md index 6dea992..e59f35a 100644 --- a/proposals/131-error-response-api-in-terms-of-error-codes.md +++ b/proposals/131-error-response-api-in-terms-of-error-codes.md @@ -51,10 +51,11 @@ decision only became apparent later: in the API. These two methods are the exception — literally — forcing the caller to route through Kafka's `ApiException` hierarchy to express what the API elsewhere expresses as a code. That inconsistency is a papercut for filter authors and an obstacle to a coherent 1.0 API. -- **Keeps `kafka-clients` on the API surface.** Proposal 116 removes the generated `*Data` classes - and protocol infrastructure from the API. If these two methods keep taking `ApiException`, the - `kafka-clients` exception classes remain a compile-time dependency of every filter that - short-circuits, undermining the goal of a self-contained, Kroxylicious-owned API surface for 1.0. +- **Forces Kafka's exception hierarchy into the owned API.** Proposal 116 makes the API surface + Kroxylicious-owned, vendoring the Kafka protocol types it keeps. If these two methods continue to + take `ApiException`, that type has to be vendored too — dragging in its ~150 subclasses — so the + owned API inherits Kafka's entire exception model just to let a filter name an error, at odds with + the goal of a small, self-contained API surface for 1.0. - **Enables the owned-`Errors` payoff.** Removing `ApiException` from these two signatures takes the exception hierarchy off the public API *shape*, but it does not by itself drop the ~150 subclasses from the owned surface: the API now speaks in Kafka's `Errors` enum, and that enum still references From 922cc1b6186b586d343b4dec48d7fc8e9fa6b612 Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Wed, 26 Aug 2026 13:28:55 +0100 Subject: [PATCH 8/9] remove some unnecessary words Signed-off-by: Keith Wall --- proposals/131-error-response-api-in-terms-of-error-codes.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/proposals/131-error-response-api-in-terms-of-error-codes.md b/proposals/131-error-response-api-in-terms-of-error-codes.md index e59f35a..11d3c10 100644 --- a/proposals/131-error-response-api-in-terms-of-error-codes.md +++ b/proposals/131-error-response-api-in-terms-of-error-codes.md @@ -167,11 +167,7 @@ deprecated overload, no `Throwable` widening, no runtime type-check to maintain. minimising the inconvenience. - **Binary compatibility:** the exception-typed overloads are removed at the bytecode level. A pre-compiled plugin linked against `errorResponse(..., ApiException)` would fail at link time - (`NoSuchMethodError`) until recompiled. This is an accepted, deliberate break and is recorded as an - explicit `japicmp` exclusion. Recompilation against the new API is transparent. (A future runtime - enhancement could catch `LinkageError`/`NoSuchMethodError` in the safe invoker and emit a targeted - "compiled against a different API version" diagnostic; that is out of scope here and belongs with - the API-versioning work.) + (`NoSuchMethodError`) until recompiled. This is an accepted. - **Behavioural parity:** for an equivalent input the new `Errors` overload produces the identical response (same error code, same message) the exception overload produced before; unit tests assert this parity. From cbabb028ff575af9a07d649dd9b077851ba0effa Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Wed, 26 Aug 2026 14:42:19 +0100 Subject: [PATCH 9/9] proposal: reject Errors.NONE in the new error-response API The new Errors overloads must denote an actual error, so Errors.NONE (the absence-of-error sentinel) is rejected with IllegalArgumentException. The errorCode parameter is @NonNull (inherited from package-info), so null is a contract violation. Add a Validation subsection and a runtime-contract compatibility bullet, and name the parameter errorCode. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Keith Wall --- ...or-response-api-in-terms-of-error-codes.md | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/proposals/131-error-response-api-in-terms-of-error-codes.md b/proposals/131-error-response-api-in-terms-of-error-codes.md index 11d3c10..57aa29e 100644 --- a/proposals/131-error-response-api-in-terms-of-error-codes.md +++ b/proposals/131-error-response-api-in-terms-of-error-codes.md @@ -83,10 +83,10 @@ On both `RequestFilterResultBuilder.errorResponse` and `RouterContext.respondWit ```java // code only — uses the Errors default message -errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors error); +errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors errorCode); // code plus an explicit message -errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors error, @Nullable String message); +errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors errorCode, @Nullable String message); ``` `RouterContext.respondWithError` gains the same two overloads. @@ -95,6 +95,17 @@ errorResponse(RequestHeaderData header, ApiMessage requestMessage, Errors error, internally, and consistent with the rest of the API surface on `main` today. When the owned `Errors` enum lands, this single type is swapped for the owned one; call sites are otherwise unchanged. +### Validation + +The `errorCode` must denote an actual error. It carries the package's default `@NonNull` annotation +(inherited from `package-info.java`), so a `null` code is already a documented contract violation. +Beyond that, `Errors.NONE` — the sentinel for the *absence* of an error — is rejected at call time +with `IllegalArgumentException`: asking for an error response that carries no error is a programming +error. This is a genuinely new constraint. The removed exception-based overloads could not express +"no error" (there is no `ApiException` for `NONE`), so nothing that compiled before is affected, and +the check fails fast rather than letting a filter emit a response that claims success on an error +path. + ### Removed API ```java @@ -171,6 +182,10 @@ deprecated overload, no `Throwable` widening, no runtime type-check to maintain. - **Behavioural parity:** for an equivalent input the new `Errors` overload produces the identical response (same error code, same message) the exception overload produced before; unit tests assert this parity. +- **Runtime contract:** the new overloads reject `Errors.NONE` with `IllegalArgumentException`, and + the `errorCode` parameter is `@NonNull` (the package default), so a `null` code is a contract + violation too. The removed exception overloads had no equivalent input, so no existing caller is + affected. ## Rejected alternatives