-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HTTP exception mapping missing ALREADY_EXISTS #2620
Comments
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
go/canonical-codes does have ALREADY_EXISTS. @chanseokoh Do you happen to remember why the condition for ALREADY_EXISTS was removed at that time? |
409, which is an HTTP response status code, is
OTOH, The conversion mapping between the two code spaces is defined by go/http-canonical-mapping. However, I just realized that the doc also says
, so maybe it's fine to restore the previous best-effort logic to identify |
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 😄 ) |
On HTTP exceptions, exceptions that should be
ALREADY_EXISTS
(409) are being propagated asABORTED
(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!
The text was updated successfully, but these errors were encountered: