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

[16.0] [IMP] fastapi: Factorize error handling and use it in tests with raise_server_exceptions=False #464

Merged

Conversation

paradoxxxzero
Copy link
Contributor

As of now using raise_server_exceptions=False leads to all exceptions to be returned as 500.

This PR aims to share the error handling between odoo FastAPIDispatcher and the starlette TestClient to get meaningful response in tests.

This also adds a log of 500 exceptions in testing which is very useful to get unhandled exceptions traceback while running the tests.

@OCA-git-bot
Copy link
Contributor

Hi @lmignon,
some modules you are maintaining are being modified, check this out!

Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

Thank you @paradoxxxzero for this useful change. A little comment to reduce the diff and it'll be perfect.

@paradoxxxzero paradoxxxzero force-pushed the 16.0-imp-fastapi-error-handling-in-tests branch from d214043 to ccc8c89 Compare October 16, 2024 12:29
@paradoxxxzero
Copy link
Contributor Author

Thank you @paradoxxxzero for this useful change. A little comment to reduce the diff and it'll be perfect.

Done :)

@lmignon
Copy link
Contributor

lmignon commented Oct 16, 2024

ping @AnizR

Copy link
Contributor

@AnizR AnizR left a comment

Choose a reason for hiding this comment

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

Great! Thanks for your contribution.

Code LGTM

@lmignon
Copy link
Contributor

lmignon commented Oct 16, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-464-by-lmignon-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 9d7192d into OCA:16.0 Oct 16, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at c89c938. Thanks a lot for contributing to OCA. ❤️

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

Successfully merging this pull request may close these issues.

4 participants