Skip to content

Commit

Permalink
Don't consider 408 as terminal failure for Location poller (#22674)
Browse files Browse the repository at this point in the history
Treat 408 the same as 429, preserving state and returning the response.
  • Loading branch information
jhendrixMSFT committed Apr 2, 2024
1 parent e036aea commit 4116d5d
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 4 deletions.
2 changes: 2 additions & 0 deletions sdk/azcore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

* Pollers that use the `Location` header won't consider `http.StatusRequestTimeout` a terminal failure.

### Other Changes

## 1.11.0 (2024-04-01)
Expand Down
8 changes: 4 additions & 4 deletions sdk/azcore/internal/pollers/loc/loc.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ func (p *Poller[T]) Poll(ctx context.Context) (*http.Response, error) {
} else if resp.StatusCode > 199 && resp.StatusCode < 300 {
// any 2xx other than a 202 indicates success
p.CurState = poller.StatusSucceeded
} else if resp.StatusCode == http.StatusTooManyRequests {
// the request is being throttled. DO NOT include
// this as a terminal failure. preserve the
// existing state and return the response.
} else if resp.StatusCode == http.StatusRequestTimeout || resp.StatusCode == http.StatusTooManyRequests {
// the request timed out or is being throttled.
// DO NOT include this as a terminal failure. preserve
// the existing state and return the response.
} else {
p.CurState = poller.StatusFailed
}
Expand Down
23 changes: 23 additions & 0 deletions sdk/azcore/internal/pollers/loc/loc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,26 @@ func TestWithThrottling(t *testing.T) {
require.EqualValues(t, 4, respCount)
require.EqualValues(t, poller.StatusSucceeded, lp.CurState)
}

func TestWithTimeout(t *testing.T) {
srv, close := mock.NewServer()
defer close()
srv.AppendResponse(mock.WithStatusCode(http.StatusAccepted))
srv.AppendResponse(mock.WithStatusCode(http.StatusRequestTimeout))
srv.AppendResponse(mock.WithStatusCode(http.StatusRequestTimeout))
srv.AppendResponse(mock.WithStatusCode(http.StatusOK))
resp := initialResponse()
resp.Header.Set(shared.HeaderLocation, srv.URL())
lp, err := New[struct{}](exported.NewPipeline(shared.TransportFunc(func(req *http.Request) (*http.Response, error) {
return srv.Do(req)
})), resp)
require.NoError(t, err)
respCount := 0
for !lp.Done() {
_, err = lp.Poll(context.Background())
require.NoError(t, err)
respCount++
}
require.EqualValues(t, 4, respCount)
require.EqualValues(t, poller.StatusSucceeded, lp.CurState)
}

0 comments on commit 4116d5d

Please sign in to comment.