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

Include Django middleware in better-exceptions #108

Closed
sumanthratna opened this issue Dec 21, 2020 · 3 comments · Fixed by #109
Closed

Include Django middleware in better-exceptions #108

sumanthratna opened this issue Dec 21, 2020 · 3 comments · Fixed by #109

Comments

@sumanthratna
Copy link
Contributor

sumanthratna commented Dec 21, 2020

Rationale

this would remove a lot of redundant code across Django projects

Proposed API

# better_exceptions/integrations/django.py (this repo)

from sys import exc_info
from better_exceptions import excepthook


class BetterExceptionsMiddleware:
    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        return self.get_response(request)

    def process_exception(self, _, exception):
        _, _, traceback = exc_info()
        excepthook(exception.__class__, exception, traceback)
# settings.py (user's Django project)

# ...
INSTALLED_APPS = [
    # ...
    "better_exceptions",
]
# ...
MIDDLEWARE = [
    # ...
    "better_exceptions.integrations.django.BetterExceptionsMiddleware",
]
# ...

This should be really easy to implement but I want to make sure you're (@Qix-) okay with maintaining this code.

@Qix-
Copy link
Owner

Qix- commented Dec 21, 2020

I haven't touched Django in a long time; how does Django pick this up? Does this interfere with any of the existing code?

If not, then a PR would be welcome :)

@sumanthratna
Copy link
Contributor Author

sumanthratna commented Dec 22, 2020

I haven't touched Django in a long time; how does Django pick this up? Does this interfere with any of the existing code?

If not, then a PR would be welcome :)

To my knowledge, this should work pretty smoothly. re: existing code, I don't think so—the two code blocks I have in the original post should be all that's needed to get this feature ready to go.

I'll create a PR for this. @Qix- should I also create a unit test? We'll need to add Django as a CI dependency if so

@Qix-
Copy link
Owner

Qix- commented Dec 23, 2020

@sumanthratna normally Id say yes but in this case no, its OK.

@Qix- Qix- closed this as completed in #109 Dec 23, 2020
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 a pull request may close this issue.

2 participants