RSS feed error handling #15476
RSS feed error handling #15476
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Pass individual fetch/parse errors from child process and console.warn with parent, continuing processes. Add additional guard to make sure enough blog items were fetch; if less then break build.
handles cases where fetch returns html, but invalid RSS/XML feed
This reverts commit 02da378.
wackerow
requested review from
corwintines,
pettinarip and
minimalsm
as code owners
| if (allItems.length < RSS_DISPLAY_COUNT) | ||
| throw new Error("Insufficient number of RSS items fetched") |
There was a problem hiding this comment.
This isn't actually breaking the build as-is, since it is contained inside an api function call... 🤔 cc: @pettinarip
There was a problem hiding this comment.
Okay, with the addition of generateStaticParams on the homepage, this throw should properly break the build if the RSS fetch doesn't return the 6 results desired to display on the homepage.
does not break build as intended, only breaks front end
There was a problem hiding this comment.
By design, the fetches under src/lib/api should throw. The error handling logic should happen at the page level. Currently, the dataLoader layer is capturing these errors and throwing.
The reason this test is not failing is because the flag USE_MOCK_DATA is on in preview deploys. To fully test it, you can override the variable in netlify.toml or in netlify UI config.
| } | ||
| } | ||
| return allItems as RSSItem[][] | ||
| return allItems |
There was a problem hiding this comment.
this function fetch will never throw, is that the intention? if we want to break the build, we should
There was a problem hiding this comment.
I don't want a singular feed to break the build (ie. recent example where one blog changed their feed link)... these cases should just skip over that feed.
BUT, in the event something happens where we don't get ANY feed results, then that should break the build... which was the intent of throwing an error at the end if there aren't enough results.
This reverts commit 360c439.
Couple things: 1- Netlify: 
But, I didn't test this issue specifically through Netlify (truncating the list of RSS feeds so we don't get an adequate number of responses, which imo should break the build.... 2- Locally: I did test locally though, and it builds just fine without breaking when I only have a single RSS feed source listed (which should break it)... 
|
| @@ -127,6 +128,8 @@ const Page = async ({ params }: { params: Promise<{ locale: Lang }> }) => { | |||
| ) | |||
| } | |||
|
|
|||
| export const generateStaticParams = async () => LOCALES_CODES | |||
There was a problem hiding this comment.
Forces homepage to builds paths at build, which allows throw Error to break the build
| if (allItems.length < RSS_DISPLAY_COUNT) | ||
| throw new Error("Insufficient number of RSS items fetched") |
There was a problem hiding this comment.
Okay, with the addition of generateStaticParams on the homepage, this throw should properly break the build if the RSS fetch doesn't return the 6 results desired to display on the homepage.
Description
fetchRSS)RSS_DISPLAY_COUNT) items, else it throws a breaking error. This allows flexibility for small outages or link changes, but will block anything larger that results in missing items in the front end.Related Issue
Recent homepage outage from an external rss feed url being changed