[plugin/NEWS-RSS] Handle missing description gracefully in rss feed by Blizarre · Pull Request #13407 · koreader/koreader

Blizarre · 2025-03-13T19:55:56Z

Hi, I couldn't find instructions on how to make PRs so I just pushed one. Feel free to tell me if the procedure isn't followed. I stumbled on KoReader 3 days ago and it is a real gem, thank you very much for your work!

I found this issue on nightly 2025-03-10, v2024.11-223-g154ec621d_2025-03-03, KindlePaperWhite5.

Issue

Some RSS feeds contain items that don't have a description (for instance this one, which is valid as per https://validator.w3.org/feed/). In that case the processing for the feed just stop abruptly and we get "Could not process some feeds. Unsupported format in: XXX (Reason: Failed to download content)".

Probably because the call feed.description[1] raise an exception like in this code, and the code is run on another thread (? I am not too familiar with lua), and the exception is not catched.

> item = {}
> item.description[1]
stdin:1: attempt to index a nil value (field 'description')
stack traceback:
        stdin:1: in main chunk
        [C]: in ?

Quick Fix

I check if there is a "description" element in the feed. If that's not the case I use the empty string.
If download_full_article is not set, it means that we will have a very short document that only contains the title. However since I have no way that I can see to give the user feedback, I'd rather just generate the file and let the user come to their own conclusions (same case as with very short descriptions).

This change is

Frenzie · 2025-03-13T20:21:20Z

        local feed_description
        if feed_type == FEED_TYPE_RSS then
            feed_title = feed.title
-            feed_description = feed.description[1] or feed.description --- @todo This should select the one with type="html" if there is a choice.


You can write it as feed.description and feed.description[1] or feed.description. Having the title displayed as content under the title repeating it strikes me as a bit silly. :-)

PS Best to apply the same fix a few lines down for Atom.

Hum, that's true, I tried to keep the same style as these lines below:

if feed["content:encoded"] ~= nil then -- Spec: https://web.resource.org/rss/1.0/modules/content/ feed_description = feed["content:encoded"]

But your one-liner is cleaner. I'll update the PR.

Having the title displayed as content under the title repeating it strikes me as a bit silly. :-)

I didn't want to set it to nil, I thought it would blow up without a description? Let's have a look.

If it does you can set it to "".

Edit: meaning feed.description and feed.description[1] or feed.description or ""

Yep, it blew up... We could add a feed.description or "No description available" somewhere. The description becomes the main body of the document. so there should be "something"

Ok, I have pushed a new version. I am not too sure about inlining the feed_description or "No description available", inside the method call but....
Oh.. translations....

Yeah, How do I handle that new Message string? Should I create a new entry in the localization files? I will just set it to empty string for now and if needed we can go the extra mile and request translations

In this case I'm of the opinion that if it's empty it's empty. ;-)

For translation in general you can simply surround the string by _() and it'll be picked up automatically, provided it's of the form of "" or [[]].

Frenzie · 2025-03-13T20:51:25Z

                feed,
                feed_title,
-                feed_description,
+                feed_description or "No description available",


I'd rather keep the logic up above for clarity. (And with a plain empty string.)

Frenzie · 2025-03-13T21:03:47Z

Thanks!

Blizarre · 2025-03-13T21:08:33Z

You merged a bit too fast, I pushed a commit where I moved the or "" up as requested :p
Thank you very much for the extremely quick review cycle, that was very helpful.

Frenzie · 2025-03-13T21:19:31Z

A small focused PR is easy to review. And ultimately I didn't really say anything other than let's keep the style the same for now. ^_^ That part of the code could surely use some improvements but I'd do that after adding some automated tests to reduce the chance of accidental regressions.

It's actually an annoyance of mine that having a feed that purely notifies (i.e., no description/summary/content/whatever) is technically against the Atom spec. A common (?) workaround is to put only a single . in there.

Regression in #13407. Reported in <#13752 (reply in thread)>.

To help prevent situations like koreader#13799. As also referenced in <koreader#13407 (comment)>. Also fixes single item RSS and single entry Atom feeds in passing. References koreader#3073.

To help prevent situations like #13799. As also referenced in <#13407 (comment)>. Also fixes single item RSS and single entry Atom feeds in passing. References #3073.

…oreader#13407)

Regression in koreader#13407. Reported in <koreader#13752 (reply in thread)>.

To help prevent situations like koreader#13799. As also referenced in <koreader#13407 (comment)>. Also fixes single item RSS and single entry Atom feeds in passing. References koreader#3073.

handle missing description gracefully in rss feed

60d20d5

Frenzie reviewed Mar 13, 2025

View reviewed changes

Frenzie added this to the 2025.02 milestone Mar 13, 2025

Frenzie added the Plugin label Mar 13, 2025

Alternative implementation that display a fixed string

0ffd4d0

Frenzie reviewed Mar 13, 2025

View reviewed changes

Comment thread plugins/newsdownloader.koplugin/main.lua Outdated

Blizarre added 2 commits March 13, 2025 20:57

Cowardly backing away from adding a new translation string

0728efc

Revert change

c4e5d10

Frenzie merged commit 09c9a83 into koreader:master Mar 13, 2025

Frenzie added the NewsDownloader label Mar 13, 2025

Frenzie added a commit that referenced this pull request May 16, 2025

[plugin] NewsDownloader: fix RSS feeds

85a9172

Regression in #13407. Reported in <#13752 (reply in thread)>.

Frenzie mentioned this pull request May 16, 2025

[plugin] NewsDownloader: fix RSS feeds #13799

Merged

Frenzie added a commit that referenced this pull request May 16, 2025

[plugin] NewsDownloader: fix RSS feeds (#13799)

76e88f9

Regression in #13407. Reported in <#13752 (reply in thread)>.

Frenzie mentioned this pull request May 16, 2025

Add tests for NewsDownloader #13806

Merged

Frenzie added a commit that referenced this pull request May 30, 2025

Add tests for NewsDownloader (#13806)

0257caa

To help prevent situations like #13799. As also referenced in <#13407 (comment)>. Also fixes single item RSS and single entry Atom feeds in passing. References #3073.

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026

[plugin/NEWS-RSS] Handle missing description gracefully in RSS feed (k…

11aae50

…oreader#13407)

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026

[plugin] NewsDownloader: fix RSS feeds (koreader#13799)

e7605eb

Regression in koreader#13407. Reported in <koreader#13752 (reply in thread)>.

Uh oh!

Uh oh!

Conversation

Blizarre commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Quick Fix

Uh oh!

Frenzie Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

Frenzie Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

Blizarre Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

Frenzie Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Blizarre Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Blizarre Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Blizarre Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

Frenzie Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

Frenzie Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Frenzie commented Mar 13, 2025

Uh oh!

Blizarre commented Mar 13, 2025

Uh oh!

Frenzie commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Blizarre commented Mar 13, 2025 •

edited

Loading

Frenzie Mar 13, 2025 •

edited

Loading

Blizarre Mar 13, 2025 •

edited

Loading

Blizarre Mar 13, 2025 •

edited

Loading

Frenzie commented Mar 13, 2025 •

edited

Loading