Aws-sdk-android: Cognito sign in with unconfirmed user throws different exception now

[markorakita]

Describe the bug
Last time I was testing my authentication flow, when you try to sign in with user who didn't finish sign up verification step, you would get UserNotConfirmedException. In latest SDK, you get CognitoInternalErrorException, with its cause exception being UserNotConfirmedException.

I would say that is a breaking change. It would be nice if we could be somehow informed of such changes. Here is a nice example how Facebook does it: https://github.com/facebook/facebook-android-sdk/blob/master/CHANGELOG.md

To Reproduce

• Sign up on Cognito (_AWSMobileClient.getInstance().signUp()_)
• Don't confirm sign up
• Try to sign in (_AWSMobileClient.getInstance().signIn()_)

Which AWS service(s) are affected?
AWS Cognito

Expected behavior
Current behavior is okay, but I would expect to have some way to find out about such breaking changes without manually testing each SDK update.

Environment Information (please complete the following information):

• AWS Android SDK Version: 2.16.9

[jamesonwilliams]

Hm, AWSMobileClient ought not to emit a Cognito exception, lest an internal one, I don't think. Sounds like _unintended_ regression, not an intentional breaking change.

As a separate matter, I agree that there is a good degree of opportunity for us to improve the changelog. This is a current topic of discussion among the internal team.

[markorakita]

Great to hear that, thanks Jameson!

I suspected it might be intentional because I already encountered similar issue in #713

[TrekSoft]

Note that on another ticket someone mentioned that, "I think what you're describing is not a result of using the latest SDK but a result of enabling legacy "Prevent User Existence Errors. https://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-pool-managing-errors.html"

[markorakita]

@TrekSoft Why have you closed the thread? Have you actually verified that this is the result of that change? Because it is not nor it should be.

If the setting is not enabled or set to LEGACY, the Cognito authentication, confirmation and password recovery operations return a UserNotFoundException when a non existing username is provided in input.

This is the only result of not enabling PreventUserExistenceErrors setting (or setting it to legacy), and it should in no way cause CognitoInternalErrorException to be thrown from the SDK.

When you close the thread like this then we don't have a chance to respond, because we can't reopen it, and since it is closed probably no one will track the responses to it anymore...

[TrekSoft]

Sorry about that - based on the your original issue, I thought you weren't requesting we change the exception back (which would now be a breaking change for people who have adapted to the new Cognito exception type) but rather requesting that when we do breaking changes like this in the future, we are clearer in reporting them.

Jameson acknowledged this and also indicated that he believed this change was unintentional so I believed the issue had been resolved and copied over that other user's suggestion as relevant material here.

Is there something further you were wanting us to address on this issue?

Also, fyi, I was unaware that users are unable to reopen issues and I do receive alerts and respond to follow up comments such as this one on issues I've closed.

[markorakita]

Thanks for explanation David, and thanks for reopening!

You are right, I wasn't requesting any change here, as you said it would be another breaking change now. I expected some of these two scenarios:
1) Change was made intentionally but wasn't properly documented. In that case I was requesting improvement of your documentation or some kind of changelog that we can track.
2) Change was made unintentionally. In this case reasonable expectation (that I didn't write into the issue description) was that you are interested in finding out what caused it, since it might affect you in some other unknown ways beside this one.

Jameson acknowledged that it was actually scenario 2), but proper investigation into what caused the change was never done. Since it is easy to handle this in client code, I was totally okay with this being low-pri issue, staying forever in the backlog.

When I saw that you closed it, I thought that you just assumed it was related to PreventUserExistenceErrors setting and called it a day :)

[TrekSoft]

Hmm, I just followed your steps to reproduce with the latest version of the SDK and am getting a UserNotConfirmed exception.

[markorakita]

Hm, I just tried it in SDK version 2.16.9 and I also get UserNotConfirmed exception. Since it is the same version of SDK as the one where I was hitting this issue, then it is probably something on the service side that they now fixed. I guess it is now exponentially harder and less important to figure out what exactly caused the change before, so if you want to close the issue then it's fine by me.