Re-opening 5617?

[Tom Christie]

Hi All,

  I recently bumped into the same issue as reported in #5617.  (And duplicate #6377.)

IE: Django's default 500 view uses a Context, not a RequestContext, which means any 500 templates which uses MEDIA_URL or suchlike won't work.

I thought I'd re-punt the suggestion given in one of the comments - attempt to render using the RequestContext, and only fall back to the Context if that fails.  It seems like a reasonable approach, but it looks like it's something that didn't ever get properly considered, because no-one pushed the issue to the list, as far as I can tell.

Where do we stand on this?  I'd have thought that a large amount of installs are having to define their own 500 view function, which does nothing more than the default view, other than to use a RequestContext so that the template can render correctly.  Would there be any value in re-opening the ticket given the suggested fallback approach?

Cheers,

  Tom.

[Carl Meyer]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Tom,

Thanks for bringing this to the list.

On 09/20/2011 10:32 AM, Tom Christie wrote:
> I recently bumped into the same issue as reported in #5617

> <https://code.djangoproject.com/ticket/5617>. (And duplicate #6377
> <https://code.djangoproject.com/ticket/6377>.)

> IE: Django's default 500 view uses a Context, not a RequestContext,
> which means any 500 templates which uses MEDIA_URL or suchlike won't work.
>
> I thought I'd re-punt the suggestion given in one of the comments -
> attempt to render using the RequestContext, and only fall back to the
> Context if that fails. It seems like a reasonable approach, but it
> looks like it's something that didn't ever get properly considered,
> because no-one pushed the issue to the list, as far as I can tell.
>
> Where do we stand on this? I'd have thought that a large amount of
> installs are having to define their own 500 view function, which does
> nothing more than the default view, other than to use a RequestContext
> so that the template can render correctly. Would there be any value in
> re-opening the ticket given the suggested fallback approach?

Personally I have always made the 500 page simple and completely
standalone (all CSS inline in the page) so it has no external
dependencies, and have never had a problem with this approach.

The concern I have with the proposed try-and-try-again approach is that
it makes people think they can write a 500 template with all kinds of
dependencies on RequestContext, and then if there is an error that does
actually break one of their context processors, their 500 page will in
all likelihood be horribly broken. I think it's better to encourage 500
pages that are simple all the time rather than beautiful some of the
time and completely broken some of the time.

An approach I'd be more open to considering would be to have the default
handler500 manually inject STATIC_URL into the template context (without
using RequestContext). I think this alone would take care of the vast
majority of issues people have with this, and simply looking up a
setting is no more likely to raise additional errors than the process of
rendering a template itself is (whereas context processors might do
anything). (It shouldn't be MEDIA_URL; since Django 1.3 that setting is
only for user-uploaded media, which is not something that should be
needed on a 500 page).

Thoughts on this?

Carl
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk55DnMACgkQ8W4rlRKtE2czWQCfWXkYN9CznrNd78bD6HtE7fCN
t3kAnRHBK7jjE1LAtKrSSPZkG9OVVGyi
=CJAN
-----END PGP SIGNATURE-----

[Tom Christie]

Heya,

  Thanks for the feedback.  I quite like the explicit 'STATIC_URL' only approach, although I think a lot of users would still run into a problem there, because 'request' isn't also added in explicitly to the Context...

  For context, my particular use case is a simple '500.html' template, that extends a 'base.html' template.  I don't use any other context in the base template other than 'request' and 'STATIC_URL'.  In the case of a 500 error, I'd see the template render correctly, except that it'd look like the user isn't logged in.  Coming across that as a dev that'd confuse the hell out of me the first time I came across it unless I already understood the 500 Context behavior, and it's not ideal from the end-user perspective either.

  I'd imagine that plenty of other setups would have a similar setup, so you could argue that returning this:

Context({'STATIC_URL': settings.STATIC_URL, 'request': request})

would be an okay thing to do in the default 500 handler.

  Personally I think that'd probably be absolutely fine (and the most sensible default 500 view), although the obvious counter argument is that that's getting into the realms of special-case, rather than default-case.  (That's not my opinion, but it'd be a valid argument.)

  What do you reckon?

Cheers,

  Tom

[Jannis Leidel]

On 21.09.2011, at 11:14, Tom Christie wrote:

> Heya,
>
> Thanks for the feedback. I quite like the explicit 'STATIC_URL' only approach, although I think a lot of users would still run into a problem there, because 'request' isn't also added in explicitly to the Context...
>
> For context, my particular use case is a simple '500.html' template, that extends a 'base.html' template. I don't use any other context in the base template other than 'request' and 'STATIC_URL'. In the case of a 500 error, I'd see the template render correctly, except that it'd look like the user isn't logged in. Coming across that as a dev that'd confuse the hell out of me the first time I came across it unless I already understood the 500 Context behavior, and it's not ideal from the end-user perspective either.
>
> I'd imagine that plenty of other setups would have a similar setup, so you could argue that returning this:
>
> Context({'STATIC_URL': settings.STATIC_URL, 'request': request})
>
> would be an okay thing to do in the default 500 handler.

Passing STATIC_URL to the 500.html template is not needed anymore
now that we've added a new {% static %} template tag:

https://docs.djangoproject.com/en/dev/releases/1.4/#static-template-tag

I strongly believe this ticket shouldn't be reopened because of that.

Jannis

[Jannis Leidel]

Oh, I forgot to say, there is of course also a non-staticfiles version
of the {% static %} template tag in case you don't use the contrib app
staticfiles:

https://docs.djangoproject.com/en/dev/ref/templates/builtins/#static

Jannis

[Carl Meyer]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ah, thanks, I'd forgotten about the new {% static %} tag. I agree that
this makes adding STATIC_URL to the handler500 context unnecessary.

Adding the request is a non-starter, IMO; the "request" context
processor isn't even in the default list of TEMPLATE_CONTEXT_PROCESSORS,
so this would mean adding something to the 500 page context that isn't
even added by RequestContext by default. If you want the request object
in your 500 page, you're well into custom handler500 territory.

(IMO it's poor practice to have the 500 template inherit from your
site's normal base template; even if it works now, it's easy to
introduce things into your base template that could break under certain
500 error conditions. Better to have the 500 template standalone.
Likewise with having the 500 template display logged-in status: that
depends on the full session/auth stack, which likely depends on some
kind of external session store - the 500 error could easily be due to
that external store being unavailable in the first place!)

Carl
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk553OYACgkQ8W4rlRKtE2e4FwCcDRh9kZZO7syAP4BxfJF+0w2u
xhIAnAu9cYF9Mve4vVMIfkeSeTsOBhKx
=tnRL
-----END PGP SIGNATURE-----

[Tom Christie]

Hey Jannis,

> https://docs.djangoproject.com/en/dev/releases/1.4/#static-template-tag

That's rather nice.

> Adding the request is a non-starter, IMO; the "request" context
> processor isn't even in the default list of TEMPLATE_CONTEXT_PROCESSORS,
> so this would mean adding something to the 500 page context that isn't
> even added by RequestContext by default. If you want the request object
> in your 500 page, you're well into custom handler500 territory.

Yeah, that's me being stupid!  I should really have been talking about having 'user' available, given that 'django.contrib.auth.context_processors.auth' _is_ enabled in TEMPLATE_CONTEXT_PROCESSORS.

So you _could_ argue that returning 'Context({'user': request.user})' would be a more useful default than the current empty context.

However I think I'm convinced by Carls point that attempting to do things with 'user' inside a 500 view might not be best practice in any case.

Thanks for the input both.  I've linked from the bugs to here.  Should be nice and clear for anyone hitting them again in the future.

I'm considering making the doc's on the 500 view more obviously explicit about the use of Context, not RequestContext, as it's a bit of a gotcha, and reasonably easy to miss.  IE, putting the text "...is rendered with an empty Context..." into a Note block to make it more obvious.  I'll submit a separate bug and patch for that when I've got a moment.

Cheers,

  Tom