HTTP exception mapping missing ALREADY_EXISTS
On HTTP exceptions, exceptions that should be ALREADY_EXISTS (409) are being propagated as ABORTED (also 409).
This code change seems to have removed the behavior: https://github.com/googleapis/sdk-platform-java/commit/cdf461425812f5392436fcd8b4dc6096347036c9#diff-ecd504e117f5a7eedb2f2df5df3a32ac6bc85e21c870ff702c0de2771b33a21eL125-L129
Is there a reason for this, or can we restore this logic?
Thanks!
The pull request associated to the commit points to a wrong URL due to repository relocation. The correct pull request is https://github.com/googleapis/gax-java/pull/1570
gax-java surfaces errors in terms of ApiExceptions, which are modeled precisely following the canonical status codes (go/canonical-codes). (That is, there's a one-to-one correspondence between the canonical error codes and the Java exceptions.)
go/canonical-codes does have ALREADY_EXISTS.
@chanseokoh Do you happen to remember why the condition for ALREADY_EXISTS was removed at that time?
ALREADY_EXISTS(409) are being propagated
409, which is an HTTP response status code, is CONFLICT, not ALREADY_EXISTS: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409
as
ABORTED(also 409).
OTOH, ABORTED is a canonical error code, not an HTTP status code. And its value is 10, not 409.
The conversion mapping between the two code spaces is defined by go/http-canonical-mapping.
However, I just realized that the doc also says
The guiding principle is that a mapping is only defined when it is unequivocally correct. In particular, there may be some circumstances where there is a more appropriate HTTP status code for a canonical code (because HTTP codes are finer-grained), and when you know that is the case it is fine to substitute an alternative code.
, so maybe it's fine to restore the previous best-effort logic to identify ALREADY_EXISTS?
But I don't have the expertise on this subject. At least I think the current behavior isn't wrong based on go/http-canonical-mapping.
Memo: "It was compatibility testing errors for gRPC and HTTP for Datastore. gRPC throws ALREADY_EXISTS, but I ran into this issue for HTTP (reGAPIC)." (Kristen)
Discussed offline with @suztomo, I will try to pick this up to fix in future cycles (but if someone needs this sooner, feel free to start work on it 😄 )