From 92c241b26edb9d407c32f34e74738b59001d116b Mon Sep 17 00:00:00 2001 From: Alex Hope-O'Connor Date: Tue, 16 Dec 2025 23:07:06 +1000 Subject: [PATCH] Improve time series endpoint error handling and remove arbitrary limits - Make time series endpoint consistent with main radar endpoint (triggers background update, provides cache status) - Add validation for large time ranges with meaningful error messages - Make time range limit configurable via TimeSeries:MaxTimeRangeHours (defaults to CacheRetentionHours) - Remove arbitrary max limits from demo app settings (frame interval, refresh interval) - Update documentation to reflect configurable limits and improved error handling --- Controllers/RadarController.cs | 91 +++++++++++++++++++++++++- README.md | 116 ++++++++++++++++++++++++++++++--- Services/BomRadarService.cs | 18 ++++- Views/RadarTest/Index.cshtml | 97 ++++++++++++++++++++++++--- appsettings.json | 4 ++ 5 files changed, 307 insertions(+), 19 deletions(-) diff --git a/Controllers/RadarController.cs b/Controllers/RadarController.cs index c428ff4..71aa007 100644 --- a/Controllers/RadarController.cs +++ b/Controllers/RadarController.cs @@ -219,11 +219,100 @@ public class RadarController : ControllerBase return BadRequest(new { error = "startTime must be before or equal to endTime" }); } + // Validate time range size to prevent excessive data loading + // Default: base limit on cache retention, but allow override via config + var cacheRetentionHours = _configuration.GetValue("CacheRetentionHours", 24); + var configuredMaxHours = _configuration.GetValue("TimeSeries:MaxTimeRangeHours"); + + // Use configured value if set, otherwise use cache retention (with minimum of 24 hours) + var maxTimeRangeHours = configuredMaxHours ?? Math.Max(cacheRetentionHours, 24); + + if (start.HasValue && end.HasValue) + { + var timeRange = end.Value - start.Value; + if (timeRange.TotalHours > maxTimeRangeHours) + { + var reason = configuredMaxHours.HasValue + ? $"configured limit: {configuredMaxHours} hours" + : $"cache retention: {cacheRetentionHours} hours"; + + return BadRequest(new { + error = $"Time range exceeds maximum allowed duration of {maxTimeRangeHours} hours (based on {reason}). Please specify a smaller range.", + requestedHours = timeRange.TotalHours, + maxHours = maxTimeRangeHours, + cacheRetentionHours = cacheRetentionHours, + configuredMaxHours = configuredMaxHours + }); + } + } + + // Check if location has any cache at all (to distinguish from "no data in range") + var cacheRange = await _bomRadarService.GetCacheRangeAsync(suburb, state, cancellationToken); + var locationHasCache = cacheRange.TotalCacheFolders > 0; + + // If no cache exists for this location, trigger background update and return detailed 404 + if (!locationHasCache) + { + // Trigger background cache update (non-blocking) + _ = Task.Run(async () => + { + try + { + await _bomRadarService.TriggerCacheUpdateAsync(suburb, state, CancellationToken.None); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Background cache update failed for {Suburb}, {State}", suburb, state); + } + }); + + // Get cache status to include in 404 response + var cacheExpirationMinutes = (int)_configuration.GetValue("CacheExpirationMinutes", 15.5); + var cacheManagementCheckIntervalMinutes = _configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); + var cacheStatus = await _cacheService.GetCacheStatusAsync( + suburb, + state, + CachedDataType.Radar, + cacheExpirationMinutes, + cacheManagementCheckIntervalMinutes, + cancellationToken); + + return NotFound(new { + error = "No cached data found for this location. Cache update has been triggered in background. Please retry in a few moments.", + retryAfter = 30, // seconds + refreshEndpoint = $"/api/cache/{suburb}/{state}/refresh", + updateTriggered = cacheStatus.UpdateTriggered, + cacheExists = cacheStatus.CacheExists, + cacheIsValid = cacheStatus.CacheIsValid, + cacheExpiresAt = cacheStatus.CacheExpiresAt, + nextUpdateTime = cacheStatus.NextUpdateTime, + message = cacheStatus.Message + }); + } + + // Location has cache, check if there's data in the requested time range var result = await _bomRadarService.GetRadarTimeSeriesAsync(suburb, state, start, end, cancellationToken); if (result.CacheFolders.Count == 0) { - return NotFound(new { error = "No historical data found for the specified time range." }); + // Location has cache, but no data in the requested time range + var oldestCache = cacheRange.OldestCache?.CacheTimestamp; + var newestCache = cacheRange.NewestCache?.CacheTimestamp; + + return NotFound(new { + error = "No historical data found for the specified time range.", + availableRange = new { + oldest = oldestCache, + newest = newestCache, + totalCacheFolders = cacheRange.TotalCacheFolders, + timeSpanMinutes = cacheRange.TimeSpanMinutes + }, + requestedRange = new { + start = start, + end = end + }, + suggestion = "Try adjusting the time range to match the available cached data." + }); } return Ok(result); diff --git a/README.md b/README.md index 44b2fb5..0924520 100644 --- a/README.md +++ b/README.md @@ -198,7 +198,7 @@ All configuration can be done via environment variables, which override the defa | Variable | Description | Default | Example | |----------|-------------|---------|---------| | `CACHEDIRECTORY` | Directory path for cache storage | `/app/cache` | `/data/bom-cache` | -| `CACHERETENTIONHOURS` | Hours to retain cached data before cleanup | `24` | `48` | +| `CACHERETENTIONHOURS` | Hours to retain cached data before cleanup (can be any positive integer) | `24` | `48`, `72`, `168` (1 week) | | `CACHEEXPIRATIONMINUTES` | Minutes before cache is considered expired | `12.5` | `15` | | `TIMEZONE` | Timezone for time parsing (IANA format) | `Australia/Brisbane` | `Australia/Sydney` | @@ -231,6 +231,13 @@ All configuration can be done via environment variables, which override the defa | `DEBUG__ENABLED` | Enable debug mode (saves debug screenshots) | `false` | `true` | | `DEBUG__WAITMS` | Additional wait time in debug mode | `2000` | `5000` | +#### Time Series Configuration + +| Variable | Description | Default | Example | +|----------|-------------|---------|---------| +| `TIMESERIES__WARNINGFOLDERCOUNT` | Number of cache folders that triggers a warning log when processing time series requests | `200` | `300` | +| `TIMESERIES__MAXTIMERANGEHOURS` | Maximum time range allowed for time series queries (null = use CacheRetentionHours) | `null` (uses CacheRetentionHours) | `72` | + #### Docker Compose Port Configuration | Variable | Description | Default | @@ -399,7 +406,7 @@ GET /api/radar/{suburb}/{state}/timeseries?startTime={iso8601}&endTime={iso8601} - `startTime` (query, optional): ISO 8601 start time (e.g., `2025-01-15T00:00:00Z`) - `endTime` (query, optional): ISO 8601 end time (defaults to now) -**Response:** +**Response (200 OK):** ```json { "cacheFolders": [ @@ -417,11 +424,55 @@ GET /api/radar/{suburb}/{state}/timeseries?startTime={iso8601}&endTime={iso8601} ] } ], - "totalFrames": 7, - "timeSpanMinutes": 60 + "startTime": "2025-01-15T07:00:00Z", + "endTime": "2025-01-15T10:00:00Z", + "totalFrames": 7 } ``` +**Status Codes:** +- `200 OK`: Historical data available +- `400 Bad Request`: Invalid request (e.g., time range exceeds maximum allowed duration, invalid time format, startTime after endTime) +- `404 Not Found`: + - **Location not cached**: No cache exists for this location. Cache update is triggered in background. Response includes cache status: + ```json + { + "error": "No cached data found for this location. Cache update has been triggered in background. Please retry in a few moments.", + "retryAfter": 30, + "refreshEndpoint": "/api/cache/Brisbane/QLD/refresh", + "updateTriggered": true, + "cacheExists": false, + "cacheIsValid": false, + "cacheExpiresAt": null, + "nextUpdateTime": "2025-01-15T10:12:30Z", + "message": "No cache exists, update triggered" + } + ``` + - **No data in range**: Cache exists but no data in the requested time range. Response includes available cache range: + ```json + { + "error": "No historical data found for the specified time range.", + "availableRange": { + "oldest": "2025-01-15T08:00:00Z", + "newest": "2025-01-15T10:00:00Z", + "totalCacheFolders": 10, + "timeSpanMinutes": 120 + }, + "requestedRange": { + "start": "2025-01-15T00:00:00Z", + "end": "2025-01-15T10:00:00Z" + }, + "suggestion": "Try adjusting the time range to match the available cached data." + } + ``` + +**Time Range Limits:** +- Maximum time range is configurable via `TimeSeries:MaxTimeRangeHours` (defaults to `CacheRetentionHours` or minimum 24 hours) +- If `TimeSeries:MaxTimeRangeHours` is not set, the limit automatically matches your `CacheRetentionHours` setting +- This ensures you can always query all available cached data (e.g., if retention is 72 hours, you can query up to 72 hours) +- If no time range is specified, returns all available historical data +- **Note**: `CacheRetentionHours` can be set to any positive integer value (24, 48, 72, 168, etc.) + #### Get Cache Range Get information about available historical cache data. @@ -513,11 +564,11 @@ http://localhost:8082/radar/Brisbane/QLD - **Slideshow Playback**: Play, pause, and navigate through radar frames - **Frame Navigation**: Use slider, buttons, or keyboard shortcuts (arrow keys, spacebar) -- **Extended Timespans**: View historical data from 1 hour to 24 hours +- **Extended Timespans**: View historical data with configurable time ranges (based on cache retention settings) - **Custom Time Ranges**: Select specific start and end times for historical viewing -- **Auto-Refresh**: Automatically checks for new data at configurable intervals +- **Auto-Refresh**: Automatically checks for new data at configurable intervals (minimum 5 seconds, no maximum) - **Cache Status**: Real-time display of cache validity, expiration, and update status -- **Settings Panel**: Configure frame intervals, refresh rates, and playback options +- **Settings Panel**: Configure frame intervals (minimum 0.1 seconds, no maximum), refresh rates, and playback options ### Keyboard Shortcuts @@ -580,6 +631,33 @@ async function getHistoricalRadar(suburb, state, hoursBack = 3) { `/api/radar/${suburb}/${state}/timeseries?startTime=${startTime.toISOString()}&endTime=${endTime.toISOString()}` ); + if (response.status === 400) { + const error = await response.json(); + // Error will include maxHours and cacheRetentionHours for context + throw new Error(error.error || 'Invalid time range request'); + } + + if (response.status === 404) { + const error = await response.json(); + + // Check if location doesn't exist (cache update triggered) + if (error.updateTriggered !== undefined || error.cacheExists !== undefined) { + // Trigger cache update if endpoint provided + if (error.refreshEndpoint) { + fetch(error.refreshEndpoint, { method: 'POST' }).catch(() => {}); + } + throw new Error(`No cache found. ${error.message || 'Cache update triggered, please retry in a few moments.'}`); + } + + // Cache exists but no data in range + if (error.availableRange) { + const range = error.availableRange; + throw new Error(`${error.error} Available data: ${range.oldest} to ${range.newest}. ${error.suggestion || ''}`); + } + + throw new Error(error.error || 'No historical data found'); + } + if (!response.ok) { throw new Error(`HTTP ${response.status}`); } @@ -656,7 +734,29 @@ curl -X POST http://localhost:8082/api/cache/Brisbane/QLD/refresh **Get historical data (last 3 hours):** ```bash -curl "http://localhost:8082/api/radar/Brisbane/QLD/timeseries?startTime=2025-01-15T07:00:00Z" +curl "http://localhost:8082/api/radar/Brisbane/QLD/timeseries?startTime=2025-01-15T07:00:00Z&endTime=2025-01-15T10:00:00Z" +``` + +**Get historical data with error handling:** +```bash +# Check response status +response=$(curl -s -w "\n%{http_code}" "http://localhost:8082/api/radar/Brisbane/QLD/timeseries?startTime=2025-01-15T07:00:00Z&endTime=2025-01-15T10:00:00Z") +http_code=$(echo "$response" | tail -n1) +body=$(echo "$response" | sed '$d') + +if [ "$http_code" = "200" ]; then + echo "Success: $body" +elif [ "$http_code" = "400" ]; then + echo "Bad Request: $body" +elif [ "$http_code" = "404" ]; then + echo "Not Found: $body" + # Check if cache update was triggered + if echo "$body" | grep -q "updateTriggered"; then + echo "Cache update triggered, retry in a few moments" + fi +else + echo "Error ($http_code): $body" +fi ``` ## Troubleshooting diff --git a/Services/BomRadarService.cs b/Services/BomRadarService.cs index 9a3d100..4c19ef2 100644 --- a/Services/BomRadarService.cs +++ b/Services/BomRadarService.cs @@ -15,6 +15,7 @@ public class BomRadarService : IBomRadarService, IDisposable private readonly IConfiguration _configuration; private readonly double _cacheExpirationMinutes; private readonly int _cacheManagementCheckIntervalMinutes; + private readonly int _timeSeriesWarningFolderCount; public BomRadarService( ILogger logger, @@ -32,6 +33,7 @@ public class BomRadarService : IBomRadarService, IDisposable _configuration = configuration; _cacheExpirationMinutes = configuration.GetValue("CacheExpirationMinutes", 12.5); _cacheManagementCheckIntervalMinutes = configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); + _timeSeriesWarningFolderCount = configuration.GetValue("TimeSeries:WarningFolderCount", 200); if (_cacheExpirationMinutes <= 0) { @@ -41,6 +43,10 @@ public class BomRadarService : IBomRadarService, IDisposable { throw new ArgumentException("CacheManagement:CheckIntervalMinutes must be between 1 and 60", nameof(configuration)); } + if (_timeSeriesWarningFolderCount <= 0) + { + throw new ArgumentException("TimeSeries:WarningFolderCount must be greater than 0", nameof(configuration)); + } } public async Task GetCachedRadarAsync(string suburb, string state, CancellationToken cancellationToken = default) @@ -429,6 +435,16 @@ public class BomRadarService : IBomRadarService, IDisposable filteredFolders = filteredFolders.Where(f => f.CacheTimestamp <= endTime.Value); } + var filteredFoldersList = filteredFolders.ToList(); + + // Warn if processing a large number of folders (configurable threshold) + if (filteredFoldersList.Count > _timeSeriesWarningFolderCount) + { + _logger.LogWarning( + "Processing large number of cache folders ({Count}) for time series request. Suburb: {Suburb}, State: {State}, StartTime: {StartTime}, EndTime: {EndTime}", + filteredFoldersList.Count, suburb, state, startTime, endTime); + } + var result = new List(); var encodedSuburb = Uri.EscapeDataString(suburb); var encodedState = Uri.EscapeDataString(state); @@ -437,7 +453,7 @@ public class BomRadarService : IBomRadarService, IDisposable // Use a HashSet to track unique absolute observation times we've already seen var seenAbsoluteTimes = new HashSet(); - foreach (var folderInfo in filteredFolders.OrderByDescending(f => f.CacheTimestamp)) + foreach (var folderInfo in filteredFoldersList.OrderByDescending(f => f.CacheTimestamp)) { // Load frames from this folder's radar subfolder var cachedFrames = await _cacheService.GetFramesFromCacheFolderAsync( diff --git a/Views/RadarTest/Index.cshtml b/Views/RadarTest/Index.cshtml index 3034efa..6047f1c 100644 --- a/Views/RadarTest/Index.cshtml +++ b/Views/RadarTest/Index.cshtml @@ -837,8 +837,8 @@
- -
Time between frames in slideshow
+ +
Time between frames in slideshow (minimum 0.1 seconds)
@@ -847,8 +847,8 @@
- -
How often to check for new radar data
+ +
How often to check for new radar data (minimum 5 seconds)
@@ -860,7 +860,7 @@ -
Select custom time range (times are in local timezone)
+
Select custom time range (times are in local timezone). Maximum range is based on your cache retention settings.
@@ -1121,8 +1121,61 @@ }); if (!response.ok) { - const error = await response.json().catch(() => ({ error: `HTTP ${response.status}` })); - throw new Error(error.error || 'Failed to fetch historical radar'); + const errorData = await response.json().catch(() => ({ error: `HTTP ${response.status}` })); + + // Handle 400 Bad Request (e.g., time range too large) + if (response.status === 400) { + const errorMessage = errorData.error || 'Invalid request'; + let details = ''; + if (errorData.requestedHours && errorData.maxHours) { + details = ` Requested: ${errorData.requestedHours.toFixed(1)} hours, Maximum: ${errorData.maxHours} hours.`; + if (errorData.cacheRetentionHours) { + details += ` (Limit is based on cache retention: ${errorData.cacheRetentionHours} hours)`; + } + if (errorData.configuredMaxHours) { + details += ` (Override configured: ${errorData.configuredMaxHours} hours)`; + } + } + throw new Error(errorMessage + details); + } + + // Handle 404 Not Found + if (response.status === 404) { + // Check if this is "location doesn't exist" vs "no data in range" + if (errorData.updateTriggered !== undefined || errorData.cacheExists !== undefined) { + // Location doesn't exist - trigger cache update if endpoint provided + if (errorData.refreshEndpoint) { + fetch(errorData.refreshEndpoint, { + method: 'POST', + signal: AbortSignal.timeout(5000) + }).catch(err => { + console.debug('Background cache refresh trigger failed (non-critical):', err); + }); + } + + // Return special error code to indicate location missing + return { + frames: null, + error: 'location_missing', + cacheStatus: errorData + }; + } else if (errorData.availableRange) { + // Cache exists but no data in requested range + const availableRange = errorData.availableRange; + const rangeMessage = availableRange.oldest && availableRange.newest ? + ` Available data: ${formatDate(availableRange.oldest)} to ${formatDate(availableRange.newest)}.` : + availableRange.totalCacheFolders > 0 ? + ` ${availableRange.totalCacheFolders} cache folders available.` : ''; + + throw new Error((errorData.error || 'No data in requested range') + rangeMessage); + } else { + // Generic 404 + throw new Error(errorData.error || 'No historical data found'); + } + } + + // Other errors + throw new Error(errorData.error || 'Failed to fetch historical radar'); } const data = await response.json(); @@ -1282,6 +1335,29 @@ throw new Error('Network error - API unavailable'); } + // Handle location missing case (cache update triggered) + if (result.error === 'location_missing') { + const cacheStatus = result.cacheStatus || {}; + const isUpdating = cacheStatus.updateTriggered === true || + (cacheStatus.message && cacheStatus.message.includes('in progress')); + + // Create a data-like object with cache status for updateUI + const cacheStatusData = { + frames: [], + isUpdating: isUpdating, + cacheIsValid: cacheStatus.cacheIsValid || false, + cacheExpiresAt: cacheStatus.cacheExpiresAt || null, + nextUpdateTime: cacheStatus.nextUpdateTime || null + }; + updateUI(cacheStatusData); + + const retryMessage = cacheStatus.retryAfter ? + ` Please retry in ${cacheStatus.retryAfter} seconds.` : + ' Please retry in a few moments.'; + showNoFramesMessage('No cached data found for this location. Cache update has been triggered in background.' + retryMessage); + return null; + } + const allFrames = result.frames; if (!allFrames || allFrames.length === 0) { showNoFramesMessage('No frames found for selected timespan. Try a different range or wait for more cache data.'); @@ -2126,8 +2202,11 @@ settingsBtn.addEventListener('click', showSettings); } document.getElementById('save-settings-btn').addEventListener('click', () => { - settings.frameInterval = parseFloat(document.getElementById('frame-interval-input').value) || 2.0; - settings.refreshInterval = parseInt(document.getElementById('refresh-interval-input').value) || 30; + const frameIntervalValue = parseFloat(document.getElementById('frame-interval-input').value); + settings.frameInterval = (frameIntervalValue > 0) ? frameIntervalValue : 2.0; + + const refreshIntervalValue = parseInt(document.getElementById('refresh-interval-input').value); + settings.refreshInterval = (refreshIntervalValue >= 5) ? refreshIntervalValue : 30; settings.autoPlay = document.getElementById('auto-play-input').checked; settings.timespan = document.getElementById('timespan-select').value; diff --git a/appsettings.json b/appsettings.json index 39da934..63be0d2 100644 --- a/appsettings.json +++ b/appsettings.json @@ -43,5 +43,9 @@ "Radar": { "FrameCount": 7 } + }, + "TimeSeries": { + "WarningFolderCount": 200, + "MaxTimeRangeHours": null } }