Use of instanceof Causes Unexpected Behavior in defaultRetryDecider - Jest

[mtp1376]

Checkboxes for prior research

• I've gone through Developer Guide and API reference
• I've checked AWS Forums and StackOverflow.
• I've searched for previous similar issues and didn't find any solution.

Describe the bug

Jest has problems (+, +) with instanceof and therefore the following implementation for asSdkError will always fall back to the last line (AWS SDK error wrapper for ...) which causes all of the error.XXX properties to be removed (like error.name which will be subsequently needed).

aws-sdk-js-v3/packages/middleware-retry/src/util.ts

Lines 3 to 8 in fcfb59f

	export const asSdkError = (error: unknown): SdkError => {
	if (error instanceof Error) return error;
	if (error instanceof Object) return Object.assign(new Error(), error);
	if (typeof error === "string") return new Error(error);
	return new Error(`AWS SDK error wrapper for ${error}`);
	};

The shouldRetry function uses the defaultRetryDecider:

aws-sdk-js-v3/packages/middleware-retry/src/StandardRetryStrategy.ts

Line 101 in fcfb59f

if (this.shouldRetry(err as SdkError, attempts, maxAttempts)) {

aws-sdk-js-v3/packages/middleware-retry/src/StandardRetryStrategy.ts

Line 47 in fcfb59f

return attempts < maxAttempts && this.retryDecider(error) && this.retryQuota.hasRetryTokens(error);

Because the defaultRetryDecider is calling functions that rely on error.name which is replaced by the earlier new Error therefore they don't work as expected:

aws-sdk-js-v3/packages/middleware-retry/src/retryDecider.ts

Lines 9 to 15 in fcfb59f

	export const defaultRetryDecider = (error: SdkError) => {
	if (!error) {
	return false;
	}
	return isRetryableByTrait(error) || isClockSkewError(error) || isThrottlingError(error) || isTransientError(error);
	};

For example isTransientError is using error.name as follows:

aws-sdk-js-v3/packages/service-error-classification/src/index.ts

Lines 26 to 29 in e7b1535

	export const isTransientError = (error: SdkError) =>
	TRANSIENT_ERROR_CODES.includes(error.name) ||
	NODEJS_TIMEOUT_ERROR_CODES.includes((error as { code?: string })?.code || "") ||
	TRANSIENT_ERROR_STATUS_CODES.includes(error.$metadata?.httpStatusCode || 0);

SDK version number

latest

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v18.11.0

Reproduction Steps

Use the following code and place a breakpoint on asSdkError first line to see the error is being wrapped (not because it's not a true error, but because instanceof call fails). The test is just created to cause an error to be thrown.

import { AWS } from '../../src/services/aws_macros';

jest.setTimeout(3000000);

describe('Jest-AWS Error Issue', () => {
  it('triggers AWS error to inspect', async () => {
    // use a debugger to inspect internal sdk error object
    const client = new AWS({
      credentials: {
        accessKeyId: process.env.AWS_ACCESS_KEY_ID!,
        secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY!,
      },
      region: 'us-east-1',
    });
    const results = await Promise.all(
      [...Array(100).keys()].map(async () => await client.s3Client.listBuckets({})),
    );
  });
});

Observed Behavior

Due to the failure of instanceof to act as it should, it's falling back to new Error line and therefore causing defaultRetryDecider to have unexpected behavior.

Expected Behavior

The instanceof should be replaced by something that works throughout different environments, and the defaultRetryDecider should be working.

Possible Solution

Use of (error as {name?: ...}).name? maybe fixes the issue and the instanceof is no longer needed.

Additional Information/Context

No response

[mtp1376]

Can use something like this to wrap the original error. This way people can create workarounds easily (because the original error isn't thrown away).
https://twitter.com/housecor/status/1608795552644476928

[kuhe]

is this still a problem?

we use jest for our tests. The default retry implementation has also changed since this was reported.

[github-actions]

This issue has not received a response in 1 week. If you still think there is a problem, please leave a comment to avoid the issue from automatically closing.

[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.