proposal: express error-response API in terms of error codes (#4756)

[k-wall]

Design proposal to re-express the short-circuit error-response entry points on the public Filter/Router API in terms of Errors codes rather than the kafka-clients ApiException hierarchy.

• RequestFilterResultBuilder.errorResponse and RouterContext.respondWithError gain (Errors) and (Errors, @Nullable String message) overloads.
• The exception-based overloads are removed outright (clean break) — no deprecation window, no transitional signature — taking the last kafka-clients exception type off the public API surface in one step.
• Removes the last places where the kafka-clients exception hierarchy leaks into the public API surface, complementing proposal 116 – Own the Kafka Protocol API Surface.

Note

Direction change following review. An earlier revision deprecated the exception overloads and widened them from ApiException to java.lang.Throwable to preserve source compatibility. Review discussion (thanks @robobario, @tombentley, @SamBarker) prompted that thought that the transitional machinery — a runtime type-guard, a deprecation window, and japicmp bookkeeping — buys little: filter authors must already edit source in 0.24.0 to migrate off Kafka's *Data classes (proposal 116), so the source break rides along with a change they are making anyway. The proposal now does a clean break, and the deprecate-and-widen approach is captured under Rejected alternatives.

Relates to kroxylicious/kroxylicious#4756 and implementation is in kroxylicious/kroxylicious#4757.

[robobario]

+1, looks comprehensive to me thanks @k-wall.

The runtime lose a small amount of debug information in that we had an exception stack trace prior, but I think the message should compensate and Filters should be logging their own problems if they're truly caused by some problem, rather than everyday Kafka Protocol semantics that use error codes.

[robobario]

Just wondering what the migration here looks like, since we still have these Throwable signatures established filters will continue to compile while sending down kafka-clients Exceptions. Will we maintain a translation from kafka-clients error class -> kroxylicious Errors enum until we delete the Throwable signature?

[k-wall]

If the Filter author is using the kafka-client's exceptions, then they must have the kafka-client on the classpath.
So the runtime can use a reflective technique to get the recover the error code from the exception. This frees the kroxylicious-runtime of the explicit dependency and eases the transition path.

However, after sleeping on it, I am wondering if the Throwable stuff is really worth it. The filter author already going to have to make a code change for 0.24 anyway (Data classes etc) and if they are using the Filters APIs with ApiException, they'll need to add the kafka-client to their class path.

Maybe we just accept the fact they'll need to change:

                return context.requestFilterResultBuilder().errorResponse(header, request, Errors.GROUP_AUTHORIZATION_FAILED.exception()).completed();

to:

                return context.requestFilterResultBuilder().errorResponse(header, request, Errors.GROUP_AUTHORIZATION_FAILED).completed();
            }

Pulling the plaster off quickly in one go :)

[SamBarker]

I have kroxylicious/kroxylicious#4745 which hopes to make the existing migration fully automatic.

It could probably automate the highlighted example but I'm not sure it can catch all cases. (after a day fighting with open-rewrite I'm not best inclined to try and be clever with it yet.)

[k-wall]

If openrewrite can help with the vanilla case, great. If there are a few edge cases that Filter Authors need to handle themselves (new SomeApiException(msg)) so be it.

[SamBarker]

LGTM

I think thats a good escape hatch from our current migration quagmire but has the bonus of narrowing our public API surface.

[SamBarker]

I asked Gemini about this.

Yes, the runtime can reasonably detect or mitigate this NoSuchMethodError both proactively at startup and reactively at runtime.

Proactive Option: Startup Bytecode Scanning (Recommended)
During plugin loading and validation (e.g., via ServiceLoader or extension initialization), the runtime can scan the bytecode of registered filter classes using a lightweight library like ASM or ClassGraph (which proxy runtimes often already embed):

How it works: Inspect the class file's constant pool or method instruction references (invokevirtual/invokeinterface) for calls matching the exact old signature descriptor: errorResponse(..., Lorg/apache/kafka/common/errors/ApiException;).

Outcome: If detected, the runtime halts startup immediately with a clear, actionable log: "Plugin 'X' was compiled against an older version of Kroxylicious and calls a removed binary signature. Please recompile against version 0.24.0+."

Why it helps: It converts a delayed, silent runtime failure that only surfaces when an error condition occurs into a predictable, fail-fast startup check.

Reactive Option: LinkageError Catch Blocks
Because NoSuchMethodError is a java.lang.Error (a subclass of LinkageError), unhandled calls will bypass standard catch (Exception e) blocks in the filter execution engine.

How it works: The runtime's pipeline executor wraps filter method invocations with a catch block for NoSuchMethodError or LinkageError.

Outcome: Rather than causing an unhandled thread crash or opaque error response, the runtime catches the error, inspects the stack trace, and logs a targeted diagnostic pointing directly to the breaking API change.

I think its suggestion of proactive scanning is interesting and something we should think about, probably alongside the Api Versioning proposals. however I think we have too much complexity in flight right now to pull on that thread.

I do wonder if the reactive version is worth looking at. Catch all linkageErrors or NoSuchMethodErrors in the safe invoker and provide a blanket suggestion that the filter was compiled against a different API versions?

[k-wall]

I've removed the deprecated methods from the proposal. I decided they were bringing too little benefit to justify the complexity.

[k-wall]

+1, looks comprehensive to me thanks @k-wall.

The runtime lose a small amount of debug information in that we had an exception stack trace prior

True, but the runtime has no valid use-case to look at ay chained exception. Anything chained belonged to the Filter. It was at most informational.

, but I think the message should compensate and Filters should be logging their own problems if they're truly caused by some problem, rather than everyday Kafka Protocol semantics that use error codes.

Yes.

[k-wall]

@tombentley @robobario @SamBarker thanks for the feedback, I've updated the proposal. Please re-review.