You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "ankitsultana (via GitHub)" <gi...@apache.org> on 2023/12/21 04:53:48 UTC

[I] Http Response Headers for Error Codes [pinot]

ankitsultana opened a new issue, #12190:
URL: https://github.com/apache/pinot/issues/12190

   At present, in case of query failures, Pinot broker returns a 200 HTTP Status Code and the message of the body contains a JSON with the exception and the error-code.
   
   I was wondering if we can also return the error-code in a response header. Say: `X-Pinot-Error-Code`.
   
   That way callers don't need to un-marshal the JSON blob to check if a given request failed or not.
   
   This would also be helpful for Pinot query proxies where we want to avoid parsing Pinot's response as much as possible.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Http Response Headers for Error Codes [pinot]

Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on issue #12190:
URL: https://github.com/apache/pinot/issues/12190#issuecomment-1879230662

   Please assign it to me, I will work on it. Thanks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Http Response Headers for Error Codes [pinot]

Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on issue #12190:
URL: https://github.com/apache/pinot/issues/12190#issuecomment-1867294997

   I think we can set it to -1, but open to suggestions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Http Response Headers for Error Codes [pinot]

Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana closed issue #12190: Http Response Headers for Error Codes
URL: https://github.com/apache/pinot/issues/12190


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Http Response Headers for Error Codes [pinot]

Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on issue #12190:
URL: https://github.com/apache/pinot/issues/12190#issuecomment-1866899055

   I think error code 422 make sense here. Thoughts?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Http Response Headers for Error Codes [pinot]

Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on issue #12190:
URL: https://github.com/apache/pinot/issues/12190#issuecomment-1910822002

   hey @ankitsultana, question:
   
   We have some of the places in the code where we throw exceptions as a HTTP response, for example [here](https://github.com/apache/pinot/blob/master/pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java#L194) or [here](https://github.com/apache/pinot/blob/master/pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java#L198). For such scenario, in my understanding, we don't set any particular Error Code. 
   
   Do you think using some general Error Code in such scenarios is a good idea? We can leverage [UNKNOWN_ERROR_CODE](https://github.com/apache/pinot/blob/master/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java#L84) or can introduce a new one say `FAILED_EXECUTION_ERROR_CODE`?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Http Response Headers for Error Codes [pinot]

Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on issue #12190:
URL: https://github.com/apache/pinot/issues/12190#issuecomment-1867161617

   Thanks for explanation. 
   Question, in-case of successful response, header value would be 200(I am not sure about Pinot code incase of successful result)?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Http Response Headers for Error Codes [pinot]

Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on issue #12190:
URL: https://github.com/apache/pinot/issues/12190#issuecomment-1866987690

   I think I can explain better with an example.
   
   The existing API returns responses like this:
   
   ```
   < HTTP/1.1 200 OK
   < Content-Type: application/json
   < Content-Length: 1009
   <
   * Connection #0 to host localhost left intact
   {
     "exceptions": [
       {
         "errorCode": 190,
         "message": "TableDoesNotExistError"
       }
     ],
     "numServersQueried": 0,
     ...
   ```
   
   HTTP Status Code is 200, and the error details are in the JSON Blob.
   
   What we were proposing is that we can add a header in the response, `X-Pinot-Error-Code`, which will be set to the corresponding Pinot Error Code (190 in this case).
   
   We can't change the existing HTTP Status Code because that would be backwards incompatible, so adding a header is a way to hint the caller that there was a error. The goal is that the caller shouldn't have to parse the JSON to determine if a query failed or not.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Http Response Headers for Error Codes [pinot]

Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on issue #12190:
URL: https://github.com/apache/pinot/issues/12190#issuecomment-1914406101

   Good catch. I think the broader issue is that we have two kinds of errors:
   
   1. Ones where a non-200 status code is returned.
   2. Ones where a 200 Status Code is returned.
   
   I think it should be okay to say that this new http response header only tackles Case-2, since case-1 can be detected by simply checking the HTTP Status Code.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org