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

Feat: server error - 400 errors / other exceptions during /token #2117

Merged
merged 9 commits into from
Jun 4, 2024

Conversation

angela-tran
Copy link
Member

@angela-tran angela-tran commented May 22, 2024

Part of #2032

  • 400-level error or a non-HTTPError exception during /token returns a JSON response that is used to send user to /error
  • /error shows server error template
  • Sentry notification is sent
  • Analytic event is sent (failed access token request)

Reviewing / testing

To test handling of 400-level HTTP errors, you'll have to force it like in other PRs such as #2089.
To /token, add:

        try:
+          class response:
+             def __init__(self):
+                self.status_code = 400
+
+          raise HTTPError(response=response())
           response = client.request_card_tokenization_access()
        except Exception as e:

To test other exceptions:

  • setting an incorrect client_id on PaymentProcessor causes UnsupportedTokenTypeError on client.request_card_tokenization_access
  • setting an incorrect api_base_url on PaymentProcessor causes a ConnectionError on client.ensure_active_token
  • setting an incorrect client_secret_name on PaymentProcessor could cause an error when creating Client (we read from payment_processor.client_secret_name)

@angela-tran angela-tran self-assigned this May 22, 2024
@github-actions github-actions bot added back-end Django views, sessions, middleware, models, migrations etc. tests Related to automated testing (unit, UI, integration, etc.) labels May 22, 2024
@angela-tran angela-tran force-pushed the chore/system-enrollment-error--400-token branch from c184b6c to 746f322 Compare May 22, 2024 16:24
Copy link

github-actions bot commented May 22, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  benefits/core
  views.py
  benefits/enrollment
  views.py
Project Total  

This report was generated by python-coverage-comment-action

@angela-tran angela-tran changed the title Feat: system enrollment error - 400 errors or other exceptions during /token Feat: system enrollment error - 400 errors / other exceptions during /token May 22, 2024
Base automatically changed from chore/system-enrollment-error--500-token to dev May 22, 2024 21:04
@angela-tran angela-tran changed the title Feat: system enrollment error - 400 errors / other exceptions during /token Feat: server error - 400 errors / other exceptions during /token May 22, 2024
@angela-tran angela-tran mentioned this pull request May 22, 2024
13 tasks
@angela-tran angela-tran marked this pull request as ready for review May 23, 2024 16:53
@angela-tran angela-tran requested a review from a team as a code owner May 23, 2024 16:53
benefits/enrollment/views.py Outdated Show resolved Hide resolved
benefits/enrollment/templates/enrollment/index.html Outdated Show resolved Hide resolved
Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

Very minor request, otherwise this looks good!

benefits/enrollment/views.py Outdated Show resolved Hide resolved
Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

👍

@angela-tran angela-tran merged commit 7c20abd into dev Jun 4, 2024
14 checks passed
@angela-tran angela-tran deleted the chore/system-enrollment-error--400-token branch June 4, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Django views, sessions, middleware, models, migrations etc. tests Related to automated testing (unit, UI, integration, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants