fix(fetcher): catch GenericFileException when reading the appstore cache file#891
fix(fetcher): catch GenericFileException when reading the appstore cache file#891oleksandr-nc wants to merge 1 commit into
Conversation
…che file Signed-off-by: Oleksander Piskun <oleksandr2088@icloud.com>
📝 WalkthroughWalkthroughThis PR extends cache-file error handling in 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/Fetcher/AppAPIFetcher.php (1)
154-159: ⚡ Quick winConsider capturing the deletion exception for better diagnostics.
The inner
catch (Exception)on line 156 uses PHP 8+ syntax without a variable, so the deletion failure reason is not captured. Line 157 logs the original read exception$e, which correctly identifies the root cause, but if deletion fails it would be helpful to also log why deletion failed for debugging.💡 Optional: capture and log both exceptions
} catch (GenericFileException $e) { // File exists but is unreadable (I/O or OS-level permission failure); drop it and refresh try { $file->delete(); - } catch (Exception) { - $this->logger->error('Could not read appstore cache file', ['app' => 'appstoreExAppFetcher', 'exception' => $e]); + } catch (Exception $deleteException) { + $this->logger->error('Could not read or delete appstore cache file', [ + 'app' => 'appstoreExAppFetcher', + 'read_exception' => $e, + 'delete_exception' => $deleteException, + ]); return []; }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a34e6ab-f31b-4936-a581-e9cda87874da
📒 Files selected for processing (1)
lib/Fetcher/AppAPIFetcher.php
|
CI failures are not related |
Mirrors the core fix in nextcloud/server#60286: on
GenericFileException(NC 35 now throws it when the appstore cache file can't be read) we drop the unreadable cache file and let it be refetched, returning[]cleanly if even deleting it fails.Tested on NC 35 by making the cached
appapi_apps.jsonunreadable: before,/apps/listreturns 500; after, it recovers with the full list and the cache file is recreated.