Fix Connect.Koi auto-detect throws DateTimeOffset exception when no koi.json exists in default DNN skin#18
Fix Connect.Koi auto-detect throws DateTimeOffset exception when no koi.json exists in default DNN skin#18tvatavuk wants to merge 1 commit into2sic:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a DateTimeOffset exception that occurs when HostFileChangeMonitor is initialized with a non-existent koi.json file in DNN skins. The fix adds proper file existence checks and fallback cache expiration strategies.
Key Changes:
- Added file existence check before attempting to monitor the
koi.jsonfile for changes - Implemented time-based cache expiration (5 minutes) as a fallback when the file doesn't exist or monitoring fails
- Improved cache key generation by using
ToLowerInvariant()instead ofToLower()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| policy.AbsoluteExpiration = DateTimeOffset.UtcNow.AddMinutes(5); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| // Re-check occasionally in case koi.json gets created later. | ||
| policy.AbsoluteExpiration = DateTimeOffset.UtcNow.AddMinutes(5); |
There was a problem hiding this comment.
The cache expiration time of 5 minutes is duplicated on lines 62 and 68. Consider extracting this magic number to a private constant (e.g., CacheFallbackExpirationMinutes) to improve maintainability and make it easier to adjust this value consistently in the future.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
ATM I'm not sure if this is something we want to support, as it doesn't really fix anything. This method doesn't need the restart, but it will "randomly start working" after the specified time, so I feel like it's not doing much, and could confuse people even further. |
Fixed #17