aws-sdk-js-v3 icon indicating copy to clipboard operation
aws-sdk-js-v3 copied to clipboard

Replace unused maxRetries with maxAttempts

Open trivikr opened this issue 4 years ago • 2 comments

Describe the bug

The maxRetries configuration was added in remote credentials provider back in Jun 15, 2017 in https://github.com/aws/aws-sdk-js-v3/commit/d2b251e4ca67549a23f2de13658f5f498e9121a9

The configuration maxRetries was changed to maxAttempts in middleware-retry to be compliant with other SDKs and retry strategy https://github.com/aws/aws-sdk-js-v3/pull/1244

This change wasn't done in credential providers though.

Proposed fix

  • Add new configuration maxAttempts in RemoteProviderConfig, so that it reuses the value from retry while fetching credentials.
  • Instead of deleting existing maxRetries, call it deprecated in favor of maxAttempts.

Additional context

The bug was noticed while debugging retry strategy options for RemoteProviderConfig requested in https://github.com/aws/aws-sdk-js-v3/issues/2706

trivikr avatar Aug 25 '21 16:08 trivikr

https://github.com/smithy-lang/smithy-typescript/pull/1346

andre-araujo avatar Jul 23 '24 10:07 andre-araujo

The internal utility providerConfigFromInit is called from:

  • fromContainerMetadata https://github.com/smithy-lang/smithy-typescript/blob/86862eab5e8b26520a2a3e9914d24a9ab5a98270/packages/credential-provider-imds/src/fromContainerMetadata.ts#L31
  • getInstanceMetadataProvider https://github.com/smithy-lang/smithy-typescript/blob/86862eab5e8b26520a2a3e9914d24a9ab5a98270/packages/credential-provider-imds/src/fromInstanceMetadata.ts#L37

These utilities are further called from other providers or re-exported, like:

  • remoteProvider https://github.com/aws/aws-sdk-js-v3/blob/ac98acca73a3caf30ab802be9594bcc5ad0a0a8e/packages/credential-provider-node/src/remoteProvider.ts#L24
  • resolveCredentialSource https://github.com/aws/aws-sdk-js-v3/blob/ac98acca73a3caf30ab802be9594bcc5ad0a0a8e/packages/credential-provider-ini/src/resolveCredentialSource.ts#L24-L29
  • fromInstanceMetadata: https://github.com/aws/aws-sdk-js-v3/blob/ac98acca73a3caf30ab802be9594bcc5ad0a0a8e/packages/credential-providers/src/fromInstanceMetadata.ts#L31
  • fromContainerMetadata https://github.com/aws/aws-sdk-js-v3/blob/ac98acca73a3caf30ab802be9594bcc5ad0a0a8e/packages/credential-providers/src/fromContainerMetadata.ts#L31

The ask for this issue is to remove maxRetries configuration from credential providers, as part of standardization, since we use maxAttempts in clients. It requires three things to be done:

  • Deprecate maxRetries in init used by fromContainerMetadata and getInstanceMetadataProvider in favor of maxAttempts
  • Add JSDoc that maxAttempts is recommended, and it's value if one more than maxRetries
  • In resolving the values, like in providerConfigFromInit favor value in maxAttempts over that in maxRetries, and use it for any attempts made in internal code.

trivikr avatar Jul 23 '24 16:07 trivikr