From a047504a1ca888dcb8c9888819cf65e23c0cead1 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Mon, 20 Jul 2026 14:27:12 +0200 Subject: [PATCH] fix(sync): retry failed screenshot fetches instead of caching forever 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 --- .gitignore | 4 ++ cache-failed/.gitkeep | 0 sync.php | 152 +++++++++++++++++++++++++++--------------- 3 files changed, 104 insertions(+), 52 deletions(-) create mode 100644 cache-failed/.gitkeep diff --git a/.gitignore b/.gitignore index 8bbcb13..ea821a9 100644 --- a/.gitignore +++ b/.gitignore @@ -5,4 +5,8 @@ /cache !/cache/.gitkeep +# ignore all failed-fetch retry markers but not the directory itself +/cache-failed +!/cache-failed/.gitkeep + /vendor/ diff --git a/cache-failed/.gitkeep b/cache-failed/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/sync.php b/sync.php index 465b077..ffef252 100644 --- a/sync.php +++ b/sync.php @@ -28,6 +28,7 @@ const MAX_SCREENSHOT_SIZE = 2 * 1024 * 1024; // 2 MiB const HTTP_TIMEOUT_S = 30; +const RETRY_BACKOFF_S = 6 * 3600; // don't retry a failed fetch more often than this /** * @param string $cacheUrl path to screenshot in cache @@ -55,71 +56,118 @@ function generateWarningImage(string $cacheUrl, string $url, string $message): v } /** - * @param array $apps decoded JSON from appstore + * Marks a screenshot fetch as failed, so it gets retried later instead of being + * treated as permanently done, and renders a placeholder in the meantime. + * + * @param string $cacheUrl path to screenshot in cache + * @param string $failMarker path to the retry marker for this screenshot + * @param string $url HTTP URL of screenshot + * @param string $message warning message to display */ -function handleApps(array $apps): void { - $validator = new UrlValidator(); +function markFailedFetch(string $cacheUrl, string $failMarker, string $url, string $message): void { + touch($failMarker); + generateWarningImage($cacheUrl, $url, $message); +} - foreach ($apps as $app) { - foreach ($app['screenshots'] as $screenshot) { - $url = $screenshot['url']; - $trimmedUrl = trim($url); +/** + * @param UrlValidator $validator + * @param array $screenshot decoded JSON of a single screenshot entry + */ +function handleScreenshot(UrlValidator $validator, array $screenshot): void { + $url = $screenshot['url']; + $trimmedUrl = trim($url); - if (!str_starts_with($trimmedUrl, 'https://')) { - continue; - } + if (!str_starts_with($trimmedUrl, 'https://')) { + return; + } - $cacheUrl = __DIR__ . '/cache/' . base64_encode($url); + $cacheUrl = __DIR__ . '/cache/' . base64_encode($url); + $failMarker = __DIR__ . '/cache-failed/' . base64_encode($url); - if (file_exists($cacheUrl)) { - continue; - } + if (file_exists($cacheUrl)) { + if (!file_exists($failMarker)) { + // Already fetched successfully, nothing to do + return; + } - try { - $validator->validate($url); - } catch (UrlValidationException $e) { - generateWarningImage($cacheUrl, $url, 'Failed to fetch image'); - continue; - } + if ((time() - filemtime($failMarker)) < RETRY_BACKOFF_S) { + // Fetch failed recently, don't hammer the origin - try again later + return; + } - $ctx = stream_context_create([ - 'http' => [ - 'timeout' => HTTP_TIMEOUT_S, - 'max_redirects' => 3, - 'user_agent' => 'nextcloud-usercontent-sync/1.0', - ], - ]); + // Backoff has elapsed, fall through and retry the fetch + } - $data = @file_get_contents($trimmedUrl, false, $ctx, 0, MAX_SCREENSHOT_SIZE + 1); + if (!is_dir(dirname($failMarker))) { + mkdir(dirname($failMarker), 0755, true); + } - if ($data === false) { - generateWarningImage($cacheUrl, $url, 'Failed to fetch image'); - continue; - } + try { + $validator->validate($url); + } catch (UrlValidationException $e) { + markFailedFetch($cacheUrl, $failMarker, $url, 'Failed to fetch image'); + return; + } - if (strlen($data) > MAX_SCREENSHOT_SIZE) { - generateWarningImage($cacheUrl, $url, 'Image exceeds file size limit'); - continue; - } + $ctx = stream_context_create([ + 'http' => [ + 'timeout' => HTTP_TIMEOUT_S, + 'max_redirects' => 3, + 'user_agent' => 'nextcloud-usercontent-sync/1.0', + ], + ]); - $tempUrl = tempnam(sys_get_temp_dir(), 'screenshot'); - if ($tempUrl === false) { - // This should never happen, but just in case, treat this as an internal error - // Do nothing for now and try again later - continue; - } + $data = @file_get_contents($trimmedUrl, false, $ctx, 0, MAX_SCREENSHOT_SIZE + 1); + + if ($data === false) { + markFailedFetch($cacheUrl, $failMarker, $url, 'Failed to fetch image'); + return; + } - file_put_contents($tempUrl, $data); + if (strlen($data) > MAX_SCREENSHOT_SIZE) { + markFailedFetch($cacheUrl, $failMarker, $url, 'Image exceeds file size limit'); + return; + } - $mimeType = mime_content_type($tempUrl); - if (!str_starts_with($mimeType, 'image/')) { - unlink($tempUrl); - generateWarningImage($cacheUrl, $url, 'Image not recognized'); - continue; - } + $tempUrl = tempnam(sys_get_temp_dir(), 'screenshot'); + if ($tempUrl === false) { + // This should never happen, but just in case, treat this as an internal error + // Do nothing for now and try again later + return; + } + + file_put_contents($tempUrl, $data); - rename($tempUrl, $cacheUrl); - echo(sprintf("Synced url %s\n", $url)); + $mimeType = mime_content_type($tempUrl); + if ($mimeType === false || !str_starts_with($mimeType, 'image/')) { + unlink($tempUrl); + markFailedFetch($cacheUrl, $failMarker, $url, 'Image not recognized'); + return; + } + + rename($tempUrl, $cacheUrl); + + if (file_exists($failMarker)) { + unlink($failMarker); + } + + echo(sprintf("Synced url %s\n", $url)); +} + +/** + * @param array $apps decoded JSON from appstore + */ +function handleApps(array $apps): void { + $validator = new UrlValidator(); + + foreach ($apps as $app) { + foreach ($app['screenshots'] as $screenshot) { + try { + handleScreenshot($validator, $screenshot); + } catch (\Throwable $e) { + // A single broken screenshot must never abort the whole sync run + echo(sprintf("Error while syncing %s: %s\n", $screenshot['url'] ?? '?', $e->getMessage())); + } } } } @@ -135,7 +183,7 @@ function handleApps(array $apps): void { } $apps = json_decode($json, true); - handleApps($apps); + handleApps($apps); } $json = file_get_contents('https://apps.nextcloud.com/api/v1/appapi_apps.json');