Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AuthenticationError not exported #700

Closed
someone1 opened this issue Jan 27, 2021 · 4 comments · Fixed by #716
Closed

AuthenticationError not exported #700

someone1 opened this issue Jan 27, 2021 · 4 comments · Fixed by #716
Labels
feature request A feature has been asked for or suggested by the community

Comments

@someone1
Copy link

Describe the problem you'd like to have solved

When Auth0 redirects back to the application with an error, we can't test for AuthenticationError

Describe the ideal solution

add AuthenticationError to RedirectLoginResult vs throwing it?

Alternatives and current work-arounds

import { AuthenticationError } from '@auth0/auth0-spa-js/src/errors';

try {
   client.handleRedirectCallback();
} catch(e) { 
 if ('error' in e) {
        const authErr = e as AuthenticationError;
          // handle appropriately - what's retryable? 
  }
}

Additional context

There's also no real examples or guidance available of handling error responses from failed authentication requests - when would it be ok to ignore the error thrown here and try to authenticate again vs giving up. Are the errors here the same as the standard error responses?

I'm here since I ended up in an endless redirect loop!

@someone1 someone1 added the feature request A feature has been asked for or suggested by the community label Jan 27, 2021
@stevehobbsdev
Copy link
Contributor

Thanks for the report @someone1. Can you elaborate on this:

add AuthenticationError to RedirectLoginResult vs throwing it?

Are you saying you'd prefer a property on RedirectLoginResult that gives you the error? I think here we would prefer to continue throwing the error but we should export the type, so you can do this:

import { AuthenticationError } from '@auth0/auth0-spa-js';

How does that sound?

There's also no real examples or guidance available of handling error responses from failed authentication requests

This is something we've already had additional feedback on and plan to address. It will probably come in the form of an additional readme in the repo that demonstrates the different error scenarios. Thanks for your additional feedback here!

@someone1
Copy link
Author

I think that'd definitely be a better way to import it, but this duck typing + casting approach is less than ideal. Even with the export I could not get the following code to work as I'd expect to be able to:

try {
   // ....
} catch(e) { 
 if (e instanceof AuthenticationError) {
          // ... 
  }
}

If the new export makes this method of error checking work then I think that'd be fine!

@stevehobbsdev
Copy link
Contributor

stevehobbsdev commented Jan 28, 2021

🤔 we do reset the prototype according to this, so I might expect that to work. Let me look into it.

Otherwise, I suppose we expected that you would simply check the error and error_message properties as strings, which all our error types do have. So instead of discriminating on the error type, you would check error:

try {
   // ....
} catch(e) { 
 if (e.error === 'login_required') {
          // ... 
  }
}

It's just we don't have concrete error types for all the different types of error you might want to handle, so you'd probably have to discriminate on that at some stage anyway.

@someone1
Copy link
Author

Have you had any chance to look into this?

Where possible, I'd rather detect known error types that are already being thrown and defined rather than fallback to the duck typing approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A feature has been asked for or suggested by the community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants