ARM REST methods fail on 201 response

[axw]

I am testing out the ARM APIs, and have observed frequent (but not 100%) failures from methods due to "201 Created" being returned (e.g. by ResourceGroupsClient.CreateOrUpdate).

Looking at the code, I think the second-last "autorest.Respond" in each method that expects status codes other than 200 is invalid: in each, it uses WithErrorUnlessOK; I think it should be using WithErrorUnlessStatusCode, with the same values passed into DoErrorUnlessStatusCode in the SendWithSender call above.

[ahmetb]

@axw are the respective REST calls actually returning 200/201 depending on the situation? If an endpoint returns both status codes we should not be failing. Or are we checking for the wrong status code?

[axw]

@ahmetalpbalkan The resource-group creation is responding with 201.

If you look here:
https://github.com/Azure/azure-sdk-for-go/blob/master/arm/resources/resourcegroups.go#L202
the initial check on response status will not error if it's 201. But then it continues on to here:
https://github.com/Azure/azure-sdk-for-go/blob/master/arm/resources/resourcegroups.go#L211
and will return an error because the status code is not 200/OK.

[brendandixon]

@axw Good catch! Your suggestion is correct. The code to support multiple "Ok" return codes came late and, obviously, missed a statement. We'll make changes to the needed changes to the generator. Thank you for finding this!

[axw]

@brendandixon No worries, thank you.

[vgarg]

@brendandixon just FYI, noticing a similar behavior when deleting subnets. Current SDK throws the following error upon deletion call:

Error: autorest:WithErrorUnlessStatusCode GET https://management.azure.com/subscriptions/***/providers/Microsoft.Network/locations/westus/operationResults/eddb29d0-f856-403a-bba1-233cafaaf706?api-version=2015-05-01-preview failed with 204 No Content

Note that the subnet actually gets deleted.

[brendandixon]

@vgarg What method did you use? SubnetsClient#Delete? If so, then this is the same issue. As the code is generated, it's either all correct or all wrong. :)

[vgarg]

Yep, I used SubnetsClient#Delete.

[brendandixon]

@vgarg Ok. Should be the same issue then. The sender has the correct set of return codes (e.g., 204 - No Content), but the responder does not.

[vgarg]

👍 Yeah, applying a similar fix to my copy. Frankly can't wait to get the official fixes for these as wrangling with godeps unhappiness is not fun :)

[brendandixon]

@vgarg @axw This issue is addressed in the latest changes. Why not take a look and let us know. Thanks!

[brendandixon]

Changes merged from PR #221

[vgarg]

Thanks @brendandixon . Looking forward to trying this new release.

[axw]

@brendandixon It seems there's still some issues. I just got a similar error, deleting a resource group. It works most of the time.

Error:

autorest:DoErrorIfStatusCode GET https://management.azure.com/subscriptions/6f0e3b87-f601-40fd-9727-76d05563fedd/operationresults/eyJqb2JJZCI6IlJFU09VUkNFR1JPVVBERUxFVElPTkpPQi1KVUpVOjJERU5WSVJPTk1FTlQ6MkQ1QTMzNUU5RjoyRDQ3QUE6MkQ0OUE0OjJEOHxFQ0U0NTEyNEEyRkVFMDI0LVNPVVRIRUFTVEFTSUEiLCJqb2JMb2NhdGlvbiI6InNvdXRoZWFzdGFzaWEifQ?api-version=2014-04-01-preview failed with 202 Accepted

[brendandixon]

@axw Exactly which API did you use? The standard Delete method for a ResourceGroup (https://github.com/Azure/azure-sdk-for-go/blob/master/arm/resources/groups.go@216) appears to properly handle 202 Accepted.

[axw]

@brendandixon Sorry, should have been clearer. Yes, GroupsClient.Delete is the one. I did see that in the code, so I'm not sure why it would have happened. My local clone is up-to-date.

I'm digging into the code now to understand how the operationresults query is made.

[axw]

@brendandixon It's possible that the deletion attempt took > 15mins, the default polling duration. If it happens again, I'll report back.

[brendandixon]

@axw All good. I think I understand why you saw the error: 15 minutes is the default (pulled out of thin air) polling time (that is, the maximum amount of time the request polls for success). If the request failed to complete within that window, then the code returns whatever the last response code was (in this case, 202 Accepted).

Treating a 202 as success does not feel exactly right, since success implies you can begin using the resource (or, in your case, assume it's gone). It is possible that, after a 202, an error could still occur as well. However, a 202 is not truly an error such as, say, a 500 would be. It's almost as if we want a response that distinguishes success from state (that is, the request was successfully made, it's completion state is unknown). Such a distinction does not naturally fit within either the response or error object. Hmmmm...

[axw]

@brendandixon Yes, understood. I think it might be most useful to return an error indicating that the operation has taken too long, and the sender is giving up waiting. The last response is probably not that interesting, since it's always going to be 202 (I think?)

I don't think it's OK to assume success on acceptance unless it's guaranteed that that implies it will succeed. After previous email correspondences, it was concluded that this is at least not true for resource group deletion.

[brendandixon]

@axw I'm reviewing our open issues and took another look at this. It appears that, in all cases, the http.Response is returned. If the code directly invokes the generated xyzSender, the http.Response is returned directly. If the code invokes the helper-wrapper, the http.Response is either returned directly or as an embedded member of the returned "result." So, in all cases, I believe the calling code has sufficient information to make a good decision. I don't see that there's more to do here...Ok?

[axw]

@brendandixon OK. I think it's going to be a pain to wrap each and every call to check for 202 and convert to a timeout error, but perhaps others will want to react differently. I'll close this, thanks.