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

Record getToken errors in the errors$ observable #104

Merged
merged 4 commits into from
Jan 15, 2021

Conversation

frederikprijck
Copy link
Member

By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

Whenever Auth0Client.getTokenSilently or Auth0Client.getTokenWithPopup throw an error, the AuthService's error$ observable will not record those errors.

This PR ensures it does.

Apart from that, this PR also reworks theAuthHttpInterceptor to use the AuthService instead of the Auth0Client directly, to ensure errors are also recorded as part of the error$ observable when using the interceptor.

References

#103

Testing

When errors are thrown as part of either Auth0Client.getTokenSilentlyor Auth0Client.getTokenWithPopup when calling AuthService.getAccessTokenSilently or AuthService.getAccessTokenWithPopup, the error$ observable should record those errors.

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not master

@frederikprijck frederikprijck requested a review from a team as a code owner January 4, 2021 09:24
@frederikprijck frederikprijck added CH: Changed PR is changing something review:small Small review labels Jan 4, 2021
Copy link
Contributor

@stevehobbsdev stevehobbsdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good. Would it also make sense to add a short section under Call an API in the readme to demonstrate how errors should be handled when calling APIs?

Even if the work in this PR doesn't affect how the user would currently handle those errors, I think it would be a worthwhile addition to the docs and makes sense to do it here.

@frederikprijck frederikprijck merged commit 7dda5e1 into master Jan 15, 2021
@frederikprijck frederikprijck deleted the frederik/fix/getTokenErrors branch January 15, 2021 10:53
@frederikprijck frederikprijck added this to the vNext milestone Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Changed PR is changing something review:small Small review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants