getsentry / sentry-docs

Sentry's documentation (and tools to build it)
https://docs.sentry.io
Other
332 stars 1.46k forks source link

Instructions to enable user feedback for Django do not work #120

Closed keimlink closed 3 years ago

keimlink commented 7 years ago

The instructions explaining how to enable user feedback for Django do not work. Django passes an empty context to the 500.html template. So the user feedback form is never shown because the required request.sentry.id variable is missing.

django.views.defaults.server_error() calls template.render() with no arguments:

@requires_csrf_token
def server_error(request, template_name=ERROR_500_TEMPLATE_NAME):
    """
    500 error handler.
    Templates: :template:`500.html`
    Context: None
    """
    try:
        template = loader.get_template(template_name)
    except TemplateDoesNotExist:
        if template_name != ERROR_500_TEMPLATE_NAME:
            # Reraise if it's a missing custom template.
            raise
        return http.HttpResponseServerError('<h1>Server Error (500)</h1>', content_type='text/html')
    return http.HttpResponseServerError(template.render())

Looking at the code of django.template.backends.django.Template.render() and django.template.context.make_context() shows that in this case just an empty context is returned:

class DjangoTemplates(BaseEngine):

    # ...

    def render(self, context=None, request=None):
        context = make_context(context, request, autoescape=self.backend.engine.autoescape)
        try:
            return self.template.render(context)
        except TemplateDoesNotExist as exc:
            reraise(exc, self.backend)
def make_context(context, request=None, **kwargs):
    """
    Create a suitable Context from a plain dict and optionally an HttpRequest.
    """
    if context is not None and not isinstance(context, dict):
        raise TypeError('context must be a dict rather than %s.' % context.__class__.__name__)
    if request is None:
        context = Context(context, **kwargs)
    else:
        # The following pattern is required to ensure values from
        # context override those from template context processors.
        original_context = context
        context = RequestContext(request, **kwargs)
        if original_context:
            context.push(original_context)
    return context

Creating the HttpResponseServerError with a custom server_error() view like this solves the problem:

@requires_csrf_token
def server_error(request, template_name=ERROR_500_TEMPLATE_NAME):
    """500 error handler using RequestContext."""
    try:
        template = loader.get_template(template_name)
    except TemplateDoesNotExist:
        if template_name != ERROR_500_TEMPLATE_NAME:
            # Reraise if it's a missing custom template.
            raise
        return http.HttpResponseServerError('<h1>Server Error (500)</h1>', content_type='text/html')
    return http.HttpResponseServerError(template.render(None, request))
fcpauldiaz commented 7 years ago

I can't get this to work. @kamilogorek What should be in the urls.py ?

kamilogorek commented 7 years ago

cc @mitsuhiko

mitsuhiko commented 7 years ago

@fcpauldiaz i am not entirely sure what you are asking. Can you clarify what you mean with "can't get this to work"?

fcpauldiaz commented 7 years ago

@mitsuhiko The request.sentry object is still not available in my 500.html

This is my urls.py

from main import views as main_views
from django.conf.urls import  handler500 

handler500 = main_views.server_error

And tried this on the views.py

def server_error(request, template_name='500.html'):
    """500 error handler using RequestContext."""
    try:
        template = loader.get_template(template_name)
    except TemplateDoesNotExist:
        if template_name != '500.html':
            # Reraise if it's a missing custom template.
            raise
        return http.HttpResponseServerError('<h1>Server Error (500)</h1>', content_type='text/html')
    return http.HttpResponseServerError(template.render(None, request, status=500))

This is the templates/500.html

{% extends 'base.html' %}
{% block title %} Error {% endblock %}
{% block content %}
{% load raven %}
<div class="row container">
  <div class="col l10 offset-l3 s12 m8 offset-m2">
  <h3 class=lue-paypal-text center>You've encountered an error, oh noes!</h3>
  {{request.sentry}}
  {% if request.sentry.id %}
      <p>If you need assistance, you may reference this error as
      <strong>{{ request.sentry.id }}</strong>.</p>
  {% endif %}
  </div>
</div>
{% endblock %}

{% block js %}
<script>Raven.config('{% sentry_public_dsn %}').install()</script>

{% if request.sentry.id %}
  <script>
  Raven.showReportDialog({
    eventId: '{{ request.sentry.id }}',

    // use the public DSN (dont include your secret!)
    dsn: 'dns_here'
  });
  </script>
{% endif %}
{% endblock %}
dcramer commented 7 years ago

@fcpauldiaz im not sure if template.render ensures RequestContext is used, but its important that 'request' is made available to the template. If thats not happening this will never work. Additionally, if for some reason the 'Sentry' middleware isn't being installed, it also may not be available.

fcpauldiaz commented 7 years ago

This is my middleware config

MIDDLEWARE = [
    'django.middleware.security.SecurityMiddleware',
    'whitenoise.middleware.WhiteNoiseMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.common.CommonMiddleware',
    'django.middleware.csrf.CsrfViewMiddleware',
    'django.contrib.auth.middleware.AuthenticationMiddleware',
    'django.contrib.messages.middleware.MessageMiddleware',
    'django.middleware.clickjacking.XFrameOptionsMiddleware',
    'raven.contrib.django.raven_compat.middleware.SentryResponseErrorIdMiddleware'
]

And the template processors

TEMPLATES = [
    {
        'BACKEND': 'django.template.backends.django.DjangoTemplates',
        'DIRS': [os.path.join(BASE_DIR, 'templates'),],
        'APP_DIRS': True,
        'OPTIONS': {
            'context_processors': [
                'django.template.context_processors.debug',
                'django.template.context_processors.request',
                'django.contrib.auth.context_processors.auth',
                'django.contrib.messages.context_processors.messages',
            ],
            'debug': DEBUG,
        },
    },
]
dcramer commented 7 years ago

@fcpauldiaz I dont know what Django does with your template.render call, so you need to verify that 'request' is actually being passed to the template. In older versions of Django that would not happen, and I dont know if the API has since changed (and dont have time to dig into their docs/implementation right now). That owuld be my best guess at whats wrong here.

keimlink commented 7 years ago

@dcramer That's exactly why started this issue: request is not passed to the template, so you need to use a custom server_error() view. My example works with Django 1.8 and Sentry in production.

dcramer commented 7 years ago

Yeah sorry was ignoring the originally issue. I'm not sure why we built the docs the way we did (oversight?), but there's a reasonable example in our generic docs:

https://docs.sentry.io/learn/user-feedback/

It's also important to note that Sentry should monkey-patch in a middleware which ensures some things happen. Without that none of this will work either. I can't imagine that isn't working so its likely just getting down to a fully functional custom_error function.

I'd also +1 shipping with this server_handler function, and possibly overwriting Django's default (though that might be a concern for future versions).

Alternatively, we can create a template tag so you dont need a custom function, and can simply {% load sentry %} {{ sentry.event_id }}

keimlink commented 7 years ago

Shipping Raven with a custom server_error() view sounds like an excellent idea. But why not leaving it up to the user if they want to use it by setting handler500 instead of monkey-patching Django? This would make it easier to handle changes to Django's server_error() view.

The setup instructions for Raven could explain that you either have to configure something like this in your urls.py or to use your own function.

from raven.views import server_error

handler500 = server_error

If Django's server_error() view is updated to pass the request to the template context only this part of the documentation has to be updated, but not the software itself.

Hope my explanation is clear enough to be understood…

jaaved commented 6 years ago

In Django 2.0.4 using this custom handler500 worked for me:

@requires_csrf_token
def sentry_server_error(request, template_name=ERROR_500_TEMPLATE_NAME):
    """
    500 error handler.

    Templates: :template:`500.html`
    Context: None
    """
    try:
        template = loader.get_template(template_name)
    except TemplateDoesNotExist:
        if template_name != ERROR_500_TEMPLATE_NAME:
            # Reraise if it's a missing custom template.
            raise
        return HttpResponseServerError('<h1>Server Error (500)</h1>', content_type='text/html')
    #return HttpResponseServerError(template.render(RequestContext(request)))
    return HttpResponseServerError(template.render(context=None, request=request))
github-actions[bot] commented 3 years ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Accepted, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀