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

Debugging errors in the backend #86

Closed
jordaneremieff opened this issue Jul 3, 2021 · 12 comments · Fixed by #88
Closed

Debugging errors in the backend #86

jordaneremieff opened this issue Jul 3, 2021 · 12 comments · Fixed by #88

Comments

@jordaneremieff
Copy link

Hello, firstly I want to say thank you because I find this project very useful! I was finding it difficult to debug some errors in a project that I'm working on, so I subclassed the middleware to return errors to the frontend like this:

import traceback

from django.conf import settings
from django.http import HttpResponse


from django_htmx.middleware import HtmxMiddleware


class HtmxDebugMiddleware(HtmxMiddleware):
    def process_exception(self, request, exception):
        if request.htmx and settings.DEBUG:
            content = (
                "<h1>Django HTMX Error</h1><b>%s</b>"
                "<h3>Traceback</h3><textarea rows=10>%s</textarea>"
            ) % (
                exception,
                traceback.format_exc(),
            )

            return HttpResponse(content, status=200)

        return None

Here is an example of what the solution above produces when raise RuntimeError("Something bad happened on the backend!") is encountered:

Screen Shot 2021-07-04 at 2 15 57 am

Would it be desirable to include something similar in this project?

@adamchainz
Copy link
Owner

Why prefer this over django's default debug page?

@jordaneremieff
Copy link
Author

jordaneremieff commented Jul 3, 2021

@adamchainz the default behavior is that the Javascript console will display an ambiguous error with little information related to the request from the backend.

@jordaneremieff
Copy link
Author

For example, the same view in my initial comment would produce only this with the default behavior:

Screen Shot 2021-07-04 at 3 27 45 am

The area where the expected content would appear is blank. My adjustment is to expose debug information where the element is expected to appear.

@adamchainz
Copy link
Owner

Ah right, because the response code is 500 htmx doesn't insert it. There might be some way we can adjust django's debug view or htmx to work rather than reimplement it ourselves - that would be preferable?

@jordaneremieff
Copy link
Author

I think it will not be possible to repurpose Django's debug view in the case of partial rendering multiple elements in a page. Perhaps we can do this in HTMX itself, but I think there is a known limitation related to how much we might expect here.

The solution I found seemed to me the most obvious choice, but I could pursue a different solution if you think there is a better way of doing this.

@jordaneremieff
Copy link
Author

jordaneremieff commented Jul 4, 2021

My current solution could be changed to use Django debug methods similar to technical_500_response. The main advantage of doing it this way is that if multiple error responses are encountered then each element will produce a debug output.

import sys

from django.conf import settings
from django.http import HttpResponse
from django.views.debug import get_exception_reporter_class

from django_htmx.middleware import HtmxMiddleware

class HtmxDebugMiddleware(HtmxMiddleware):
    def process_exception(self, request, exception):
        if request.htmx and settings.DEBUG:
            reporter = get_exception_reporter_class(request)(request, *sys.exc_info())
            content = reporter.get_traceback_text()
            return HttpResponse(content, content_type="text/plain; charset=utf-8")

        return None

However, there are some limitations/potential issues:

  1. I've only tested this for elements that I always expect to be populated with HTML data using the default swap rule via click/load triggers. Other swap rules/triggers might not work well.
  2. The get_traceback_html method on the reporter instance will have conflicts with other styling and Javascript in the page.

An alternative could be to persist the error context in the session and redirect on the first error encountered, though I haven't tried doing it this way myself at this point. I'm thinking something (roughly) like this could work:

  • Create a dedicated error handler view.
  • Store the reporter context in the request session.
  • Set the HX-Redirect header in the response to URL of the error handler.
  • Use the reporter context from the session to render the default Django 500 page.

If either of these solutions seem like a good fit for this library, then I would be happy to explore/test further and put forward a PR.

@adamchainz
Copy link
Owner

I’ve had a play around and I think we can handle this entirely in JavaScript, with no server side changes. The problem is really that htmx throws away Django’s useful error response, working around this on the server side is just going to be harder than intersecting the behaviour on the front end.

Here’s an example script I managed to get working:

document.addEventListener("htmx:beforeOnLoad", function (event) {
  const xhr = event.detail.xhr;
  if (xhr.status == 500) {
    document.children[0].innerHTML = xhr.response;
    const url = new URL(xhr.responseURL);
    history.pushState({}, "", url.pathname + url.search);
  }
});

This puts Django’s error response full screen on the page. It doesn’t currently handle reloading the scripts, but I think we can work around this by having htmx process the response - I'm currently researching.

@jordaneremieff
Copy link
Author

jordaneremieff commented Jul 5, 2021

I've tried your script and have been able to make the Django error response work as you described, and I have a couple questions/a comment:

  1. Could this be configurable using the Django debug setting? It may be preferable to allow other areas of the page to load instead of replacing the entire page with the error response, particularly in non-development environments.
  2. Would the recommendation be to include this script directly in templates where the behavior should be applied?
  3. If multiple triggers are run on the page and more than one error occurs, then only the first error response will be displayed. This is similar to what would be expected in a normal error case, however the error response that gets displayed is unpredictable/could be any of the triggers that fail. Maybe not a big issue, but could potentially be confusing when debugging.

@adamchainz
Copy link
Owner

  1. Could this be configurable using the Django debug setting?

I would make it only appear on pages when DEBUG = True, with a template tag.

particularly in non-development environments.

Please please please never run with DEBUG = True in anything but your development environment, or write your own code to expose stack traces. It's a security risk. The Django deployment checklist recommends strongly against it.

For errors in other environments, use a tool like Sentry.

  1. Would the recommendation be to include this script directly in templates where the behavior should be applied?

Yes, I'd probably ship it via a template tag that inserts a <script src="{% static "django_htmx/ext.js" %}> or similar

  1. If multiple triggers are run on the page and more than one error occurs, then only the first error response will be displayed. This is similar to what would be expected in a normal error case, however the error response that gets displayed is unpredictable/could be any of the triggers that fail. Maybe not a big issue, but could potentially be confusing when debugging.

If there is more than one error from requests made on the page, either the requests are queued at the same time, or not.

If they are not queued at the same time, the first one will hit the error case, the full page will be replaced, and then htmx should see all the other elements as removed and not fire their events.

If they are queued at the same time, both their responses will be parsed, and both error messages displayed. So they will at least be held in history.

I'm not so bothered about the possibility for multiple errors. Once you've fixed the first, you can debug and fix the second.

@jordaneremieff
Copy link
Author

Please please please never run with DEBUG = True in anything but your development environment

Yes, my concern was for the case when debug is not enabled and the script replaces the page with non-debug server error response. The templatetag approach should resolve this by only running this behaviour when debug is enabled.

I'm not so bothered about the possibility for multiple errors. Once you've fixed the first, you can debug and fix the second.

The potential confusion I was alluding to was that in the process of debugging some cases (e.g multiple load triggers) it might seem like the first error was resolved by some change after reloading the page and a different error just happens to return before the previous error is encountered again. That said, all of this information is still captured both in the backend and console, so it is still visible somewhere - just not immediately obvious.

Thank you for investigating this.

@adamchainz
Copy link
Owner

Yes, my concern was for the case when debug is not enabled and the script replaces the page with non-debug server error response. The templatetag approach should resolve this by only running this behaviour when debug is enabled.

Ah good, my misunderstanding... I've just seen too many people running debug mode in prod 😅

The potential confusion I was alluding to was that in the process of debugging some cases (e.g multiple load triggers) it might seem like the first error was resolved by some change after reloading the page and a different error just happens to return before the previous error is encountered again.

Yes this is true. I'm not sure there's much we can do about that. Thankfully most pages should be fairly deterministic in the order that errors occur.

adamchainz added a commit that referenced this issue Jul 8, 2021
@adamchainz
Copy link
Owner

Just released in version 1.2.0 - check it out!

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