Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle enrichment failure gracefully #10

Closed
wants to merge 2 commits into from

Conversation

tabokie
Copy link
Contributor

@tabokie tabokie commented May 3, 2021

Finish the build even when failing to enrich certain feeds.

@chuanqisun
Copy link
Collaborator

Hey @tabokie, thank you for jumping in to help. I'll merge #9 and close this one for now, because I need to do some additional work before the partially successful enrichment can continue.

If we don't stop a partially successful enrichment, the cache will be replaced with the partial output, this might cause some data loss if user accidentally add a bad url to the source list. I think with your change from #9, showing user where the error is and let them fix the broken source is the best strategy.

@chuanqisun chuanqisun closed this May 4, 2021
@tabokie
Copy link
Contributor Author

tabokie commented May 4, 2021

@chuanqisun Thanks for pointing that out, though I don't think the failed sources will continue to replace cache since I add a filter for the final enrichment output, which effectively stops them from calling setCache.
I'm not so sure about the "mute failure" strategy in this PR either, but I do believe it's a bit confusing for the user to see GitHub action passes while the webpage is 404. Maybe we could do better to allow failure but print an error message in the front-end, or throw out the failure to GitHub action.

@chuanqisun
Copy link
Collaborator

chuanqisun commented May 4, 2021

Love the idea to surface it in the frontend! I should have designed the error handling better. The problem is the setCache is not aware of which sources are present. If a broken source is filtered out, the setCache will basically delete that source from the existing cache, which leads to data loss.

I've created a separate issue to track error handling. I'd love to surface it to the UI so user can immediately know if something is broken. Moving to discussion #18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants