Skip to content

Commit 6838885

Browse files
jhendrixMSFTjhendrixMSFT
authored
Check HTTP status code before looking for LRO status (Azure#19385)
Per LRO guidelines, operation/RELO must return a success HTTP status code when checking the status regardless of the operation's state. If polling on an LRO returns an unsuccessful HTTP status code, it should be surfaced as an *azcore.ResponseError.
1 parent fcd9668 commit 6838885

File tree

8 files changed

+95
-6
lines changed

8 files changed

+95
-6
lines changed

‎sdk/azcore/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
### Bugs Fixed
1212
* Fixed an issue in `runtime.SetMultipartFormData` to properly handle slices of `io.ReadSeekCloser`.
1313
* Fixed the MaxRetryDelay default to be 60s.
14+
* Failure to poll the state of an LRO will now return an `*azcore.ResponseError` for poller types that require this behavior.
1415

1516
### Other Changes
1617
* Retain contents of read-only fields when sending requests.

‎sdk/azcore/internal/pollers/async/async.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ func (p *Poller[T]) Done() bool {
9999
// Poll retrieves the current state of the LRO.
100100
func (p *Poller[T]) Poll(ctx context.Context) (*http.Response, error) {
101101
err := pollers.PollHelper(ctx, p.AsyncURL, p.pl, func(resp *http.Response) (string, error) {
102+
if !pollers.StatusCodeValid(resp) {
103+
p.resp = resp
104+
return "", exported.NewResponseError(resp)
105+
}
102106
state, err := pollers.GetStatus(resp)
103107
if err != nil {
104108
return "", err

‎sdk/azcore/internal/pollers/async/async_test.go‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ func TestPollFailed(t *testing.T) {
213213
resp.Header.Set(shared.HeaderAzureAsync, fakePollingURL)
214214
poller, err := New[widget](exported.NewPipeline(shared.TransportFunc(func(req *http.Request) (*http.Response, error) {
215215
return &http.Response{
216-
StatusCode: http.StatusBadRequest,
216+
StatusCode: http.StatusOK,
217217
Header: http.Header{},
218218
Body: io.NopCloser(strings.NewReader(`{ "status": "failed" }`)),
219219
}, nil
@@ -222,7 +222,7 @@ func TestPollFailed(t *testing.T) {
222222
require.False(t, poller.Done())
223223
resp, err = poller.Poll(context.Background())
224224
require.NoError(t, err)
225-
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
225+
require.Equal(t, http.StatusOK, resp.StatusCode)
226226
require.True(t, poller.Done())
227227
var result widget
228228
err = poller.Result(context.Background(), &result)
@@ -231,6 +231,31 @@ func TestPollFailed(t *testing.T) {
231231
require.Empty(t, result)
232232
}
233233

234+
func TestPollError(t *testing.T) {
235+
resp := initialResponse(http.MethodPatch, http.NoBody)
236+
resp.Header.Set(shared.HeaderAzureAsync, fakePollingURL)
237+
poller, err := New[widget](exported.NewPipeline(shared.TransportFunc(func(req *http.Request) (*http.Response, error) {
238+
return &http.Response{
239+
StatusCode: http.StatusNotFound,
240+
Header: http.Header{},
241+
Body: io.NopCloser(strings.NewReader(`{ "error": { "code": "NotFound", "message": "the item doesn't exist" } }`)),
242+
}, nil
243+
})), resp, "")
244+
require.NoError(t, err)
245+
require.False(t, poller.Done())
246+
resp, err = poller.Poll(context.Background())
247+
require.Error(t, err)
248+
require.Nil(t, resp)
249+
var respErr *exported.ResponseError
250+
require.ErrorAs(t, err, &respErr)
251+
require.Equal(t, http.StatusNotFound, respErr.StatusCode)
252+
require.False(t, poller.Done())
253+
var result widget
254+
err = poller.Result(context.Background(), &result)
255+
require.ErrorAs(t, err, &respErr)
256+
require.Empty(t, result)
257+
}
258+
234259
func TestPollFailedError(t *testing.T) {
235260
resp := initialResponse(http.MethodPatch, http.NoBody)
236261
resp.Header.Set(shared.HeaderAzureAsync, fakePollingURL)

‎sdk/azcore/internal/pollers/body/body.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ func (p *Poller[T]) Done() bool {
100100

101101
func (p *Poller[T]) Poll(ctx context.Context) (*http.Response, error) {
102102
err := pollers.PollHelper(ctx, p.PollURL, p.pl, func(resp *http.Response) (string, error) {
103+
if !pollers.StatusCodeValid(resp) {
104+
p.resp = resp
105+
return "", exported.NewResponseError(resp)
106+
}
103107
if resp.StatusCode == http.StatusNoContent {
104108
p.resp = resp
105109
p.CurState = pollers.StatusSucceeded

‎sdk/azcore/internal/pollers/body/body_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,3 +185,27 @@ func TestPollFailedError(t *testing.T) {
185185
require.Error(t, err)
186186
require.Nil(t, resp)
187187
}
188+
189+
func TestPollError(t *testing.T) {
190+
resp := initialResponse(http.MethodPatch, strings.NewReader(`{ "properties": { "provisioningState": "Started" } }`))
191+
poller, err := New[widget](exported.NewPipeline(shared.TransportFunc(func(req *http.Request) (*http.Response, error) {
192+
return &http.Response{
193+
StatusCode: http.StatusNotFound,
194+
Header: http.Header{},
195+
Body: io.NopCloser(strings.NewReader(`{ "error": { "code": "NotFound", "message": "the item doesn't exist" } }`)),
196+
}, nil
197+
})), resp)
198+
require.NoError(t, err)
199+
require.False(t, poller.Done())
200+
resp, err = poller.Poll(context.Background())
201+
require.Error(t, err)
202+
require.Nil(t, resp)
203+
var respErr *exported.ResponseError
204+
require.ErrorAs(t, err, &respErr)
205+
require.Equal(t, http.StatusNotFound, respErr.StatusCode)
206+
require.False(t, poller.Done())
207+
var result widget
208+
err = poller.Result(context.Background(), &result)
209+
require.ErrorAs(t, err, &respErr)
210+
require.Empty(t, result)
211+
}

‎sdk/azcore/internal/pollers/op/op.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ func (p *Poller[T]) Done() bool {
9191

9292
func (p *Poller[T]) Poll(ctx context.Context) (*http.Response, error) {
9393
err := pollers.PollHelper(ctx, p.OpLocURL, p.pl, func(resp *http.Response) (string, error) {
94+
if !pollers.StatusCodeValid(resp) {
95+
p.resp = resp
96+
return "", exported.NewResponseError(resp)
97+
}
9498
state, err := pollers.GetStatus(resp)
9599
if err != nil {
96100
return "", err

‎sdk/azcore/internal/pollers/op/op_test.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,15 +238,15 @@ func TestOperationFailed(t *testing.T) {
238238
resp.Header.Set(shared.HeaderOperationLocation, fakePollingURL)
239239
poller, err := New[widget](exported.NewPipeline(shared.TransportFunc(func(req *http.Request) (*http.Response, error) {
240240
return &http.Response{
241-
StatusCode: http.StatusBadRequest,
241+
StatusCode: http.StatusOK,
242242
Body: io.NopCloser(strings.NewReader(`{ "status": "Failed", "error": { "code": "InvalidSomething" } }`)),
243243
}, nil
244244
})), resp, pollers.FinalStateViaLocation)
245245
require.NoError(t, err)
246246
require.False(t, poller.Done())
247247
resp, err = poller.Poll(context.Background())
248248
require.NoError(t, err)
249-
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
249+
require.Equal(t, http.StatusOK, resp.StatusCode)
250250
require.True(t, poller.Done())
251251
var result widget
252252
err = poller.Result(context.Background(), &result)
@@ -270,6 +270,27 @@ func TestPollFailed(t *testing.T) {
270270
require.False(t, poller.Done())
271271
}
272272

273+
func TestPollError(t *testing.T) {
274+
resp := initialResponse(http.MethodPut, strings.NewReader(`{ "status": "Updating" }`))
275+
resp.Header.Set(shared.HeaderOperationLocation, fakePollingURL)
276+
poller, err := New[widget](exported.NewPipeline(shared.TransportFunc(func(req *http.Request) (*http.Response, error) {
277+
return &http.Response{
278+
StatusCode: http.StatusNotFound,
279+
Header: http.Header{},
280+
Body: io.NopCloser(strings.NewReader(`{ "error": { "code": "NotFound", "message": "the item doesn't exist" } }`)),
281+
}, nil
282+
})), resp, pollers.FinalStateViaLocation)
283+
require.NoError(t, err)
284+
require.False(t, poller.Done())
285+
resp, err = poller.Poll(context.Background())
286+
require.Error(t, err)
287+
require.Nil(t, resp)
288+
var respErr *exported.ResponseError
289+
require.ErrorAs(t, err, &respErr)
290+
require.Equal(t, http.StatusNotFound, respErr.StatusCode)
291+
require.False(t, poller.Done())
292+
}
293+
273294
func TestMissingStatus(t *testing.T) {
274295
resp := initialResponse(http.MethodPatch, strings.NewReader(`{ "status": "Updating" }`))
275296
resp.Header.Set(shared.HeaderOperationLocation, fakePollingURL)

‎sdk/azcore/internal/pollers/util_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,10 @@ func TestPollHelper(t *testing.T) {
233233

234234
require.Error(t, err)
235235
pl = exported.NewPipeline(shared.TransportFunc(func(*http.Request) (*http.Response, error) {
236-
return &http.Response{}, nil
236+
return &http.Response{
237+
StatusCode: http.StatusNotFound,
238+
Body: http.NoBody,
239+
}, nil
237240
}))
238241
err = PollHelper(context.Background(), fakeEndpoint, pl, func(*http.Response) (string, error) {
239242
return "", errors.New("failed")
@@ -242,7 +245,10 @@ func TestPollHelper(t *testing.T) {
242245

243246
require.Error(t, err)
244247
pl = exported.NewPipeline(shared.TransportFunc(func(*http.Request) (*http.Response, error) {
245-
return &http.Response{}, nil
248+
return &http.Response{
249+
StatusCode: http.StatusOK,
250+
Body: http.NoBody,
251+
}, nil
246252
}))
247253
err = PollHelper(context.Background(), fakeEndpoint, pl, func(*http.Response) (string, error) {
248254
return "inProgress", nil

0 commit comments

Comments
 (0)