[mehmetbahadirgungor]

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Bug fix

⤵️ What is the current behavior?

When a cached file is retrieved, CacheStore fires an unawaited future to update cache-hit metadata without observing errors. If SQLite fails during this write (e.g. SQLite error 14 / unable to open database file), the error escapes as an unhandled asynchronous exception, even though a valid cached file is present on disk. In addition, errors during database retrieval or timer-scheduled cleanup could escape or leave pending futures uncompleted.

🆕 What is the new behavior (if this is a feature change)?

• Errors from the cache-hit metadata update and scheduled cleanup are observed and logged via cacheLogger at warning level with stack traces, preventing unhandled asynchronous exceptions while returning the cached file.
• Failed metadata reads are logged at warning level and treated as a cache miss (null), allowing network download fallbacks to proceed cleanly when the database is unavailable.
• If persisting a newly downloaded file fails in WebHelper, the orphaned file is removed from disk.
• Pending retrieval futures clean up properly and avoid evicting newer in-flight futures.

💥 Does this PR introduce a breaking change?

No

🐛 Recommendations for testing

Run flutter test test/cache_store_test.dart, flutter test test/web_helper_test.dart, and flutter test. Unit tests verify that:

• getFile returns FileInfo without unhandled errors when the database update fails on a cache hit.
• Failed database reads are treated as cache misses and clean up pending cache.
• Scheduled cleanup errors are handled and logged without unhandled async errors.
• Downloaded files are cleaned up if putFile fails.

📝 Links to relevant issues/docs

Fixes #535

🤔 Checklist before submitting

• All projects build
• Follows style guide lines (code style guide)
• Relevant documentation was updated
• Rebased onto current main
• Version bumped and dated CHANGELOG entry added

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

[rickdijk]

8 inline comments below. The first three are the most important: unhandled errors can still escape through _cleanupCache, emptyCache/removeCachedFile can now leave orphaned DB rows, and a failed DB read now blocks the network download fallback.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

[mehmetbahadirgungor]

Thanks for the thorough and insightful review @rickdijk! All 8 points have been addressed in the latest commit:

1. Fallback on failed DB read: _getCacheDataFromDatabase now catches read errors, logs them at warning with stack traces, and returns null (cache miss) so that WebHelper can cleanly fall back to downloading files from the network.
2. Prevent emptyCache / removeCachedFile interruption: Removed await on _futureCache.remove(key) in _removeCachedFile so that pending retrieval failures never abort file deletion or database row cleanup.
3. _cleanupCache error handling: Removals started in _cleanupCache are now properly collected and awaited with await Future.wait(futures); before deleteAll, and _scheduleCleanup catches and logs errors with stack traces.
4. Clean async loader: Replaced manual Completer with a private _loadCacheData helper using try / finally and an identical(_futureCache[key], currentFuture()) guard to prevent evicting newer pending futures.
5. Stack trace logging: All warning log handlers now capture (Object e, StackTrace s) and include the stack trace.
6. Strengthened tests:
   • Replaced wall-clock delays with deterministic untilCalled and verify assertions.
   • Added verify(config.mockRepo.get(any)).called(2); ensuring the future cache does not get stuck.
   • Verified that warnings are actually logged via CacheLogger.
   • Added a test covering _scheduleCleanup failure handling.

[rickdijk]

Thanks @mehmetbahadirgungor, all 8 points from the first round are addressed. Format, analyze and the full test suite pass locally on this commit. I left two small comments and a nit below. None of them block merging: with the CHANGELOG/description update in, I'm happy to approve, and the orphaned-file case can be a follow-up.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

[mehmetbahadirgungor]

Thanks again @rickdijk! All 3 follow-up points are in:

1. Delete orphaned file on putFile error: In WebHelper._manageResponse, if _store.putFile throws after saving a new file, the newly downloaded file is now deleted before rethrowing (with a new unit test in web_helper_test.dart).
2. CHANGELOG & PR description: Added the note about treating failed metadata reads as a cache miss with a warning log, and updated the PR description's "new behavior" section accordingly.
3. Deterministic test logging: Refactored TestCacheLogger with waitForLog(Pattern) and a setUpTestLogger() helper, removing all Future.delayed calls across the tests.

[rickdijk]

Thanks @mehmetbahadirgungor, the three follow-ups look good. I've checked that the new tests fail on main and pass here, and format, analyze and the full suite pass locally. There's one last small thing below, introduced by the cleanup in the latest commit. Once that's in I'll approve, and I'll approve the CI workflow run too.

[mehmetbahadirgungor]

Thanks @rickdijk! That's a great catch. Handled in the latest commit:

In CacheStore.putFile, we now save previousCacheObject = _memCache[cacheObject.key]. If _updateCacheDataInDatabase throws:

• If a previous cache object existed in memory, it is restored (_memCache[key] = previousCacheObject).
• Otherwise, the key is evicted from memory (_memCache.remove(key)).
• The error is rethrown.

Added a unit test in cache_store_test.dart verifying that memory cache never points to an unpersisted/deleted file when database write fails, and restores previous entries when applicable.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

[rickdijk]

Thanks @mehmetbahadirgungor, this looks good. The memory-cache fix is correct, and the new test fails without it. Format, analyze and all 116 tests pass locally on this commit. Approving, and I'll merge once CI is green.