Code

Opened 7 years ago

Closed 4 years ago

Last modified 14 months ago

#5617 closed New feature (wontfix)

Default server_error view should render with RequestContext

Reported by: Nick Fishman <kwlogical@…> Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Release blocker Keywords: server_error, RequestContext, error, templates
Cc: ionel.mc@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently, the default server_error view in django.views.defaults renders using the following code:

return http.HttpResponseServerError(t.render(Context({})))

However, this prevents the context processors specified in SETTINGS.TEMPLATE_CONTEXT_PROCESSORS from being loaded. For example, if a base.html might include branding that includes the current date and time (which is passed in through a custom context processor). Currently, if we try to extend base.html in the 500.html template, we get an error because the context variable for the date and time isn't loaded.

The best solution seems to be similar to the way the page_not_found view renders:

return http.HttpResponseServerError(t.render(RequestContext(request)))

Although the page_not_found view also passes in a request_path variable to RequestContext, I'm not sure if this is necessary.

Attachments (2)

server_error.patch (579 bytes) - added by Nick Fishman <kwlogical@…> 7 years ago.
Patch to render with RequestContext
server_error.2.patch (462 bytes) - added by Nick Fishman <kwlogical@…> 7 years ago.
Patch with generic django path

Download all attachments as: .zip

Change History (11)

Changed 7 years ago by Nick Fishman <kwlogical@…>

Patch to render with RequestContext

comment:1 Changed 7 years ago by Nick Fishman <kwlogical@…>

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Changed 7 years ago by Nick Fishman <kwlogical@…>

Patch with generic django path

comment:2 Changed 7 years ago by ubernostrum

  • Resolution set to wontfix
  • Status changed from new to closed

The default server-error handler was deliberately written this way to lessen the odds of new exceptions being raised during the execution of the server_error view.

comment:3 Changed 6 years ago by tobias

what about something like this?

def render_safe(request, template_name):
	t = loader.get_template(template_name)
	try:
		context = RequestContext(request, {'request_path': request.path})
		return t.render(context)
	except:
		return t.render(Context({}))


def page_not_found(request, template_name='404.html'):
	"""
	Default 404 handler.

	Templates: `404.html`
	Context:
		request_path
			The path of the requested URL (e.g., '/app/pages/bad_page/')
	"""
	return http.HttpResponseNotFound(render_safe(request, template_name))


def server_error(request, template_name='500.html'):
	"""
	500 error handler.

	Templates: `500.html`
	Context: None
	"""
	return http.HttpResponseServerError(render_safe(request, template_name))

comment:4 Changed 5 years ago by stavros

I would like to second this. I find tobias's approach best, as it doesn't break the handler on error and still allows people to use the RequestContext. I think that every site that uses MEDIA_URL will have a hacky handler500 to pass this to the template, and I think it would be much better if this was default Django bejaviour.

comment:5 Changed 4 years ago by IonelMaries

  • Cc ionel.mc@… added
  • Resolution wontfix deleted
  • Status changed from closed to reopened

I'm seconding this too. I know this is a django design choice but it's very inflexible - please review again.

comment:6 Changed 4 years ago by Alex

  • Resolution set to wontfix
  • Status changed from reopened to closed

This ticket was closed by a core developer, if you want to reopen the discussion about this please do it on the django-developers mailing list.

comment:7 Changed 3 years ago by tomchristie

  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

comment:8 in reply to: ↑ description Changed 2 years ago by anonymous

  • Component changed from HTTP handling to ORM aggregation
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to New feature

Replying to Nick Fishman <kwlogical@…>:

Currently, the default server_error view in django.views.defaults renders using the following code:

return http.HttpResponseServerError(t.render(Context({})))

However, this prevents the context processors specified in SETTINGS.TEMPLATE_CONTEXT_PROCESSORS from being loaded. For example, if a base.html might include branding that includes the current date and time (which is passed in through a custom context processor). Currently, if we try to extend base.html in the 500.html template, we get an error because the context variable for the date and time isn't loaded.

The best solution seems to be similar to the way the page_not_found view renders:

return http.HttpResponseServerError(t.render(RequestContext(request)))

Although the page_not_found view also passes in a request_path variable to RequestContext, I'm not sure if this is necessary.

comment:9 Changed 14 months ago by akaariai

  • Component changed from ORM aggregation to Database layer (models, ORM)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.