fix: continue sponsor data fetch when one source fails#995
fix: continue sponsor data fetch when one source fails#995sethamus wants to merge 2 commits intoeslint:mainfrom
Conversation
✅ Deploy Preview for new-eslint ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for fr-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for ja-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for zh-hans-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for es-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for de-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hi-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for pt-br-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR updates the fetch:sponsors data-fetch script to be resilient to individual upstream sponsor-source failures so that one rejected fetch doesn’t abort the entire run (addressing #949).
Changes:
- Wrapes Open Collective, GitHub Sponsors, and thanks.dev fetches with per-promise
.catch()handlers duringPromise.all(). - Logs per-source fetch failures and falls back to empty sponsor/donation data for the failing source.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fetchOpenCollectiveData().catch(error => { | ||
| console.error("Failed to fetch Open Collective data.", error); | ||
| return { | ||
| sponsors: [], | ||
| donations: [], | ||
| }; | ||
| }), | ||
| fetchGitHubSponsors().catch(error => { | ||
| console.error("Failed to fetch GitHub Sponsors data.", error); | ||
| return { | ||
| sponsors: [], | ||
| donations: [], | ||
| }; | ||
| }), |
There was a problem hiding this comment.
Catching and defaulting Open Collective / GitHub Sponsors to empty data means the daily Data Fetch workflow can succeed and then commit/push src/_data/sponsors.json + donations.json with zeros if either upstream is temporarily down. That can effectively wipe sponsor/donation data on main. Consider keeping Open Collective/GitHub failures fatal (only tolerate thanks.dev), or gate the write/commit so empty fallbacks don’t overwrite existing data unless a minimum set of sources succeeded.
| fetchOpenCollectiveData().catch(error => { | |
| console.error("Failed to fetch Open Collective data.", error); | |
| return { | |
| sponsors: [], | |
| donations: [], | |
| }; | |
| }), | |
| fetchGitHubSponsors().catch(error => { | |
| console.error("Failed to fetch GitHub Sponsors data.", error); | |
| return { | |
| sponsors: [], | |
| donations: [], | |
| }; | |
| }), | |
| fetchOpenCollectiveData(), | |
| fetchGitHubSponsors(), |
There was a problem hiding this comment.
I think Copilot’s comment is valid. I tested it locally, and for example, if the GitHub Sponsors fetch request fails for some reason, it ends up erasing all GitHub Sponsors-related information from the JSON data under src/_data.
Also, in data-fetch.yml we commit that information directly to the main branch, so if an issue like #949 occurs again, the lost data could remain missing for a long time.
eslint.org/.github/workflows/data-fetch.yml
Lines 38 to 54 in 62b274e
I'm not entirely sure what the desired solution is, but at minimum I think we could preserve the existing information rather than erasing all of it.
The detailed approach for this wasn't mentioned in #949, so I'm raising it for further discussion.
There was a problem hiding this comment.
I also think we should preserve the data we already have (that was previously fetched from the now-failing source).
nzakas
left a comment
nzakas
left a comment
There was a problem hiding this comment.
Thanks for looking at this. I also agree that we should retain existing data.
In addition, we should still ideally have the CI job marked as failed after writing the file. Otherwise, we could silently be missing data for months and not know until someone logs at the logs.
|
I believe I’ve addressed all the feedback here. |
| blockedSponsors, | ||
| ] = await Promise.all([ | ||
| fetchOpenCollectiveData().catch(error => { | ||
| hadSourceFetchFailure = true; |
There was a problem hiding this comment.
Instead of setting to true, can we make this an array where we track which data sources failed? That way we can output that info into the CI directly.
Prerequisites checklist
AI acknowledgment
What is the purpose of this pull request?
This PR fixes the sponsor data fetch flow so a failure from one upstream source does not cause the entire
fetch:sponsorsscript to fail.What changes did you make? (Give an overview)
Updated
tools/fetch-sponsors.jsso the Open Collective, GitHub Sponsors, and thanks.dev fetches each handle their own failure during thePromise.all()call. If one source rejects, the script now logs the error and falls back to empty data for that source, allowing the rest of the sponsor data to still be written.Related Issues
Fixes #949
Is there anything you'd like reviewers to focus on?