Skip to content

Commit

Permalink
some APIs treat 412s as the conflict code. (#4277) (#7915)
Browse files Browse the repository at this point in the history
* some APIs treat 412s as the conflict code.

it stands for 'invalid etag' in that context.

* add unit test.

Signed-off-by: Modular Magician <magic-modules@google.com>
  • Loading branch information
modular-magician authored Dec 2, 2020
1 parent 431682b commit ec11e2b
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 2 deletions.
3 changes: 3 additions & 0 deletions .changelog/4277.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
iam: fixed iam conflict handling so that optimistic-locking retries will succeed more often.
```
4 changes: 2 additions & 2 deletions google/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,11 @@ func isFailedPreconditionError(err error) bool {
}

func isConflictError(err error) bool {
if e, ok := err.(*googleapi.Error); ok && e.Code == 409 {
if e, ok := err.(*googleapi.Error); ok && (e.Code == 409 || e.Code == 412) {
return true
} else if !ok && errwrap.ContainsType(err, &googleapi.Error{}) {
e := errwrap.GetType(err, &googleapi.Error{}).(*googleapi.Error)
if e.Code == 409 {
if e.Code == 409 || e.Code == 412 {
return true
}
}
Expand Down
22 changes: 22 additions & 0 deletions google/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,3 +575,25 @@ func TestRetryTimeDuration_URLTimeoutsShouldRetry(t *testing.T) {
t.Errorf("expected the retryFunc to be called %v time(s), instead was called %v time(s)", expectedRunCount, runCount)
}
}

func TestConflictError(t *testing.T) {
confErr := &googleapi.Error{
Code: 409,
}
if !isConflictError(confErr) {
t.Error("did not find that a 409 was a conflict error.")
}
if !isConflictError(errwrap.Wrapf("wrap", confErr)) {
t.Error("did not find that a wrapped 409 was a conflict error.")
}
confErr = &googleapi.Error{
Code: 412,
}
if !isConflictError(confErr) {
t.Error("did not find that a 412 was a conflict error.")
}
if !isConflictError(errwrap.Wrapf("wrap", confErr)) {
t.Error("did not find that a wrapped 412 was a conflict error.")
}
// skipping negative tests as other cases may be added later.
}

0 comments on commit ec11e2b

Please sign in to comment.