diff --git a/DEVELOPER.md b/DEVELOPER.md index 00bb23d..17c480b 100644 --- a/DEVELOPER.md +++ b/DEVELOPER.md @@ -297,10 +297,11 @@ These are internal app tags, not OSM tags. The underscore prefix makes this expl - **Sandbox (OSM API)**: Zoom ≥ 13 (stricter due to bbox limits) **Smart error handling & splitting:** -- **50k node limit**: Automatically splits query into 4 quadrants, recursively up to 3 levels deep -- **Timeout errors**: Also triggers splitting (dense areas with many profiles) -- **Rate limiting**: Extended backoff (30s), no splitting (would make it worse) -- **Surgical detection**: Only splits on actual limit errors, not network issues +- **50k node limit**: Automatically splits query into 4 quadrants, recursively up to 3 levels deep. This is a deterministic result-set size limit, so shrinking the queried area is the correct fix. +- **Timeout errors**: Treated as a plain retryable network error — no splitting. Timeouts usually indicate server load or an expensive query, not "too much area", so firing off more sub-requests would only add load to an already-struggling server. +- **Rate limiting (HTTP 429)**: Extended backoff (30s), no splitting (would make it worse). Still surfaces the same "slow" status indicator as the node-limit-splitting case, so the user sees consistent feedback either way. +- **Surgical detection**: Only splits on the actual 50k node limit, not timeouts or other network issues. + **Query optimization & deduplication:** - **Pre-fetch limit**: 4x user's display limit (e.g., 1000 nodes for 250 display limit) diff --git a/lib/dev_config.dart b/lib/dev_config.dart index 0d16a25..9fe96d6 100644 --- a/lib/dev_config.dart +++ b/lib/dev_config.dart @@ -99,13 +99,10 @@ const Duration kMarkerTapTimeout = Duration(milliseconds: 250); const Duration kMapLongPressTimeout = Duration(milliseconds: 600); // Duration to trigger "add node here" on empty map area const Duration kDebounceCameraRefresh = Duration(milliseconds: 500); -// Pre-fetch area configuration -const double kPreFetchAreaExpansionMultiplier = 3.0; // Expand visible bounds by this factor for pre-fetching const double kNodeRenderingBoundsExpansion = 1.3; // Expand visible bounds by this factor for node rendering to prevent edge blinking const double kRouteProximityThresholdMeters = 500.0; // Distance threshold for determining if user is near route when resuming navigation const double kResumeNavigationZoomLevel = 16.0; // Zoom level when resuming navigation -const int kPreFetchZoomLevel = 10; // Always pre-fetch at this zoom level for consistent area sizes -const int kMaxPreFetchSplitDepth = 3; // Maximum recursive splits when hitting Overpass node limit + // Data refresh configuration const int kDataRefreshIntervalSeconds = 60; // Refresh cached data after this many seconds diff --git a/lib/services/map_data_provider.dart b/lib/services/map_data_provider.dart index 6f5e99b..89ca2bf 100644 --- a/lib/services/map_data_provider.dart +++ b/lib/services/map_data_provider.dart @@ -54,8 +54,8 @@ class MapDataProvider { required List profiles, UploadMode uploadMode = UploadMode.production, int maxResults = 0, // 0 = no limit for offline downloads - int maxTries = 3, }) async { + if (AppState.instance.offlineMode) { throw OfflineModeException("Cannot fetch remote nodes for offline area download in offline mode."); } diff --git a/lib/services/node_data_manager.dart b/lib/services/node_data_manager.dart index 2dbaeeb..c4e1603 100644 --- a/lib/services/node_data_manager.dart +++ b/lib/services/node_data_manager.dart @@ -215,7 +215,18 @@ class NodeDataManager extends ChangeNotifier { } } - /// Fetch nodes with automatic area splitting if needed + /// Fetch nodes, splitting the query area into quadrants only when Overpass + /// reports the query would exceed its hard 50k node limit ([NodeLimitError]). + /// + /// Splitting is deliberately NOT used for other failure modes: + /// - [RateLimitError] (HTTP 429): the server explicitly asked us to slow + /// down, so firing off up to 4x more requests would be counterproductive. + /// We back off instead, while still surfacing the "slow" status + /// indicator to the user (same UI feedback as the splitting case). + /// - Timeouts / other transient failures now surface as [NetworkError] + /// (see [OverpassService]) and are not caught here at all — they're + /// handled by the normal retry/backoff/fallback logic and reported as a + /// plain timeout/error status by the caller. Future> fetchWithSplitting( LatLngBounds bounds, List profiles, { @@ -238,7 +249,7 @@ class NodeDataManager extends ChangeNotifier { return nodes; } on NodeLimitError { - // Hit node limit or timeout - split area if not too deep + // Hit the deterministic 50k node limit - split area if not too deep if (splitDepth >= maxSplitDepth) { debugPrint('[NodeDataManager] Max split depth reached, giving up'); return []; @@ -254,13 +265,20 @@ class NodeDataManager extends ChangeNotifier { return _fetchSplitAreas(bounds, profiles, splitDepth + 1, isUserInitiated: isUserInitiated); } on RateLimitError { - // Rate limited - wait and return empty - debugPrint('[NodeDataManager] Rate limited, backing off'); + // Rate limited - back off without splitting the area. Splitting here + // would multiply request volume against a server that just told us to + // slow down. We still show the same "slow" status indicator so the + // user gets feedback, but there is no recursive quadrant fetching. + debugPrint('[NodeDataManager] Rate limited, backing off (no area splitting)'); + if (isUserInitiated) { + NetworkStatus.instance.setSplitting(); + } await Future.delayed(const Duration(seconds: 30)); return []; } } + /// Fetch data by splitting area into quadrants Future> _fetchSplitAreas( LatLngBounds bounds, diff --git a/lib/services/overpass_service.dart b/lib/services/overpass_service.dart index 760a303..bb47a86 100644 --- a/lib/services/overpass_service.dart +++ b/lib/services/overpass_service.dart @@ -32,7 +32,20 @@ class OverpassService { String get _primaryEndpoint => _endpointOverride ?? defaultEndpoint; /// Fetch surveillance nodes from Overpass API with retry and fallback. - /// Throws NetworkError for retryable failures, NodeLimitError for area splitting. + /// + /// Throws: + /// - [NodeLimitError] when the query would exceed Overpass's hard 50k node + /// limit. This is the *only* case where the caller should split the + /// query area into smaller regions, since it's a deterministic function + /// of how much data lives in the requested bounds. + /// - [RateLimitError] when rate limited (HTTP 429 or equivalent). Callers + /// should back off, not split/retry — splitting would only amplify load + /// on a server that just told us to slow down. + /// - [NetworkError] for timeouts and other retryable HTTP/network failures. + /// Timeouts are usually a sign of server load or an expensive query, not + /// "too much area" — retrying with a hail of smaller sub-requests makes + /// the problem worse, so timeouts are treated like any other transient + /// network failure (handled by the existing retry/backoff/fallback logic). Future> fetchNodes({ required LatLngBounds bounds, required List profiles, @@ -70,22 +83,16 @@ class OverpassService { final errorBody = response.body; - // Node limit error - caller should split area + // Node limit error - a deterministic result-set size limit. This is + // the one case where splitting the query area into smaller regions is + // the correct fix, since it directly reduces nodes-per-request. if (response.statusCode == 400 && (errorBody.contains('too many nodes') && errorBody.contains('50000'))) { - debugPrint('[OverpassService] Node limit exceeded, area should be split'); + debugPrint('[OverpassService] Node limit exceeded (50k), area should be split'); throw NodeLimitError('Query exceeded 50k node limit'); } - // Timeout error - also try splitting (complex query) - if (errorBody.contains('timeout') || - errorBody.contains('runtime limit exceeded') || - errorBody.contains('Query timed out')) { - debugPrint('[OverpassService] Query timeout, area should be split'); - throw NodeLimitError('Query timed out - area too complex'); - } - - // Rate limit + // Rate limit - back off, don't split or hammer the server with more requests. if (response.statusCode == 429 || errorBody.contains('rate limited') || errorBody.contains('too many requests')) { @@ -93,6 +100,17 @@ class OverpassService { throw RateLimitError('Rate limited by Overpass API'); } + // Timeout / runtime limit exceeded - treat as a plain retryable network + // error. This is usually server load or query cost, not "area too + // big" — splitting into up to 64 sub-requests would only make load + // on the server worse. + if (errorBody.contains('timeout') || + errorBody.contains('runtime limit exceeded') || + errorBody.contains('Query timed out')) { + debugPrint('[OverpassService] Query timed out'); + throw NetworkError('Query timed out'); + } + throw NetworkError('HTTP ${response.statusCode}: $errorBody'); } catch (e) { if (e is NodeLimitError || e is RateLimitError || e is NetworkError) { @@ -173,7 +191,9 @@ out ids; } } -/// Error thrown when query exceeds node limits or is too complex - area should be split +/// Error thrown when a query would exceed Overpass's 50k node limit. +/// The caller should split the query area into smaller regions to resolve this +/// — it's a deterministic function of how much data lives within the bounds. class NodeLimitError extends Error { final String message; NodeLimitError(this.message); @@ -189,7 +209,7 @@ class RateLimitError extends Error { String toString() => 'RateLimitError: $message'; } -/// Error thrown for network/HTTP issues - retryable +/// Error thrown for network/HTTP issues (including timeouts) - retryable class NetworkError extends Error { final String message; NetworkError(this.message); diff --git a/test/services/overpass_service_test.dart b/test/services/overpass_service_test.dart index 0c067d7..91a19da 100644 --- a/test/services/overpass_service_test.dart +++ b/test/services/overpass_service_test.dart @@ -255,27 +255,28 @@ void main() { ); }); - test('response with "timeout" throws NodeLimitError', () async { + test('response with "timeout" throws NetworkError (not split)', () async { stubErrorResponse(400, 'runtime error: timeout in query execution'); await expectLater( () => service.fetchNodes( bounds: bounds, profiles: profiles, policy: const ResiliencePolicy(maxRetries: 0)), - throwsA(isA()), + throwsA(isA()), ); }); - test('response with "runtime limit exceeded" throws NodeLimitError', + test('response with "runtime limit exceeded" throws NetworkError (not split)', () async { stubErrorResponse(400, 'runtime limit exceeded'); await expectLater( () => service.fetchNodes( bounds: bounds, profiles: profiles, policy: const ResiliencePolicy(maxRetries: 0)), - throwsA(isA()), + throwsA(isA()), ); }); + test('HTTP 429 throws RateLimitError', () async { stubErrorResponse(429, 'Too Many Requests');