[Django] #36775: Raise ImproperlyConfigured when Feed.link is missing instead of failing with AttributeError

[Django]

#36775: Raise ImproperlyConfigured when Feed.link is missing instead of failing
with AttributeError
-------------------------------------+-------------------------------------
Reporter: yureiblack | Type: Bug
Status: new | Component:
| contrib.syndication
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Currently, the `Feed.get_feed()` method retrieves the feed's `link` using:

link = self._get_dynamic_attr("link", obj)

If a Feed subclass does not define a `link` attribute, `link` becomes
`None`.
This results in an unhelpful exception when Django later calls:

link = add_domain(current_site.domain, link, request.is_secure())

Since `add_domain()` calls `url.startswith()`, a missing `link`
causes:

AttributeError: 'NoneType' object has no attribute 'startswith'

This error does not clearly indicate the actual problem.

According to Django documentation, every `Feed` class must define
`link`. Therefore, it should raise a more appropriate and explicit
exception.

The attached PR adds the following check:

if link is None:
raise ImproperlyConfigured(
"Feed class must define a 'link' attribute."
)

This results in a clearer error message and avoids unexpected crashes.

PR: https://github.com/django/django/pull/20371
--
Ticket URL: <https://code.djangoproject.com/ticket/36775>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36775: Raise ImproperlyConfigured when Feed.link is missing instead of failing
with AttributeError
-------------------------------------+-------------------------------------

Reporter: yureiblack | Owner:
Type: | yureiblack
Cleanup/optimization | Status: assigned
Component: contrib.syndication | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_tests: 0 => 1
* owner: (none) => yureiblack
* stage: Unreviewed => Accepted
* status: new => assigned
* type: Bug => Cleanup/optimization


Old description:

> Currently, the `Feed.get_feed()` method retrieves the feed's `link`
> using:
>
> link = self._get_dynamic_attr("link", obj)
>
> If a Feed subclass does not define a `link` attribute, `link` becomes
> `None`.
> This results in an unhelpful exception when Django later calls:
>
> link = add_domain(current_site.domain, link, request.is_secure())
>
> Since `add_domain()` calls `url.startswith()`, a missing `link`
> causes:
>
> AttributeError: 'NoneType' object has no attribute 'startswith'
>
> This error does not clearly indicate the actual problem.
>
> According to Django documentation, every `Feed` class must define
> `link`. Therefore, it should raise a more appropriate and explicit
> exception.
>
> The attached PR adds the following check:
>
> if link is None:
> raise ImproperlyConfigured(
> "Feed class must define a 'link' attribute."
> )
>
> This results in a clearer error message and avoids unexpected crashes.
>
> PR: https://github.com/django/django/pull/20371

New description:

Currently, the `Feed.get_feed()` method retrieves the feed's `link` using:

{{{#!py

link = self._get_dynamic_attr("link", obj)
}}}
If a Feed subclass does not define a `link` attribute, `link` becomes
`None`.
This results in an unhelpful exception when Django later calls:

{{{#!py

link = add_domain(current_site.domain, link, request.is_secure())
}}}
Since `add_domain()` calls `url.startswith()`, a missing `link`
causes:

{{{#!py

AttributeError: 'NoneType' object has no attribute 'startswith'
}}}
This error does not clearly indicate the actual problem.

According to Django documentation, every `Feed` class must define
`link`. Therefore, it should raise a more appropriate and explicit
exception.

The attached PR adds the following check:

{{{#!py

if link is None:
raise ImproperlyConfigured(
"Feed class must define a 'link' attribute."
)
}}}
This results in a clearer error message and avoids unexpected crashes.

PR: https://github.com/django/django/pull/20371

--

Comment:

Seems reasonable, thanks. (I think this is the "better error" described as
"wouldn't kill us" in a very similar scenario in ticket:2218#comment:1.)

Could you include a test?
--
Ticket URL: <https://code.djangoproject.com/ticket/36775#comment:1>