Uploaded image for project: 'Couchbase Server'
  1. Couchbase Server
  2. MB-50876

Error handling API was broken in analytics 7.1.0-2256

    XMLWordPrintable

Details

    • Bug
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • 7.1.0
    • 7.1.0
    • analytics
    • Untriaged
    • 1
    • Yes
    • CX Sprint 282

    Description

      SDKs expect that "status" field will always be rendered in JSON payload of the service reseponse. It seems like recent changes in rendering errors, changed the shape of the payload from plain text to JSON, but the new structure was not discussed/reviewed by SDK team representative.

      This is our specification for analytics service, and it states that status is not optional field.
      https://github.com/couchbaselabs/sdk-rfcs/blob/master/rfc/0057-sdk3-analytics.md#analyticsresult

      Previous versions were rendering some errors as plain text, and the SDKs adapted to parse that representation. Error handling is critical component of the SDKs, and we expect to be notified about any changes in the on-the-wire message representation.

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            Michael Blow yes, "status": "errors" will work I think. Are there any other places that have also been changed and we need to check before 7.1 ships?

            daschl Michael Nitschinger added a comment - Michael Blow yes, "status": "errors" will work I think. Are there any other places that have also been changed and we need to check before 7.1 ships?
            michael.blow Michael Blow added a comment - - edited

            Are there any other places that have also been changed and we need to check before 7.1 ships?

            Other places other than the link servlet? Yes, there were quite a few instances where we were either:
            1. returning incomplete JSON
            2. returning error codes as text using code:msg
            3. returning text messages with content-type JSON

            All of these now return a response with an appropriate content-type. They all return error code responses in the same manner, so I will update these to include a status.

            If you need to have more specifics as to which APIs were updated, I can probably compile that.

            michael.blow Michael Blow added a comment - - edited Are there any other places that have also been changed and we need to check before 7.1 ships? Other places other than the link servlet? Yes, there were quite a few instances where we were either: 1. returning incomplete JSON 2. returning error codes as text using code:msg 3. returning text messages with content-type JSON All of these now return a response with an appropriate content-type. They all return error code responses in the same manner, so I will update these to include a status. If you need to have more specifics as to which APIs were updated, I can probably compile that.

            I think we are good as long as they all "follow" the api and include the status and have the same response format.

            daschl Michael Nitschinger added a comment - I think we are good as long as they all "follow" the api and include the status and have the same response format.

            Build couchbase-server-7.1.0-2281 contains cbas-core commit 0496f67 with commit message:
            MB-50876: include requisite status on json error response

            build-team Couchbase Build Team added a comment - Build couchbase-server-7.1.0-2281 contains cbas-core commit 0496f67 with commit message: MB-50876 : include requisite status on json error response
            umang.agrawal Umang added a comment -

            Verified with build 7.1.0-2308.

            umang.agrawal Umang added a comment - Verified with build 7.1.0-2308.

            People

              umang.agrawal Umang
              avsej Sergey Avseyev
              Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes

                  PagerDuty