refactor(auth): clean signIn state after all relevant async works finish

[zhouzh1]

Description of changes

• Moved all the cleanActiveSignInState method invocations to after the cacheCognitoTokens method invocations, so that we can ensure the signIn state is only cleaned when all the tasks which're possible to throw error have been successfully finished, otherwise it would cause the relevant APIs isn't able to be retried on errors (e.g. network error).

Issue #, if available

Description of how you validated changes

Checklist

• PR description included
• yarn test passes

Checklist for repo maintainers

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

[zhouzh1]

@cshfang @HuiSF @pranavosu Could you help take a look if this PR makes sense?

[jjarvisp]

Hi @zhouzh1, thanks for the pull request! These changes look reasonable; will review internally with the team and follow up if we need anything else.

[jjarvisp]

Hey @zhouzh1, are you able to clean up the linting errors so we can get the unit tests running? You should just need to run yarn lint:fix --scope @aws-amplify/auth

[jjarvisp]

Sample integration test run: https://github.com/aws-amplify/amplify-js/actions/runs/12206362394

[zhouzh1]

@jjarvisp I tried the yarn lint:fix --scope @aws-amplify/auth command, but seems that all the displayed linting errors aren't related to my change (my change is simple, I guess it wouldn't introduce linting error), they're all relevant with the failed module path resolving.

[zhouzh1]

@jjarvisp @HuiSF One more question, I am thinking that is the cleanActiveSignInState invocation in the catch clause necessary? Because it will cause the associated API not able to be retried as well, and also, I saw that there is no such a invocation in the confirmSignIn API's corresponding catch clause, not sure if this kind of inconsistence is intentional or not. Please help confirm that, thanks.

[jjarvisp]

Hey @zhouzh1, this is intentional. In the case above, omitting the cleanActiveSignInState with confirmSignIn allows users to manually retry the api without losing context of the current sign in session - for example, submitting an OTP code that may have been entered incorrectly initially. Conversely, during the signIn operation there is no context that needs to be persisted. Each sign in operation should be independent and all required dependencies are available from the input or configuration files.

If you're running into any more challenges, feel free to open an issue and we can discuss there!