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

🔥 Remove custom exception handling #151

Merged
merged 2 commits into from
Feb 6, 2024

Conversation

connormaglynn
Copy link
Contributor

@connormaglynn connormaglynn commented Feb 5, 2024

👀 Purpose

♻️ What's changed

  • Removed custom exception handling which rendered a template, displaying GitHub Exceptions (which could potentially leak sensitive information)
  • Removed unused template page
  • Remove unused /error route
  • Add type annotations to the error handlers to show that they will receive the full exception - which may contain sensitive information

📝 Notes

  • It's better to favour generic handling over custom handling for the start of the application
    • if we run into exceptions, we should try to handle them for the client i.e. retry the request.
    • If we run into a situation where an exception is thrown and it'd be useful to provide context, we should curate content for users instead of displaying the exception. For example, if we run out of seats so the invitation can't be sent we may want to inform the user that "Unfortunately, the organisation is currently full - please raise your request manually or try again later"
  • In regards to debugging these scenarios when users experience them - all exceptions will be recorded in Sentry where only Operations Engineering team have access and contain a full stack trace of the exception

Copy link
Contributor

github-actions bot commented Feb 5, 2024

🦙 MegaLinter status: ❌ ERROR

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 4 0 0 0.73s
✅ PYTHON flake8 4 0 0.43s
✅ PYTHON isort 4 2 0 0.17s
❌ PYTHON pylint 4 2 5.89s
✅ REPOSITORY gitleaks yes no 0.44s
✅ REPOSITORY trivy yes no 4.69s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@connormaglynn connormaglynn changed the title 🔥 Remove custom exception page 🔥 Remove custom exception handling Feb 5, 2024
@connormaglynn connormaglynn self-assigned this Feb 6, 2024
@connormaglynn connormaglynn merged commit e242d38 into main Feb 6, 2024
3 of 4 checks passed
@connormaglynn connormaglynn deleted the remove-custom-exception-page branch February 6, 2024 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants