rewrite retry as promise based

[hkjpotato]

Description of changes

TL,DR

Rewrite the retry method in amazon-cognito-identity-js as promised based, to avoid the implicit dependency on the regeneratorRuntime polyfill.

Details

This is to fix #8514, a breaking change reported by the customer. The issue is caused by a recent change in amazon-cognito-identity-js, where aysnc and await syntax is used for the first time in that package.

It creates an implicit dependency on regeneratorRuntime in all its built code (umd/es/commonjs), and affect downstream packages like auth.

• amazon-cognito-identity-js was written long ago with traditional nodejs callback style + promise.
• It was written in ES6 and transpiled to ES5 by babel using @babel/preset-env, which includes babel/plugin-transform-regenerator (see discussion)
• The plugin detects async and await keyword and convert them into the regeneratorRuntime methods, assuming regeneratorRuntime is made available globally.

Customers now have to explicitly add and define regenerator-runtime in their codebase (not needed for CRA). They don't need to before. Rewriting the retry as promised base helps avoid the extra step for the customers.

---

Why other package (Auth) does not have this problem? Because they are written in typescript and get compiled by tsc first. Typescript use inline __awaiter helper to deal with async/await.

In the future, we could consider unifying the language and compile tool, to avoid polyfilling ES6 feature differently. This is out of scope of the quick fix though.

Issue #, if available

#8514

Description of how you validated changes

async/await is just syntax sugar for promise, switching the syntax does not affect how javascript gets executed in terms of the event loop (micro tasks).

profiling on retry by async/await

profiling on retry by promise

Checklist

• PR description included
• yarn test passes
• Tests are changed or added
• Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[elorzafe]

LGTM thanks @hkjpotato 🌮

[hkjpotato]

going to test it with the integration test sample

[codecov-commenter]

Codecov Report

Merging #8524 (b6ef84e) into main (10857d5) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #8524   +/-   ##
=======================================
  Coverage   77.96%   77.96%           
=======================================
  Files         237      237           
  Lines       16824    16824           
  Branches     3617     3614    -3     
=======================================
  Hits        13116    13116           
- Misses       3581     3582    +1     
+ Partials      127      126    -1     

Impacted Files	Coverage Δ
packages/amazon-cognito-identity-js/src/Client.js	52.04% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10857d5...b6ef84e. Read the comment docs.

[ericclemmons]

Thanks for doing this! It's a lesson for us to learn that differences in build tooling can introduce different behavior for the same implementation 😓

[TheDutchCoder]

I'm the customer who reported this and I think (but I'm not 100% sure), that the issue is that the transpiled version of this package is being used, instead of the native ESM version.

Maybe there's no ESM version at all (I haven't checked), but the transpiled version seems to have some external dependencies still that users have to configure in their build tool (webpack/Vite/etc).

However (like in our case) we don't cater to older browsers and depend on ESM modules all the way (and certain functionality like, in this case, async/await).

I've wondered if there's a way to have all aws internal dependencies be resolved as ESM modules when appropriate, but I would assume that's a much larger discussion?

Anyway, just wanted to shed some light on it! Thanks for taking care of this!

[hkjpotato]

will send out integ test change in another pr

[hkjpotato]

@TheDutchCoder we need to look more into it in the future but I think ESM is the module system, while async and await is an ES6 syntax defined in its spec https://tc39.es/ecma262/multipage/ecmascript-language-functions-and-classes.html#sec-async-function-definitions

[github-actions]

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.