fix(sync): retry failed screenshot fetches instead of caching forever#23
Merged
Merged
Conversation
Previously, a failed screenshot fetch (timeout, transient origin error, unrecognized mime type, etc.) wrote a warning placeholder directly into the same cache path that gates re-fetching, so any failure became permanent - the URL was never retried again. A single uncaught error per screenshot (e.g. mime_content_type() returning false) could also abort the entire sync run, leaving everything later in the apps list unsynced. This adds a separate cache-failed/ marker per URL with a backoff window, so failed fetches are retried periodically instead of being stuck forever, and wraps per-screenshot handling so one bad entry can't kill the whole run. Assisted-by: claude:claude-sonnet-5 Signed-off-by: Anna Larch <anna@nextcloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sync.phpwrites the failure placeholder (generateWarningImage()) into the exact same cache path thatfile_exists($cacheUrl)uses to decide "already handled, skip forever" — so any transient fetch failure (timeout, temporary origin error, unrecognized mime type) becomes permanent and is never retried.mime_content_type()returningfalse, which then failsstr_starts_with()'s type check) aborts the entire sync run, leaving every app later in the list unsynced until the same bug is hit again next run.cache-failed/marker directory that tracks failed URLs with a 6h backoff, so failures get retried instead of being stuck forever, and wraps per-screenshot handling inhandleApps()so one bad entry can't kill the whole run.Test plan
php -l sync.phppasseshandleScreenshot()logic against a real failing URL: confirmed a fail marker is created on failure, a retry within the backoff window is a no-op, and a retry after the backoff window elapses re-attempts the fetch🤖 Generated with Claude Code