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

When SDK introduces client side rate limiting in default RetryStrategy, the error is swallowed

Open trivikr opened this issue 1 year ago • 1 comments

Checkboxes for prior research

Describe the bug

When SDK introduces client side rate limiting in Retry Strategy, the error is swallowed

SDK version number

All, used v3.335.0 in testing

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

All, used v16.20.0 in testing

Reproduction Steps

Here is a repro which adds custom Handler which returns a Timeout for every call, and a retryStrategy with just 100ms delay between retries.

import { Kinesis } from "@aws-sdk/client-kinesis"; // v3.335.0
import { NodeHttp2Handler } from "@aws-sdk/node-http-handler";
import { ConfiguredRetryStrategy } from "@aws-sdk/util-retry";

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

const client = new Kinesis({
  region: "us-west-2",
  requestHandler: new NodeHttp2HandlerReturnsTimeout(),
  retryStrategy: new ConfiguredRetryStrategy(
    4, // max attempts.
    () => 100 // delay only for 100ms between retries to speed up client side rate limiting.
  ),
});

while (true) {
  try {
    await client.listStreams({});
  } catch (error) {
    if (error.$metadata.attempts === 1) {
      // SDK introduced client side rate limiting.
      throw error;
    }
  }
}

Observed Behavior

Error is thrown by the SDK with no info on why the error was thrown

file:///Users/trivikr/workspace/test/test.mjs:8
    const timeoutError = new Error("Request timed out");
                         ^

Error [TimeoutError]: Request timed out
    at NodeHttp2HandlerReturnsTimeout.handle (file:///Users/trivikr/workspace/test/test.mjs:8:26)
    at /Users/trivikr/workspace/test/node_modules/@aws-sdk/client-kinesis/dist-cjs/commands/ListStreamsCommand.js:36:58
    at /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-serde/dist-cjs/deserializerMiddleware.js:5:32
    at /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-signing/dist-cjs/middleware.js:14:26
    at async /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-retry/dist-cjs/retryMiddleware.js:27:46
    at async /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-logger/dist-cjs/loggerMiddleware.js:7:26
    at async file:///Users/trivikr/workspace/test/test.mjs:27:5 {
  '$metadata': { attempts: 1, totalRetryDelay: 0 }
}

Expected Behavior

Error thrown by the SDK with info on why the error was thrown

/Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-retry/dist-cjs/retryMiddleware.js:40
                    const errorToThrow = new Error(refreshError.message, { cause: lastError });
                                         ^

Error: No retry token available
    at /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-retry/dist-cjs/retryMiddleware.js:40:42
    at async /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-logger/dist-cjs/loggerMiddleware.js:7:26
    at async file:///Users/trivikr/workspace/test/test.mjs:27:5 {
  '$metadata': { attempts: 1, totalRetryDelay: 0 },
  [cause]: Error [TimeoutError]: Request timed out
      at NodeHttp2HandlerReturnsTimeout.handle (file:///Users/trivikr/workspace/test/test.mjs:8:26)
      at /Users/trivikr/workspace/test/node_modules/@aws-sdk/client-kinesis/dist-cjs/commands/ListStreamsCommand.js:36:58
      at /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-serde/dist-cjs/deserializerMiddleware.js:5:32
      at /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-signing/dist-cjs/middleware.js:14:26
      at async /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-retry/dist-cjs/retryMiddleware.js:27:46
      at async /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-logger/dist-cjs/loggerMiddleware.js:7:26
      at async file:///Users/trivikr/workspace/test/test.mjs:27:5
}

This error was thrown by updating the catch clause in node_modules/@aws-sdk/middleware-retry/dist-cjs/retryMiddleware.js as follows:

                catch (refreshError) {
                    const errorToThrow = new Error(refreshError.message, { cause: lastError });
                    errorToThrow.$metadata = {
                        attempts: attempts + 1,
                        totalRetryDelay,
                    };
                    throw errorToThrow;
                }

Possible Solution

Update the catch clause in retryMiddleware to not swallow the error thrown by refreshRetryTokenForRetry

Additional Information/Context

  • Source code which throws "No retry token available" error: https://github.com/aws/aws-sdk-js-v3/blob/7f8d884852a85eedc5b048afb13c89e80b94b7e1/packages/util-retry/src/StandardRetryStrategy.ts#L36
  • The refreshError is swallowed and lastError is thrown with metadata: https://github.com/aws/aws-sdk-js-v3/blob/7f8d884852a85eedc5b048afb13c89e80b94b7e1/packages/middleware-retry/src/retryMiddleware.ts#L60-L67

trivikr avatar May 24 '23 15:05 trivikr