fix(util-retry): use different retry token instances per request

[kuhe]

Issue

#4754

Description

Use different instances of the retry token when new requests are made from the same client. They will however share the token availability if coming from the same client and RetryStrategy instance.

Currently, because the retry token object and its closure persist between calls of the same client using the same StandardRetryStrategy instance, the retry count stays at its max value.

Testing

import { S3 } from "@aws-sdk/client-s3";
import { HttpResponse } from "@aws-sdk/protocol-http";
import { ConfiguredRetryStrategy } from "@aws-sdk/util-retry";

class MockRequestHandler {
  // Returns 429 error to cause a retry in the client.
  // No network request is made.
  async handle() {
    console.log("    Request (mock) responding with 429 throttling error.");
    return {
      response: new HttpResponse({
        statusCode: 429,
        body: Buffer.from(""),
      }),
    };
  }
}

let n = 0;
console.log("Time:", n++);
const i = setInterval(() => {
  console.log("Time:", n++); // print a time marker to the console to show time between retries.
}, 1000);

// initialize client with customized retry strategy and mock request handler.
const s3 = new S3({
  retryStrategy: new ConfiguredRetryStrategy(3, (_: number) => _ * 1000),
  requestHandler: new MockRequestHandler(),
});

const go = () =>
  s3
    .listBuckets({})
    .then((r) => console.log(r.$metadata))
    .catch((r) => console.error(r.$metadata))
    .then(async () => {
      console.log("Retries exhausted.");
    });

Promise.all([go(), go(), go()]).then(() => clearInterval(i));


// with the bug, multiple requests from the same instance will exhaust each others' retry counts

[trivikr]

In the offline discussion, we decided to deprecate the following functions from StandardRetryToken

• hasRetryTokens
• getRetryTokenCount
• releaseRetryTokens

Although getLastRetryCost should be renamed to getRetryCost as RetryToken is immutable, we decided to keep it.

They have access to availableCapacity, and can modify it.
Anything which modifies availableCapacity should be in retryStrategy.

[trivikr]

Verified the fix by printing capacityAmount with the following code which calls two operations for two clients.

import {
  S3Client,
  ListBucketsCommand,
  ListObjectsCommand,
} from "../aws-sdk-js-v3/clients/client-s3/dist-cjs/index.js";
import {
  DynamoDBClient,
  ListTablesCommand,
  ListBackupsCommand,
} from "../aws-sdk-js-v3/clients/client-dynamodb/dist-cjs/index.js";
import { NodeHttpHandler } from "../aws-sdk-js-v3/packages/node-http-handler/dist-cjs/index.js";
import { ConfiguredRetryStrategy } from "../aws-sdk-js-v3/packages/util-retry/dist-cjs/index.js";

// Simulate a timeout error.
class NodeHttpHandlerReturnsTimeout extends NodeHttpHandler {
  async handle(request, options) {
    const timeoutError = new Error("Request timed out");
    timeoutError.name = "TimeoutError";
    throw timeoutError;
  }
}

const s3Client = new S3Client({
  region: "us-west-2",
  requestHandler: new NodeHttpHandlerReturnsTimeout(),
  retryStrategy: new ConfiguredRetryStrategy(2, () => 100),
});

const dynamodbClient = new DynamoDBClient({
  region: "us-west-2",
  requestHandler: new NodeHttpHandlerReturnsTimeout(),
  retryStrategy: new ConfiguredRetryStrategy(3, () => 50),
});

const calls = {};

const callRequest = async (client, command, operation) => {
  if (!calls[operation]) {
    calls[operation] = 0;
  }

  while (true) {
    try {
      await client.send(command);
    } catch (error) {
      const metadata = error.$metadata;
      calls[operation] = calls[operation] + 1;
      console.log(
        `[${operation}]: Call #${calls[operation]}, Attempts: ${metadata.attempts}, Retry delay: ${metadata.totalRetryDelay}ms`
      );

      if (calls[operation] === 3) {
        break;
      }
    }
  }
};

await Promise.all([
  callRequest(s3Client, new ListBucketsCommand({}), "s3:listBuckets"),
  callRequest(
    s3Client,
    new ListObjectsCommand({ Bucket: "foo" }),
    "s3:listObjects"
  ),
  callRequest(dynamodbClient, new ListTablesCommand({}), "ddb:listTables"),
  callRequest(dynamodbClient, new ListBackupsCommand({}), "ddb:listBackups"),
]);

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.