fix(sync): retry failed screenshot fetches instead of caching forever

.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/

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');