Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.3.0
    • Fix Version/s: 1.4.0
    • Component/s: spec
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Currently Java internally adds a type at the start of each protocol message's error list for system errors. This is used when an exception is encountered on the server while executing an RPC that does not match one of the messages declared errors. The type is "string", so it will never conflict with another error, since all other errors are defined as records.

      For example, if a protocol has a message like:

      "foo": {
        "request": [],
        "response": "null",
        "errors": ["MyError"]
      }
      

      Then errors are written and read with the schema ["string", "MyError"], although that schema never appears publicly.

      This should either be documented or changed.

      1. AVRO-447.patch
        2 kB
        Doug Cutting
      2. AVRO-447.patch
        2 kB
        Doug Cutting
      3. AVRO-447.patch
        2 kB
        Doug Cutting

        Issue Links

          Activity

          Hide
          Doug Cutting added a comment -

          One alternative is, rather than putting the system error at the front of the union, to have three types of responses: normal (value is "response" schema), error (value is "error" schema) or system_error (value is "string"). This would involve changing the "error flag" boolean into an enum. This would be an incompatible change, but I doubt much depends on this currently undocumented behaviour. Rather I suspect that errors do not currently operate correctly between java and other languages.

          Show
          Doug Cutting added a comment - One alternative is, rather than putting the system error at the front of the union, to have three types of responses: normal (value is "response" schema), error (value is "error" schema) or system_error (value is "string"). This would involve changing the "error flag" boolean into an enum. This would be an incompatible change, but I doubt much depends on this currently undocumented behaviour. Rather I suspect that errors do not currently operate correctly between java and other languages.
          Hide
          Jeff Hodges added a comment -

          For those interested, the current ruby implementation only has the string as a valid type if there is no value given for the errors key. That is:

          writers_schema = remote_message_schema.errors || SYSTEM_ERROR_SCHEMA
          readers_schema = local_message_schema.errors || SYSTEM_ERROR_SCHEMA
          

          I will be changing this to be inclusive in a new ticket.

          Show
          Jeff Hodges added a comment - For those interested, the current ruby implementation only has the string as a valid type if there is no value given for the errors key. That is: writers_schema = remote_message_schema.errors || SYSTEM_ERROR_SCHEMA readers_schema = local_message_schema.errors || SYSTEM_ERROR_SCHEMA I will be changing this to be inclusive in a new ticket.
          Hide
          Jeff Hammerbacher added a comment -

          Was bitten by this one (see AVRO-558). After discussion with Doug, marking as a blocker for 1.4.0.

          Show
          Jeff Hammerbacher added a comment - Was bitten by this one (see AVRO-558 ). After discussion with Doug, marking as a blocker for 1.4.0.
          Hide
          Doug Cutting added a comment -

          Here's a patch that documents the system error schema.

          Show
          Doug Cutting added a comment - Here's a patch that documents the system error schema.
          Hide
          Jeff Hammerbacher added a comment -

          Hey Doug,

          Looks good to me, with one addition: let's be explicit about the fact that the JSON serialization of the schema only includes the specified error union, not the effective union.

          Thanks,
          Jeff

          Show
          Jeff Hammerbacher added a comment - Hey Doug, Looks good to me, with one addition: let's be explicit about the fact that the JSON serialization of the schema only includes the specified error union, not the effective union. Thanks, Jeff
          Hide
          Jeff Hammerbacher added a comment -

          Hey Doug,

          Also, I used the term "declared errors" instead of "specified errors" in my patch for AVRO-560. Perhaps "declared" can be used?

          Thanks,
          Jeff

          Show
          Jeff Hammerbacher added a comment - Hey Doug, Also, I used the term "declared errors" instead of "specified errors" in my patch for AVRO-560 . Perhaps "declared" can be used? Thanks, Jeff
          Hide
          Doug Cutting added a comment -

          Here's a new version that incorporates Jeff's suggestions.

          Show
          Doug Cutting added a comment - Here's a new version that incorporates Jeff's suggestions.
          Hide
          Jeff Hammerbacher added a comment -

          Let's change "appended to the front of the declared union" to "prepended to the declared union" and "effective union, however a protocol's" to "effective union; however, a protocol's" and ship it.

          Show
          Jeff Hammerbacher added a comment - Let's change "appended to the front of the declared union" to "prepended to the declared union" and "effective union, however a protocol's" to "effective union; however, a protocol's" and ship it.
          Hide
          Doug Cutting added a comment -

          Here's a new version incorporating Jeff's latest suggestions.

          Show
          Doug Cutting added a comment - Here's a new version incorporating Jeff's latest suggestions.
          Hide
          Doug Cutting added a comment -

          I just committed this.

          Show
          Doug Cutting added a comment - I just committed this.

            People

            • Assignee:
              Doug Cutting
              Reporter:
              Doug Cutting
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development