Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultRateLimiter is never enabled due to type confusion #1328

Open
diego-santacruz opened this issue Jul 3, 2024 · 0 comments
Open

DefaultRateLimiter is never enabled due to type confusion #1328

diego-santacruz opened this issue Jul 3, 2024 · 0 comments

Comments

@diego-santacruz
Copy link

While experimenting with the AdaptiveRetryStrategy in @smithy/util-retry with Cognito's ListUsers API (which has a very low quota) I discovered that the updateClientSendingRate() method of DefaultRateLimiter never enables the rate limiter because the test to check if the response is a throttling error always returns false.

The issue is that updateClientSendingRate() considers that response argument is the SdkError and directly calls isThrottlingError(response), but AdaptiveRetryStrategy in @smithy/util-retry calls it with either a RetryErrorInfo or an empty object, and the RetryErrorInfo object has the SdkError in its error property. But actually there would be no need to call isThrottlingError(response) since the errorType property of RetryErrorInfo already has the necessary information.

However, the deprecated AdaptiveRetryStrategy in @smithy/middleware-retry seems to call updateClientSendingRate() with another object type, but I am not sure which one it is.

The following test program demonstrates the problem (replace the POOL_ID by a valid Cognito pool ID). with the FIX LINE commented out the requests are throttled but the state of the retry strategy at the end of the tests show that the rate limiter is not enabled, so the bucket capacity is never considered. Unccommenting the FIX LINE fixes the problem by overriding the argument passed to updateClientSendingRate() to be the SdkError object.

const { exit } = require('node:process');
const { CognitoIdentityProviderClient, ListUsersCommand } = require('@aws-sdk/client-cognito-identity-provider');
const { RETRY_MODES, AdaptiveRetryStrategy, DefaultRateLimiter } = require('@smithy/util-retry');

class MyRateLimiter extends DefaultRateLimiter {
  async getSendToken() {
    if (this.enabled) {
      console.log('RATE LIMITING');
    }
    return super.getSendToken();
  }
  updateClientSendingRate(response) {
    // -------------- FIX LINE: commenting the line below fixes the issue.
    // response = response?.error ? response.error : response;
    return super.updateClientSendingRate(response);
  }
}
const rateLimiter = new MyRateLimiter();
const retryStrategy = new AdaptiveRetryStrategy(3, {
  rateLimiter
});

const client = new CognitoIdentityProviderClient({
  region: 'eu-central-1',
  retryStrategy
 })

async function test () {
  console.log('INITIAL RETRY STRATEGY', await client.config.retryStrategy());
  const queries = new Array(70).fill().map(async () => {
    // await new Promise((resolve) => setTimeout(resolve, 5));
    return client.send(new ListUsersCommand({
      UserPoolId: 'POOL_ID',
      Limit: 1
    }));
  })
  const results = await Promise.allSettled(queries);
  let numFailed = 0;
  for (let i in results) {
    if (results[i].value) {
      console.log(`SUCCESS ${i}, attempts ${results[i].value.$metadata.attempts} totalRetryDelay ${results[i].value.$metadata.totalRetryDelay}`)
    } else if (results[i].reason) {
      numFailed++;
      console.log(`FAILED ${i}`, results[i].reason);
    }
  }
  console.log('END RETRY STRATEGY', await client.config.retryStrategy());
  console.log(`FAILED % ${numFailed / results.length * 100}`);
}

async function run () {
  console.log('---- PASS 1');
  await test();
  console.log('---- PASS 2');
  await test();
}

run().catch((e) => {
  console.error('Failed\n', e);
  exit(1)
})
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant