[jira] [Commented] (KAFKA-7787) Add error specifications to KAFKA-7609

["Tom Bentley (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KAFKA-7787?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17017873#comment-17017873 ] 

Tom Bentley commented on KAFKA-7787:
------------------------------------

[~cmccabe] I was thinking about this and have a partially working implementation, but wanted your thoughts before I spend more time on it.

cc: [~hachikuji] who raised the original comment on the PR for the message generator.

The following approach works for all enum-like codes in the protocol, not just error codes.

h2. Declaring coded values

The generator will support a new kind of JSON object input {{"type": "codes"}}, which describes a set of distinct named integer (byte, short, etc) values. For example:
{code:language=js}
{
  "type": "codes",
  "name": "ErrorCodes",
  "valueType": "int16"
  "codes": [
    { "name": "UNKNOWN_SERVER_ERROR", "value": -1,
      "about": "The server experienced an unexpected error when processing the request." },
    { "name": "NONE", "value": 0,
      "about": "No error." },
    { "name": "OFFSET_OUT_OF_RANGE", "value": 1,
      "about": "The requested offset is not within the range of offsets maintained by the server." },
  ...
  ]
}
{code}

 * The `valueType` is the type of the integer values.
 * The `codes` lists each of the allowed values. The `about` is optional.

This would generate a class of constants:

{code:language=java}
class ErrorCodes {
    public final static short UNKNOWN_SERVER_ERROR = -1;
    public final static short NONE = 0;
    ...

    public static boolean isValid(short v) {
        return NONE <= v && v <= MAX; 
    }
}
{code}

* The {{isValid()}} method validates that its parameter is one of the allowed values.
* It's an error for two constants to have the same value.
* There need be no requirement for the values to be contiguous.


Continuing the example this allows the existing `Errors` enum to be written as:

{code:language=java}
enum Errors {
    NONE(ErrorCodes.NONE, ...);
    ...
}
{code}

h2. Using codes in field specs

The field spec will support a {{domain}} property which names the set of codes that values of the field may take. For example an {{ErrorCode}} field:

{code:language=js}
 {
  "name": "ErrorCode",
  "type": "int16",
  "domain": {
    "name": "ErrorCodes",
    "values": [
      { "name": "NONE", "validVersions": "0+" },
      { "name": "FOO", "validVersions": "0+" },
      { "name": "BAR", "validVersions": "3+" },
      ...
    ]
   }
 }
{code}

* The {{name}} is the name of a corresponding codes declaration.
* The {{values}} is optional. When it's missing then any of the values in the codes declaration are permitted. When it's present, then only the given values are allowed. Values are given as an object with a `name` that identifies a value from the codes declaration and optionally, a {validVersions} which allows a given code to only be allowed in the given versions of the message.

The owning {Data} class (or inner classes of the {Data} class) will gain a method for validating the error codes. The implementation would depend on whether {values} and/or {validVersions} were given, but might look like this:

{code:language=java}
public static boolean isValidErrorCode(short v, short version) {
    switch (version) {
    case 0:
    case 1:
    case 2:
        return v == ErrorCodes.NONE || v == ErrorCodes.FOO;
    case 3:
        return v == ErrorCodes.NONE || v == ErrorCodes.FOO || v == ErrorCodes.BAR;
     }
}
{code}

h2. Validation

We can call the validation methods and throw:

 * When serializing requests
 * When deserializing requests
 * When serializing responses, except for error code fields.

The reason for distinguishing error code fields arises from the difficultly of knowing for certain which exception types can be thrown in the code called from the handler in the broker. We don't want a mistake the allowed error codes to result in a needless exception in the broker. So for these instead of throwing we could log the unexpected value.

We could use properties of the field spec to configure what code was generated for serialization and deserialization on a per-message basis.

Thoughts?



> Add error specifications to KAFKA-7609
> --------------------------------------
>
>                 Key: KAFKA-7787
>                 URL: https://issues.apache.org/jira/browse/KAFKA-7787
>             Project: Kafka
>          Issue Type: Sub-task
>            Reporter: Colin McCabe
>            Assignee: Tom Bentley
>            Priority: Minor
>
> In our RPC JSON, it would be nice if we could specify what versions of a response could contain what errors.  See the discussion here: https://github.com/apache/kafka/pull/5893#discussion_r244841051



--
This message was sent by Atlassian Jira
(v8.3.4#803005)