Failure to send full traceback

[rexes-aka]

Hello! First of all, I like your project.

In Django, we know that, by default, server error (HTTP 500) triggers server to send an email about exception to the admins. But it seems that this package seems to erase tracebacks of such exception.

Traceback (most recent call last):
  None

Django==4.2.3 drf-standardized-errors==0.12.5

[ghazi-git]

Thank you for reporting the issue, I'll see if I can reproduce it on my end. Still, it would be really helpful if you can provide more details and/or sample code to reproduce it: what exception was raised (in case you had another way of knowing what exception happened), if you're customizing the exception handler, your logging configuration since sending an email to the admin is done in a logging handler.

In the meantime, you can create a custom exception handler to manually send the email, the code should be sth like this

from drf_standardized_errors.handler import ExceptionHandler

class CustomExceptionHandler(ExceptionHandler):
    def report_exception(self, exc: exceptions.APIException, response: Response) -> None:
        super().report_exception(exc, response)
        # code for sending an email with the exception data. Here's how django does it as an example
        # https://github.com/django/django/blob/dff965798ea1bafc130a2e86530146e84607014f/django/utils/log.py#L122-L128

Then, update the setting to point to the custom exception handler class

DRF_STANDARDIZED_ERRORS = {"EXCEPTION_HANDLER_CLASS": "path.to.CustomExceptionHandler"}

[kevalrajpalknight]

Hey! I am junior developer, Looking forward to work on the Open Source Project. I see this as good opportunity. It would be great if you can allow me to contribute to this project and guide me how to make open source contribution. I really appreciate your response @rexes-aka .

[ghazi-git]

Thank you @kevalrajpalknight for offering to help.

For this issue, it would be nice to get more info from rexes-aka, but even without that, here are few pointers on how to help:

• First, reproducing the error:

  • you can use for that an existing project you have or create a small drf API project (the one from the quickstart tutorial should be good enough)
  • next, integrate this package with the project (you can follow the docs to do that)
  • then, configure django logging. Since this issue is about the traceback missing from the error email, make sure you have the mail_admins handler. Here's a link to the default django logging conf as an example but remove the require_debug_false filter since you'll probably have DEBUG=True locally.
  • Also, using the console backend for email rather than sending an actual email should do the trick
  • last, create an API view that raises an exception when you make a request to it

• Assuming you did reproduce the issue:

  • checkout the development guide for the project to set up a local dev environment
  • if you want to understand the existing code, check this part of the docs, and you can also read the source code. Reading the contents of handler.py should be enough to give you a very good idea about what the package is doing (you can ignore the openapi stuff). Start with drf_standardized_errors.handler.exception_handler.
  • add a test that reproduces the error
  • fix the issue. Most likely, the fix should go under drf_standardized_errors.handler.ExceptionHandler.report_exception
  • add an entry to docs/changelog.md about the fix

[kevalrajpalknight]

Thanks @ghazi-git that's really helpful. I'm excited to contribute to the project and I look forward to work on this issue. Please let me know if there are any specific guidelines or instructions to follow other than mentioned above.

[kevalrajpalknight]

Just a quick update, I was able to replicate the same issue.

I try to look into this issue. Once again thanks @ghazi-git

[kevalrajpalknight]

Hey @ghazi-git, I have identified the source of the problem

# handler.py
    def convert_unhandled_exceptions(self, exc: Exception) -> exceptions.APIException:
        """
        Any non-DRF unhandled exception is converted to an APIException which
        has a 500 status code.
        """
        if not isinstance(exc, exceptions.APIException):
            return exceptions.APIException(detail=str(exc))
        else:
            return exc

# rest_framework/exceptions.py
class APIException(Exception):
    """
    Base class for REST framework exceptions.
    Subclasses should provide `.status_code` and `.default_detail` properties.
    """
    status_code = status.HTTP_500_INTERNAL_SERVER_ERROR
    default_detail = _('A server error occurred.')
    default_code = 'error'

    def __init__(self, detail=None, code=None):
        if detail is None:
            detail = self.default_detail
        if code is None:
            code = self.default_code

        self.detail = _get_error_details(detail, code)

Please look into above mentioned code. APIException class does not provide .__traceback__ so if I try to call traceback.print_tr(exceptions.APIException(detail=str(exc))) It prints None where else it give proper traceback for the error on traceback.print_tr(exc). We require to make change in the convert_unhandled_exceptions method without effecting the other functionality.

[ghazi-git]

Great work, @kevalrajpalknight 👏👏 seems like you've already found the root cause

if not isinstance(exc, exceptions.APIException):
    return exceptions.APIException(detail=str(exc))

So, we need to find a way that creates the APIException and also preserves the traceback info (maybe python has sth similar to raise NewException from OldException without actually raising the exception).

You're very close to fixing the issue. Thanks for taking the time to debug the issue and sorry I couldn't get back to you sooner.

[ghazi-git]

just realized that the issue is in the exception that gets reported. In django 4.0 and below, the original exception is reported (returned by sys,get_info()) but in django 4.1 and newer, the drf exception (the one that has no traceback) is the one reported. So, we need to report the original exception in django 4.1 and newer.

Would you like to open a PR with the fix?

[kevalrajpalknight]

Hey @ghazi-git ,

Great catch! Your suggestion worked perfectly, and now I can see the traceback in the email. Your insight into the issue with Django 4.1 and newer versions was spot on.

I've made the changes you recommended to the report_exception method. Now, instead of using sys.exc_info() directly, I've captured the original exception info in the variable original_exc before any other actions are taken. This ensures that the original exception's traceback is reported in Django 4.1 and newer versions. Additionally, I've deleted the original_exc at the end to avoid any potential reference cycles.

Here's the updated snippet:

def report_exception(
    self, exc: exceptions.APIException, response: Response
) -> None:

    if is_server_error(exc.status_code):
        try:
            drf_request: Request = self.context["request"]
            request = drf_request._request
        except AttributeError:
            request = None
        signals.got_request_exception.send(sender=None, request=request)

        # Capture the original exception info
+       original_exc = sys.exc_info()

        if django.VERSION < (4, 1):
            log_response(
                "%s: %s",
                exc.detail,
                getattr(request, "path", ""),
                response=response,
                request=request,
+             exc_info=original_exc,
-              exc_info=sys.exc_info(),
            )
        else:
            log_response(
                "%s: %s",
                exc.detail,
                getattr(request, "path", ""),
                response=response,
                request=request,
+                 exception=original_exc[1],
-                  exception=exc,
            )

        # Clear the original exception info to avoid reference cycles
        del original_exc

I really appreciate your help on this. Now, the only thing left on my to-do list is creating my first test case. I'll be diving into the various test cases you've created for reference.

Thanks again, and let me know if you have any more insights or suggestions!

Cheers! 😄

[kevalrajpalknight]

Hey, @ghazi-git Sorry for taking so long to prepare suitable test case for this PR. Please I would really appreciate your help on this issue and I am completely okay with reverting this PR. Do let me know your thoughts about this.