#25369: Wrong exception being used in official example creates very subtle bugs
for
the syndication feed framework.
-------------------------------------+-------------------------------------
Reporter: PaulWGraham | Owner: nobody
Type: Bug | Status: new
Component: contrib.syndication | Version: 1.8
Severity: Normal | Resolution:
Keywords: syndication feed | Triage Stage:
future subtle documentation | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by PaulWGraham:
Old description:
> == '''The Setup''' ==
>
> '''A. Example code from documentation'''
>
> {{{
> from django.contrib.syndication.views import Feed
> from django.shortcuts import get_object_or_404
>
> class BeatFeed(Feed):
> description_template = 'feeds/beat_description.html'
>
> def get_object(self, request, beat_id):
> return get_object_or_404(Beat, pk=beat_id)
> }}}
>
> '''B. Code from Feed(object)'''
>
> {{{
> def __call__(self, request, *args, **kwargs):
> try:
> obj = self.get_object(request, *args, **kwargs)
> except ObjectDoesNotExist:
> raise Http404('Feed object does not exist.')
> }}}
>
> In the '''A complex example''' (
> [https://docs.djangoproject.com/en/1.8/ref/contrib/syndication/#a
> -complex-example] ) example code the method {{{get_object()}}} uses
> {{{get_object_or_404()}}} to raise an {{{Http404}}} exception (See A.
> above). However the the Feed class reference on the same page indicates
> that {{{ObjectDoesNotExist}}} exception should be raised on error below
> looking at the code of the {{{__call__()}}} method of the Feed class
> shows that it calls the {{{get_object()}}} method it does indeed catch
> {{{ObjectDoesNotExist}}} exceptions (See B. above). However, the reason
> the example code runs without any obvious errors and the root of the
> problem at hand is that upon catching an ObjectDoesNotExist exception the
> Feed class then raises an {{{Http404}}} error. The {{{Http404}}}
> exception being raised in the example code for {{{get_object()}}} is not
> being caught where it should be, inside {{{Feed.__call__()}}} but
> ultimately farther up the chain by the code that was meant to catch
> errors from {{{Feed.__call__()}}}
>
> == '''The Problem''' ==
>
> It should be noted that people have been depending upon the example in
> question as reference and that changing how the Feed class handles
> exceptions or changing code that handles from
> {{{get_object()}}} could be hazardous as it would could break previously
> conformant code created by users. In fact, I discovered this bug while
> reading a third party Django RSS tutorial that was based on the official
> Django documentation.
>
> There are at least two problematic scenarios that spring to mind:
>
> The first problematic scenario is that the way the Feed class handles
> {{{ObjectDoesNotExist}}} exceptions changes and the new error handling
> code is skipped by an {{{Http404}}} error. Ex.
>
> In class Feed:
>
> {{{
> def __call__(self, request, *args, **kwargs):
> try:
> obj = self.get_object(request, *args, **kwargs)
> except ObjectDoesNotExist:
> # Any new error handling code placed here will
> # be skipped the Http404 exception client code
> # could be using. <-- Problem
> raise Http404('Feed object does not exist.')
> }}}
>
> The second problematic scenario if users have been calling the
> {{{Feed.__call__()}}} method for any reason (subclassing Feed and
> overidding __call__ method perhaps) with the idea, backed by an example
> from the documentation, that they should only be catching {{{Http404}}}
> on error Ex.
>
> {{{
> try:
> x = my_feed_instance = get_object(request, *args, **kwargs)
> except Http404:
> # Code here won't be run if an ObjectDoesNotExist exception is
> raised.
> }}}
New description:
== '''The Setup''' ==
'''A. Example code from documentation'''
{{{
from django.contrib.syndication.views import Feed
from django.shortcuts import get_object_or_404
class BeatFeed(Feed):
description_template = 'feeds/beat_description.html'
def get_object(self, request, beat_id):
return get_object_or_404(Beat, pk=beat_id)
}}}
'''B. Code from Feed(object)'''
{{{
def __call__(self, request, *args, **kwargs):
try:
obj = self.get_object(request, *args, **kwargs)
except ObjectDoesNotExist:
raise Http404('Feed object does not exist.')
}}}
In the '''A complex example''' (
[https://docs.djangoproject.com/en/1.8/ref/contrib/syndication/#a-complex-
example] ) example code the method {{{get_object()}}} uses
{{{get_object_or_404()}}} to raise an {{{Http404}}} exception (See A.
above). However the the Feed class reference on the same page indicates
that {{{ObjectDoesNotExist}}} exception should be raised on error. Looking
at the code of the {{{__call__()}}} method of the Feed class shows that it
calls the {{{get_object()}}} method and it does indeed catch
{{{ObjectDoesNotExist}}} exceptions (See B. above). However, the reason
the example code runs without any obvious errors and the root of the
problem at hand is that upon catching an ObjectDoesNotExist exception the
Feed class then raises an {{{Http404}}} error. This means that the
{{{Http404}}} exception being raised in the example code for
{{{get_object()}}} is not being caught where it should be, inside
{{{Feed.__call__()}}} but ultimately falls through that try: except:
block and is handled farther up the chain by the code that was meant to
catch errors from {{{Feed.__call__()}}}
== '''The Problem''' ==
It should be noted that people have been depending upon the example in
question as reference and that changing how the Feed class handles
exceptions or changing code that handles from
{{{get_object()}}} could be hazardous as it would could break previously
conformant code created by users. In fact, I discovered this bug while
reading a third party Django RSS tutorial that was based on the official
Django documentation.
There are at least two problematic scenarios that spring to mind:
The first problematic scenario is that the way the Feed class handles
{{{ObjectDoesNotExist}}} exceptions changes and the new error handling
code is skipped by an {{{Http404}}} error. Ex.
In class Feed:
{{{
def __call__(self, request, *args, **kwargs):
try:
obj = self.get_object(request, *args, **kwargs)
except ObjectDoesNotExist:
# Any new error handling code placed here will
# be skipped the Http404 exception client code
# could be using. <-- Problem
raise Http404('Feed object does not exist.')
}}}
The second problematic scenario if users have been calling the
{{{Feed.__call__()}}} method for any reason (subclassing Feed and
overidding __call__ method perhaps) with the idea, backed by an example
from the documentation, that they should only be catching {{{Http404}}} on
error Ex.
{{{
try:
x = my_feed_instance = get_object(request, *args, **kwargs)
except Http404:
# Code here won't be run if an ObjectDoesNotExist exception is
raised.
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25369#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/069.38f1fb572a0a2e9b2db4293b1755aede%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
