HTTP exception mapping missing ALREADY_EXISTS

[kolea2]

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: cdf4614#diff-ecd504e117f5a7eedb2f2df5df3a32ac6bc85e21c870ff702c0de2771b33a21eL125-L129

Is there a reason for this, or can we restore this logic?

Thanks!

[suztomo]

The pull request associated to the commit points to a wrong URL due to repository relocation. The correct pull request is googleapis/gax-java#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?

[chanseokoh]

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?

[chanseokoh]

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.

[suztomo]

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)

[kolea2]

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 😄 )