[#11001] Handle datastore exceptions using the new Google-provided SDK #11025
Conversation
|
Hi @jianhandev, these parts of your pull request do not appear to follow our contributing guidelines:
|
jianhandev
changed the title
| if (error.equals(DatastoreTimeoutException.class.getSimpleName())) { | ||
| throw new DatastoreTimeoutException("DatastoreTimeoutException testing"); | ||
| if (error.equals(DatastoreException.class.getSimpleName())) { | ||
| throw new DatastoreException(4, "DatastoreException testing", "DEADLINE_EXCEEDED"); |
There was a problem hiding this comment.
Maybe we can assign a variable name instead of hardcoding the HTTP status code here.
Besides, according to documentation:
DEADLINE_EXCEEDED: A deadline was exceeded on the server.
It seems to be a server-side error and therefore status code should be 5xx?
There was a problem hiding this comment.
It seems to be a server-side error and therefore status code should be 5xx?
I perceived the code to be one of the canonical error codes here. I'm not 100% sure whether it refers to the canonical or the HTTP status code (if it is, it would be 504 Gateway Timeout). 'DEADLINE_EXCEEDED' has error code 4 which I'm assuming corresponds to a timeout exception.
// The deadline expired before the operation could complete. For operations
// that change the state of the system, this error may be returned
// even if the operation has completed successfully. For example, a
// successful response from a server could have been delayed long
// enough for the deadline to expire.
//
// HTTP Mapping: 504 Gateway Timeout
DEADLINE_EXCEEDED = 4;
There was a problem hiding this comment.
@jianhandev OK, I think 4 should be correct.
Their devs seem to be confused as well.
googleapis/google-cloud-java#2442 (comment)
| @@ -105,7 +105,7 @@ private void invokeServlet(HttpServletRequest req, HttpServletResponse resp) thr | |||
| log.warning(enfe.getClass().getSimpleName() + " caught by WebApiServlet: " | |||
| + TeammatesException.toStringWithStackTrace(enfe)); | |||
| throwError(resp, HttpStatus.SC_NOT_FOUND, enfe.getMessage()); | |||
| } catch (DeadlineExceededException | DatastoreTimeoutException e) { | |||
| } catch (DeadlineExceededException | DatastoreException e) { | |||
There was a problem hiding this comment.
This block translates the exception to 504 Gateway Timeout error, but I believe only a small subset of DatastoreException fits into that.
There was a problem hiding this comment.
Based on this file only DEADLINE_EXCEEDED maps to 504 Gateway Timeout. I'm thinking we can handle DatastoreException separately in a new catch clause and throw an error with HttpStatus .SC_INTERNAL_SERVER_ERROR and the specific error message depending on the type of datastore error.
…/teammates into 11001-handle-datastore-exceptions-with-new-api
Derek-Hardy
added
s.ToMerge
#11025) * Remove usage of DatastoreTimeoutException from old api * Get WebApiServlet to throw specific datastore error messages * Replace hard-coded values with enum values of google.rpc.Code
#11025) * Remove usage of DatastoreTimeoutException from old api * Get WebApiServlet to throw specific datastore error messages * Replace hard-coded values with enum values of google.rpc.Code
#11025) * Remove usage of DatastoreTimeoutException from old api * Get WebApiServlet to throw specific datastore error messages * Replace hard-coded values with enum values of google.rpc.Code
#11025) * Remove usage of DatastoreTimeoutException from old api * Get WebApiServlet to throw specific datastore error messages * Replace hard-coded values with enum values of google.rpc.Code
#11025) * Remove usage of DatastoreTimeoutException from old api * Get WebApiServlet to throw specific datastore error messages * Replace hard-coded values with enum values of google.rpc.Code
#11025) * Remove usage of DatastoreTimeoutException from old api * Get WebApiServlet to throw specific datastore error messages * Replace hard-coded values with enum values of google.rpc.Code
#11025) * Remove usage of DatastoreTimeoutException from old api * Get WebApiServlet to throw specific datastore error messages * Replace hard-coded values with enum values of google.rpc.Code
wkurniawan07
added
the
c.Task
#11025) * Remove usage of DatastoreTimeoutException from old api * Get WebApiServlet to throw specific datastore error messages * Replace hard-coded values with enum values of google.rpc.Code
…ovided SDK (TEAMMATES#11025) * Remove usage of DatastoreTimeoutException from old api * Get WebApiServlet to throw specific datastore error messages * Replace hard-coded values with enum values of google.rpc.Code
Part of #11001
In the new Google-provided SDK used by Objectify 6 to access the datastore, exceptions are handled using
DatastoreExceptionwhich classifies errors based on canonical error codes (e.g.NOT_FOUND,DEADLINE_EXCEEDEDetc).Considerations made:
DatastoreTimeoutException(provided by the old appengine api) is thrown if a timeout is encountered when trying to access the the datastore. Now the Google-provided SDK used by Objectify 6 does not have such a specific class to handle timeouts, canonical error codes are used instead to specify error type.Solution:
DatastoreExceptionseparately fromDeadlineExceededExceptionto throw more specific datastore error messages.