Skip to content

Commit

Permalink
Don't consider 429 as terminal failure for Location poller (#22651)
Browse files Browse the repository at this point in the history
If the polling request is being throttled, preserve the current state
and return the *http.Response.
  • Loading branch information
jhendrixMSFT committed Mar 29, 2024
1 parent a25e0d3 commit 3c80879
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 0 deletions.
1 change: 1 addition & 0 deletions sdk/azcore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
### Bugs Fixed

* `runtime.SetMultipartFormData` won't try to stringify `[]byte` values.
* Pollers that use the `Location` header won't consider `http.StatusTooManyRequests` a terminal failure.

### Other Changes

Expand Down
4 changes: 4 additions & 0 deletions sdk/azcore/internal/pollers/loc/loc.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +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. we DO NOT want to
// include this as terminal failure so preserve the
// existing state and return the response.
} else {
p.CurState = poller.StatusFailed
}
Expand Down
24 changes: 24 additions & 0 deletions sdk/azcore/internal/pollers/loc/loc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared"
"github.com/Azure/azure-sdk-for-go/sdk/internal/mock"
"github.com/Azure/azure-sdk-for-go/sdk/internal/poller"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -173,3 +174,26 @@ func TestSynchronousCompletion(t *testing.T) {
require.Equal(t, poller.StatusSucceeded, lp.CurState)
require.True(t, lp.Done())
}

func TestWithThrottling(t *testing.T) {
srv, close := mock.NewServer()
defer close()
srv.AppendResponse(mock.WithStatusCode(http.StatusTooManyRequests))
srv.AppendResponse(mock.WithStatusCode(http.StatusAccepted))
srv.AppendResponse(mock.WithStatusCode(http.StatusTooManyRequests))
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 3c80879

Please sign in to comment.