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

bugfix: Use the intended 4XX errors in flask #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tomjelen
Copy link

For flask, the intended 4XX codes defined in the exceptions
were not utilized correctly. This resulted in generic 500 errors
being returned to the client on common client-side auth errors.

Purpose

Return correct HTTP-status codes on expected client-side errors.

Does this introduce a breaking change?

[ ] Yes
[x ] No

Pull Request Type

What kind of change does this Pull Request introduce?

[x ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • Get the code
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install
  • Get a sign-in/signup page running from a flask server.
  • Navigate to signup
  • Time out your server session, or mess up your client-side auth state in one way or another.
  • Click cancel in the sign-up form
  • flask server log and HTTP-response is now a normally handled 4xx status code, instead of an internal server error.

What to Check

Verify that the following are valid

  • I decided to use 401 as a generic status code in the AuthError class. Within the current implementation, I couldn't see something more appropriate.

Other Information

None

For flask, the intended 4XX codes defined in the exceptions
were not utilized correctly. This resulted in generic 500 errors
being returned to the client on common client side auth-errors.
@ghost
Copy link

ghost commented Jun 28, 2021

CLA assistant check
All CLA requirements met.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant