Skip to content

NewsDownloader: deal with 404 and other error statuses better#13165

Merged
Frenzie merged 3 commits into
koreader:masterfrom
Frenzie:9363-better-failure
Jan 29, 2025
Merged

NewsDownloader: deal with 404 and other error statuses better#13165
Frenzie merged 3 commits into
koreader:masterfrom
Frenzie:9363-better-failure

Conversation

@Frenzie

@Frenzie Frenzie commented Jan 29, 2025

Copy link
Copy Markdown
Member

May be relevant to #9363. In any case one of the most typical kind of failure wasn't treated properly, so a 404 would mess up everything instead of popping up a dialog.


This change is Reviewable

May be relevant to koreader#9363. In any case one of the most typical kind of failures wasn't treated properly, so a 404 would mess up everything instead of popping up a dialog.
if code >= 400 and code < 500 then
logger.warn("socket error:", status or code)
if code == 400 then
return false, "Bad request - invalid URL or parameters"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea what's going on here tbh. It seems incredibly redundant with status. I'll just do:

Suggested change
return false, "Bad request - invalid URL or parameters"
return false, status or code

@pazos

pazos commented Jan 29, 2025

Copy link
Copy Markdown
Member

I've read it as "40 times better" :p

@Frenzie Frenzie changed the title NewsDownloader: deal with 40x better (especially 404) NewsDownloader: deal with 404 and other error statuses better Jan 29, 2025
@Frenzie Frenzie merged commit c1f6a02 into koreader:master Jan 29, 2025
@Frenzie Frenzie deleted the 9363-better-failure branch January 29, 2025 17:10
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
…er#13165)

May be relevant to koreader#9363. In any case one of the most typical kind of failures wasn't treated properly, so a 404 would mess up everything instead of popping up a dialog.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants