arm-storage fails to handle retry headers

[aiwilliams]

• Package Name: arm-storage
• Package Version: 13.1.0
• Operating system: MacOS 10.15.3
• nodejs
  • version: 12.13.1
• browser
  • name/version:
• typescript
  • version:
• Is the bug related to documentation in
  • README.md
  • source code documentation
  • SDK API docs on https://docs.microsoft.com

Describe the bug
When a 429 response is returned from a resource list endpoint, the DeserializationPolicy will throw a RestError so the ThrottlingRetryPolicy never gets a chance to try again.

listOperationSpec only support two responses:

         {
            200: {
                bodyMapper: ListContainerItems
            },
            default: {
                bodyMapper: CloudError
            }
         }

To Reproduce
Steps to reproduce the behavior:

1. Instantiate an instance of the StorageManagementClient and fire off a couple hundred Promises that invoke client.storageAccounts.list(). AppConfigurationClient does not retry when a request is throttled #6408 provides a good example.

Expected behavior
A 429 response should be retried, not cause the DeserializationPolicy to kill the policy chain.

[dw511214992]

Create an issue for codegen team to fix this issue: Azure/autorest.typescript#597

[aiwilliams]

I've worked around this by:

new StorageManagementClient(this.auth.credentials, this.auth.subscriptionId, {
      noRetryPolicy: true, // <-- This removes retry policies so they can be added after the Deserialization
      requestPolicyFactories: (
        defaultRequestPolicyFactories: RequestPolicyFactory[],
      ): RequestPolicyFactory[] => {
        const policyChain = [
          ...defaultRequestPolicyFactories,
          exponentialRetryPolicy(), // < -- This will not retry a 429 response
          systemErrorRetryPolicy(),
          throttlingRetryPolicy(), // < -- This thing will only retry once
        ];
        return policyChain;
      },
    });

I now realize this may be a problem better reported to ms-rest-js, because the defaultRequestPolicyFactories come from the super class of StorageManagementClient -> AzureServiceClient -> ServiceClient.

FWIW, the API answers a 429 response which the ThrottlingRetryPolicy handles once with a retry, but when that also returns a 429, it doesn't try again. The API says Retry-After 17, but the request is still rejected with a 429 and another Retry-After 17. I wish the API would return a time that is more likely to work out. That is, I would have liked to see the API drive the retries, so that we could easily burst under 100 requests and throttle back based on meaningful responses from the API. Instead, I'm going to have to use p-throttle to proactively throttle and spread 100 requests out over 5 minutes.

[dw511214992]

Thank you @aiwilliams for your information. Will involve service team to check if service can return a time.

[ghost]

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

[ramya-rao-a]

This issue depends on Azure/ms-rest-js#394 being fixed

[dw511214992]

As Azure/ms-rest-js#394 fixed and the package released, please use this new feature in the latest @azure/ms-js-rest.
close it, and please open it if you have any more issues. thanks