Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK LRO Poller: Rest the result.HttpResponse.Body before returning #1124

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

magodo
Copy link
Contributor

@magodo magodo commented Nov 26, 2024

The longRunningOperationPoller.Poll can return different poller errors, including:

  • pollers.PollingFailedError
  • pollers.PollingCancelledError

These two errors contain a HttpResponse field and a Message field. As a consumer of this error type, one will likely read the body from the HttpResponse, e.g., Azure Private Endpoint can return the following LRO responses:

{
  "status": "Failed",
  "error": {
    "code": "RetryableError",
    "message": "A retryable error occurred.",
    "details": [
      {
        "code": "ReferencedResourceNotProvisioned",
        "message": "Cannot proceed with operation because resource /subscriptions/xxxx/resourceGroups/acctest-mgd-petestt4/providers/Microsoft.Network/virtualNetworks/virtnetnamex/subnets/ssub3 used by resource /subscriptions/xxxx/resourceGroups/acctest-mgd-petestt4/providers/Microsoft.Network/networkInterfaces/example-endpoint-in-ssub3.nic.b1ecd3e7-fbcb-4e40-960b-5e7937dd8b66 is not in Succeeded state. Resource is in Updating state and the last operation that updated/is updating the resource is PutSubnetOperation."
      }
    ]
  }
}
{
  "status": "Failed",
  "error": {
    "code": "StorageAccountOperationInProgress",
    "message": "Call to Microsoft.Storage/storageAccounts failed. Error message: An operation is currently performing on this storage account that requires exclusive access.",
    "details": []
  }
}

Both are retryable, though it is not a good idea to embed this retry logic in the SDK itself. It'd be better to do it per resource type. Therefore, accessing the latest response of the LRO, and inspect the error code shall be done.

Currently, the HttpResponse is initially reset right after directly read in this function:

var respBody []byte
respBody, err = io.ReadAll(result.HttpResponse.Body)
if err != nil {
err = fmt.Errorf("parsing response body: %+v", err)
return
}
result.HttpResponse.Body.Close()
result.HttpResponse.Body = io.NopCloser(bytes.NewReader(respBody))

While it is later consumed indirectly by the call below:

lroError, parseError := parseErrorFromApiResponse(*result.HttpResponse.Response)
if parseError != nil {
return nil, parseError
}

This PR resets the response back right after this function call.

Aside: It might be tempted to use the LatestResponse() method from the poller to get the latest response body. Unfortunately, the current implementation of PollUntillDone will set it to nil on error:

if p.latestError != nil {
p.latestResponse = nil
}

Aside2: The CreateOrUpdateThenPoll method currently generated is not wrapping the poll error:

if err := result.Poller.PollUntilDone(ctx); err != nil {
return fmt.Errorf("polling after CreateOrUpdate: %+v", err)
}

This makes users have no way to cast the error to any of the poller errors. The only workaround is to split the call to CreateOrUpdate + PollUntillDone.

Relating to hashicorp/terraform-provider-azurerm#21293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-once-merged The SDK should be released once this PR is merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant