aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.42k stars 2.12k forks source link

When calling confirmsignin without calling signin, we get a JS error. #13592

Open mattiLeBlanc opened 2 months ago

mattiLeBlanc commented 2 months ago

Is this related to a new or existing framework?

Angular

Is this related to a new or existing API?

Authentication

Is this related to another service?

No response

Describe the feature you'd like to request

When I call confirmSignin I get an error like this in my console:

setPassword.component.ts:107 ERROR SignInException: 
            An error occurred during the sign in process. 

            This most likely occurred due to:
            1. signIn was not called before confirmSignIn.
            2. signIn threw an exception.
            3. page was refreshed during the sign in flow.

    at 

Describe the solution you'd like

Is it possible to send a nice error that I dont have to catch with a try/catch? Just an error response from the call, instead of throwing an error onto the stack?

Describe alternatives you've considered

I have to use a catch:


  setNewPassword(password: string) {
    return from(confirmSignIn({
      challengeResponse: password,
    })).pipe(

     map(({ isSignedIn, nextStep}) => {
      if (isSignedIn && nextStep.signInStep === 'DONE') {
        return {
          status: 'DONE'
        };
      } else {
        console.log(isSignedIn, nextStep);
        return {
          status: 'ERROR'
        };
      }
     }),
     catchError(error => {
      console.log('error');
      return of({ status: 'ERROR'})
    }),
    )
  }

Or is this the proper way?

Additional context

No response

Is this something that you'd be interested in working on?

cwomack commented 2 months ago

Hello, @mattiLeBlanc 👋. It looks like you're requesting that we implement a more user friendly error (i.e. end user of the app that you're developing), rather than an error that's catered to being thrown in development. Is this correct?

If so, we design these error messages to be more helpful in the development process with Amplify to create an efficient and easy process for making your app. You could always implement additional frontend logic to display messages, modals, etc. when these errors are experienced and word them however you'd like. Let us know if this helps clear things up or if there's additional questions. Thanks!

mattiLeBlanc commented 2 months ago

Yes, that is right. Normally you get status 200 output with next step or anything that requires our attention, but when you call setPassword without the signin step I would have to do this:

setNewPassword(password: string) {
    return from(confirmSignIn({
      challengeResponse: password,
    })).pipe(

     map(({ isSignedIn, nextStep}) => {
      if (isSignedIn && nextStep.signInStep === 'DONE')  {
        return {
          status: 'DONE'
        };
      } else {
        console.log(isSignedIn, nextStep);
        return {
          status: 'ERROR'
        };
      }
     }),
     catchError(error => {
      return of({ status: 'FAILURE'})
    }),
    )
  }

which I guess is not the end of the world. The catchError will now catch this error thrown by the Amplify Authmodule. But maybe, an improvement from confirmSignIn could just be a response with the next step be a failure, or an error response so that it is not thrown directly onto the stack?