Amplify REST requests should not automatically retry on serverside errors

[severi]

Before opening, please confirm:

• I have searched for duplicate or closed issues and discussions.
• I have read the guide for submitting bug reports.
• I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.

JavaScript Framework

Not applicable

Amplify APIs

REST API

Amplify Version

v6

Amplify Categories

api

Backend

None

Environment information

# Put output below this line
not applicable

Describe the bug

Amplify should never automatically try to retry API requests if they fail on serverside errors (500, 502, 503, 504)

https://github.com/aws-amplify/amplify-js/blob/main/packages/core/src/clients/middleware/retry/defaultRetryDecider.ts#L26C4-L26C21

This behaviour is just plain wrong and should not exists OR at the very least should be possible to be disabled

Expected behavior

Api request fails once and is not retryed

Reproduction steps

server responds with error code 500, amplify post request automatically retries 2 times after the failed attempt

Code Snippet

// Put your code below this line.

Log output

// Put your logs below this line

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

[chrisbonifacio]

Hi @severi Thank you for raising this issue.
We were able to reproduce the issue.

We'll bring this to the team's attention and see how we can address it, whether it be through changing the behavior or exposing a way to disable this behavior.

[Philip-Bro]

any updates for this

[EricDAllen]

I'm picturing a badly-coded server that is starting to fail when it gets overloaded. Arbitrarily tripling the number of requests sent to it really doesn't seem helpful.

[metuuu]

I am using TanStack Query and would like to use its retrying system rather than amplify's.
Is it even possible currently to disable amplify api retrying completely?

[chris-xylem]

Checking to see if there has been an update on this bug?

[alexlilleyxylem]

I see there was a PR opened a couple weeks ago. Any updates/movement on that?

[chris-xylem]

Any update on this issue? I see a PR for this issue has been open for 3 months now: #13641

[the03ennis]

Would love an update here on PR #13641!

[AllanZhengYP]

Hi @severi @the03ennis @chris-xylem

This is a valid feature request. Currently the API category does not expose options to control the retry behavior. However the mentioned PR would bring undesired side-effect to AWS service clients that need this behavior.

We just noticed this because it was labeled with GraphQL label which does not share the same visibility.

As an workaround, if you want full control of request lifecycle, you can create your own handler without using the REST API handler. For example, in react query, you can provide your own query function. You can access the aws credentials with fetchAuthSession() then sign the request to your REST API endpoint. There are a handful of open source packages handling signing request with AWS credentials.

[chris-xylem]

This seems more like a bug then a feature request to me as this behavior was not present v5

[lehotskysamuel]

This is absolutely a bug, this kind of behaviour should not be enforced. At the very least it should be configured as a default that can be overriden by a config.

With respect, why is this taking so long? Especially given there's been an open pull request with a fix for 6 months.

[cwomack]

@chris-xylem and @lehotskysamuel, appreciate the feedback here and I'll bring this issue and associated PR in front of the team again for review.

[cwomack]

For anyone following this issue, wanted to let you all know that this issue was reviewed again and we agree that this need to be updated to a bug (rather than feature request). There is no documented change in our migration guides going from v5 to v6 that details this behavior.

As for the associated PR #13641, we'll close that out since we want to handle the implementation of this internally. Thank you, @daixi98 for opening it and attempting to resolve this.

[lehotskysamuel]

Thank you @cwomack for reviewing this and pushing this 🙏.

[cshfang]

Hello everyone, we have been looking into a way to disable this retry behavior for these APIs. One reason we did not simply merge the PR referenced above is because we felt strongly that it was important not to simply turn off the behavior across the board for all developers using Amplify V6 but instead offer a way to opt out of the current behavior.

To that end we are proposing to add a retryStrategy property to the APIs in question which will

1. Allow for configuring retry behavior granularly (per API call)
2. Allow for flexibility to extend the retry model as it becomes desired/required

The change proposed will look something like the following:

import { get } from 'aws-amplify/api';

get({
  apiName: 'myRestApi',
  path: 'path',
  retryStrategy: { strategy 'no-retry' }
});

Where the retryStrategy type would look like the following:

export type RetryStrategy =
  | {
      // Default strategy and current behavior in V6.
      // No actions will be required from developers who prefer this retry model.
      strategy: 'default';
    }
  | {
      // Disables retries.
      strategy: 'no-retry';
    }

Tagging folks who have participated in this thread to see if this solution would be viable to move forward with. Thank you all!
@severi @alexlilleyxylem @the03ennis @chris-xylem @Philip-Bro @EricDAllen @metuuu @lehotskysamuel

[tonyawad88]

Folks, the default strategy should be set to 'no-retry'. I understand why you want to maintain the same behavior and set the 'default' strategy to be as is, but the "as is" behavior is not supposed to be this way and is a bug in my opinion. Those who need the retry automatically, should opt-in and modify the strategy. Otherwise, by default, no retries and no need to override the strategy to get the desired behavior.

[EricDAllen]

While I agree this should be categorized as a bug and the default behavior needs to change, I also understand the Amplify team's concerns about backwards compatibility for existing users. Just fixing this bug in v6 is probably too risky for what it would do to systems in the wild.

I would suggest that you roll ahead with the plan @cshfang has outlined above, but I'd also strongly suggest that you plan to change the default behavior in your next version. The decision to generically retry failed calls like this should never be made by the underlying framework. Kudos for wanting to make things easier for people, but this misguided feature never should have been built in the first place, and you should deprecate it as soon as it is safe to do so.

[lehotskysamuel]

I agree with both comments above. Personally I'd prefer the no retry to be the default but I understand backwards compatibility issues, so Eric's suggestion to deprecate and remove in the next version bump is a good middle ground.

I have one feedback about the suggested API - why make RetryStrategy an object? I'd prefer a string or enum.
Instead of retryStrategy: { strategy: 'no-retry' }
I suggest retryStrategy: RetryStrategy.NO_RETRY
or retryStrategy: 'no-retry'

Maybe that was your plan and it was just pseudo code, then ignore my comment :) .

[chris-xylem]

As others have said, I agree that the default should be 'no-retry' but understand the need to not disrupt any users that are leveraging this bug in their favor.

But all in all, I like the suggested fix and appreciate the work being done to resolve this issue.

[cshfang]

@lehotskysamuel

I agree with both comments above. Personally I'd prefer the no retry to be the default but I understand backwards compatibility issues, so Eric's suggestion to deprecate and remove in the next version bump is a good middle ground.

I have one feedback about the suggested API - why make RetryStrategy an object? I'd prefer a string or enum. Instead of retryStrategy: { strategy: 'no-retry' } I suggest retryStrategy: RetryStrategy.NO_RETRY or retryStrategy: 'no-retry'

Maybe that was your plan and it was just pseudo code, then ignore my comment :) .

Thank you for this thoughtful feedback. The rationale for making this an object stems from a bit of ambition about how we can further extend this. Imagine, if you will, some kind of future state where it is possible to have other kinds of retry strategies which exposed various other configurations alongside the strategy. For example, perhaps being able to configure the number of retries a la

retryStrategy: {
  strategy: 'default',
  retryAttempts: 30 // not recommended 😅 
}

As many of you have mentioned, for now, we want to make sure to address the immediate need of disabling retries on these REST APIs altogether and we will revisit this and consider flipping the default behavior in a major version bump.