Skip to content

Conversation

@ildyria
Copy link
Member

@ildyria ildyria commented Jan 3, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved cache event logging configuration handling to gracefully manage errors during configuration retrieval. The system now handles configuration failures smoothly, preventing disruptions to cache monitoring and enhancing overall system resilience.

✏️ Tip: You can customize this high-level summary in your review settings.

@ildyria ildyria requested a review from a team as a code owner January 3, 2026 22:12
@coderabbitai
Copy link

coderabbitai bot commented Jan 3, 2026

📝 Walkthrough

Walkthrough

The config check for cache event logging is now wrapped in a try-catch block to gracefully handle potential exceptions during configuration retrieval, suppressing logging and returning early if an error occurs instead of propagating the exception.

Changes

Cohort / File(s) Change Summary
Error Handling Enhancement
app/Listeners/CacheListener.php
Wraps getValueAsBool('cache_event_logging') config check in try-catch block to gracefully handle config retrieval exceptions by suppressing logging and returning early

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 When configs might stumble and fail,
Our listener catches them without pale,
With try-catch held tight like a clover so green,
Errors are handled—no crashes to scene! 🌿

Pre-merge checks

✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f021a29 and a802437.

📒 Files selected for processing (1)
  • app/Listeners/CacheListener.php
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.php: Any new PHP file should contain the license header and have a single blank line after the opening PHP tag
Variable names should be in snake_case in PHP
Apply the PSR-4 coding standard in PHP
Use in_array() with true as the third parameter in PHP
Only use booleans in if statements, not integers or strings
Use strict comparison (===) instead of loose comparison (==)
Avoid code duplication in both if and else statements
Do not use empty() in PHP
Use the moneyphp/money library for handling monetary values in PHP
Never use floats or doubles to represent monetary values; use integers representing the smallest currency unit (e.g., cents for USD)

**/*.php: Write or extend executable specifications (unit, behaviour, or scenario tests) ahead of implementation, confirm they fail, and then drive code to green before refactoring. List the expected success, validation, and failure branches and add thin failing tests for each path.
For PHP code, adhere to conventions: license headers in new files, strict comparison (===), no empty(), in_array() with third parameter true, snake_case variables, PSR-4 standard, test base classes (AbstractTestCase for Unit, BaseApiWithDataTest for Feature_v2).
Always run phpunit tests. If a test remains red, disable it with a TODO, note the reason, and capture the follow-up in the relevant plan.
Spotless now uses Palantir Java Format 2.78.0 with a 120-character wrap; configure IDE formatters to match before pushing code changes.
Keep each increment's control flow flat by delegating validation/normalisation into tiny pure helpers that return simple enums or result records, then compose them instead of introducing inline branching that inflates the branch count per change.
When introducing new helpers/utilities or editing files prone to style violations (records, DTOs, generated adapters), run the narrowest applicable lint target (for example phpstan) before the full pipeline. Note the command in the related plan/task.
For PHP changes, ru...

Files:

  • app/Listeners/CacheListener.php
🧠 Learnings (4)
📓 Common learnings
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3774
File: database/migrations/2025_10_28_214205_bump_version060904.php:30-41
Timestamp: 2025-10-28T21:52:21.192Z
Learning: In Lychee version bump migrations (database/migrations), cache clearing failures should not prevent the migration from succeeding. The migration catches exceptions from `Artisan::call('cache:clear')`, logs a warning, and returns early instead of throwing the exception, allowing the version update to complete successfully.
📚 Learning: 2025-10-28T21:52:21.192Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3774
File: database/migrations/2025_10_28_214205_bump_version060904.php:30-41
Timestamp: 2025-10-28T21:52:21.192Z
Learning: In Lychee version bump migrations (database/migrations), cache clearing failures should not prevent the migration from succeeding. The migration catches exceptions from `Artisan::call('cache:clear')`, logs a warning, and returns early instead of throwing the exception, allowing the version update to complete successfully.

Applied to files:

  • app/Listeners/CacheListener.php
📚 Learning: 2025-12-28T18:12:55.752Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3901
File: app/Providers/AppServiceProvider.php:0-0
Timestamp: 2025-12-28T18:12:55.752Z
Learning: When using Laravel Octane's tick API, Octane::tick(...) returns an InvokeTickCallable that only has ->seconds(int) and ->immediate() methods. There is no ->every(N) method. Use the correct usage: Octane::tick('name', fn() => ...)->seconds(N) or Octane::tick('name', fn() => ..., N). Apply this guideline to PHP files across the project (not just AppServiceProvider.php).

Applied to files:

  • app/Listeners/CacheListener.php
📚 Learning: 2026-01-03T12:36:02.514Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3922
File: app/Listeners/RecomputeAlbumSizeOnPhotoMutation.php:63-69
Timestamp: 2026-01-03T12:36:02.514Z
Learning: In Lychee, treat a PhotoDeleted event with a specific album_id as removing the photo from that album only (not deleting the photo globally). For any listener handling photo mutations, when such an event occurs, recompute the size/statistics for only the affected album, because the photo may remain in other albums due to the many-to-many relationship. Ensure no global deletion logic is triggered and that only the targeted album’s statistics are refreshed.

Applied to files:

  • app/Listeners/CacheListener.php
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: 1️⃣ PHP tests / 8.5 - sqlite -- Feature_v2
  • GitHub Check: 1️⃣ PHP tests / 8.5 - mariadb -- Feature_v2
  • GitHub Check: 1️⃣ PHP tests / 8.5 - postgresql -- Feature_v2
🔇 Additional comments (1)
app/Listeners/CacheListener.php (1)

34-41: Good defensive approach to handle missing config during setup.

The try-catch wrapper prevents the listener from crashing when the cache configuration doesn't exist yet (e.g., during initial setup). This aligns with the project's pattern of graceful cache failure handling.

The code correctly catches \Exception, which is appropriate here. ConfigManager::getValueAsBool() throws ConfigurationKeyMissingException when the config key doesn't exist during initial database setup. Catching \Exception (rather than the broader Throwable) is the right choice for a listener, as it avoids catching PHP errors that shouldn't normally occur and maintains a clear separation between expected exceptions and unexpected errors.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ildyria
Copy link
Member Author

ildyria commented Jan 3, 2026

@coderabbitai review ?

@coderabbitai
Copy link

coderabbitai bot commented Jan 3, 2026

@ildyria: I will review the changes in this PR.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@codecov
Copy link

codecov bot commented Jan 3, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.09%. Comparing base (219eb90) to head (a802437).
⚠️ Report is 3 commits behind head on master.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ildyria ildyria merged commit 1b64eb9 into master Jan 3, 2026
42 checks passed
@ildyria ildyria deleted the fix-cache-config-not-existing-yet branch January 3, 2026 22:58
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.

2 participants