Adding sanity to an error message if HttpResponse is not returned from a view.

[Justin Holmes]

Currently, we perform a sanity check to provide a useful error message
if a view returns None:

https://github.com/django/django/blob/1101467ce0756272a54f4c7bc65c4c335a94111b/django/core/handlers/base.py#L138

However, if a view returns a different object (say, a string), we
don't have a similarly useful message.

This is a very common newcomer mistake, and it causes immense frustration.

I have just had a third student encounter this problem and feel
completely blocked by it, so I think it's time to bring it to the
list.

I suggest that we add a try block, catching AttributeError, and
wrapping it, adding to the beginning of the message, "The object
returned by the view was not compatible with the Django http
subsystem. Remember: views must return HttpResponse objects."

Also, similar to the new error messages accompanying the app object, I
want to suggest that we add a link to the document describing this.

I imagine that this has been a discussion before, but I can't find a
ticket. Is there one?


--
Justin Holmes
Chief Chocobo Breeder, slashRoot

slashRoot: Coffee House and Tech Dojo
New Paltz, NY 12561
845.633.8330

[Russell Keith-Magee]

Hi Justin,

On Tue, Sep 23, 2014 at 7:10 AM, Justin Holmes <jus...@justinholmes.com> wrote:

Currently, we perform a sanity check to provide a useful error message
if a view returns None:

https://github.com/django/django/blob/1101467ce0756272a54f4c7bc65c4c335a94111b/django/core/handlers/base.py#L138

However, if a view returns a different object (say, a string), we
don't have a similarly useful message.

This is a very common newcomer mistake, and it causes immense frustration.

I have just had a third student encounter this problem and feel
completely blocked by it, so I think it's time to bring it to the
list.

I suggest that we add a try block, catching AttributeError, and
wrapping it, adding to the beginning of the message, "The object
returned by the view was not compatible with the Django http
subsystem.  Remember: views must return HttpResponse objects."

Also, similar to the new error messages accompanying the app object, I
want to suggest that we add a link to the document describing this.

I imagine that this has been a discussion before, but I can't find a
ticket.  Is there one?

Not that I can recall. I think it's one of those ideas that's just so obvious, nobody thought to report it before :-)

I can't think of any reason why what you propose would be a bad idea.

I don't think that big of a change is required in the error message, though - the current message is fine, as long as you replace "none" with "type(response)", along with protection for the case where a HttpReponse *is* returned, but has internally raised an AttributeError:

try:

    (...handle response)

except AttributeError:

    if isinstance(response, HttpResponse):

        raise

    raise ValueError('The view %s.%s didn't return an HttpResponse object. It returned %s instead."

                                 % (callback.__module__, view_name, type(response)))

 

Yours,

Russ Magee %-)