From 1d6219edb3b960d7a14b6da47fd6c6d54396da65 Mon Sep 17 00:00:00 2001 From: YuYu1015 Date: Tue, 20 Oct 2026 22:55:34 +0800 Subject: [PATCH] fix(location): stop recording a refused GPS fix as a crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Platform: all Fix(zh-Hant): 關掉定位或拒絕權限不再被記成應用程式錯誤 Fix(en-US): turning location off or denying it is no longer recorded as an app error --- lib/core/geo/device_location_reporter.dart | 17 +++++-- lib/core/geo/location_service.dart | 23 +++++++++ .../geo/device_location_reporter_test.dart | 47 +++++++++++++++++++ test/core/geo/location_service_test.dart | 46 ++++++++++++++++++ 4 files changed, 128 insertions(+), 5 deletions(-) diff --git a/lib/core/geo/device_location_reporter.dart b/lib/core/geo/device_location_reporter.dart index 4e3d8917b..c271370d1 100644 --- a/lib/core/geo/device_location_reporter.dart +++ b/lib/core/geo/device_location_reporter.dart @@ -2,7 +2,8 @@ import 'dart:async'; import 'package:dio/dio.dart'; import 'package:dpip/core/logging/log.dart'; -import 'package:dpip/core/geo/location_service.dart' show GpsFix; +import 'package:dpip/core/geo/location_service.dart' + show GpsFix, isLocationRefusal; import 'package:dpip/core/settings/setting_keys.dart'; import 'package:dpip/core/settings/settings_store.dart'; @@ -88,11 +89,17 @@ class DeviceLocationReporter { if (_sub != null) return; _sub = _positions().listen( _handle, - // A services-off toggle errors the stream; keeping the error here means it - // never rides up as an uncaught async error, and dropping the subscription - // lets [restart] re-create it when services return. + // A services-off toggle, or a permission revoked while the stream is + // open, errors it. That is the user changing a setting. Keeping the + // error here means it never rides up as an uncaught async error, and + // dropping the subscription lets [restart] re-create it when services + // return. A refusal is not a fault, so it stays out of crash reports. onError: (Object error, StackTrace stackTrace) { - Log.handle(error, stackTrace, 'location stream'); + if (isLocationRefusal(error)) { + Log.warning('location stream stopped: location is off or denied'); + } else { + Log.handle(error, stackTrace, 'location stream'); + } _sub = null; }, onDone: () => _sub = null, diff --git a/lib/core/geo/location_service.dart b/lib/core/geo/location_service.dart index f291c4ed2..67f43c620 100644 --- a/lib/core/geo/location_service.dart +++ b/lib/core/geo/location_service.dart @@ -16,6 +16,15 @@ import 'package:permission_handler/permission_handler.dart' as ph; /// A GPS coordinate fix in decimal degrees. typedef GpsFix = ({double lat, double lng}); +/// Whether [error] is a user who turned location off or denied the permission. +/// +/// Geolocator raises these instead of returning an empty fix. Callers already +/// treat that as "no location". Filing it as a crash report makes a phone with +/// location off look broken. +bool isLocationRefusal(Object error) => + error is LocationServiceDisabledException || + error is PermissionDeniedException; + /// Resolves the device's current township from GPS. /// /// Wraps `geolocator` (native `CLLocationManager` / `FusedLocationProvider`). @@ -247,6 +256,10 @@ class LocationService { Log.warning('GPS fix timed out — no current location available'); return null; } catch (error, stackTrace) { + if (isLocationRefusal(error)) { + Log.warning('GPS fix unavailable: location is off or denied'); + return null; + } Log.handle(error, stackTrace, 'GPS fix'); return null; } @@ -263,6 +276,12 @@ class LocationService { try { return await _lastKnownFix(); } catch (error, stackTrace) { + if (isLocationRefusal(error)) { + Log.warning( + 'last-known GPS fix unavailable: location is off or denied', + ); + return null; + } Log.handle(error, stackTrace, 'last-known GPS fix'); return null; } @@ -282,6 +301,10 @@ class LocationService { Log.warning('GPS fix timed out — no current location available'); return null; } catch (error, stackTrace) { + if (isLocationRefusal(error)) { + Log.warning('GPS fix unavailable: location is off or denied'); + return null; + } Log.handle(error, stackTrace, 'GPS fix'); return null; } diff --git a/test/core/geo/device_location_reporter_test.dart b/test/core/geo/device_location_reporter_test.dart index 4b41c1ce7..5cdea3876 100644 --- a/test/core/geo/device_location_reporter_test.dart +++ b/test/core/geo/device_location_reporter_test.dart @@ -3,9 +3,12 @@ import 'dart:async'; import 'package:dio/dio.dart'; import 'package:dpip/core/geo/device_location_reporter.dart'; import 'package:dpip/core/geo/location_service.dart' show GpsFix; +import 'package:dpip/core/logging/crash_sink.dart'; +import 'package:dpip/core/logging/log.dart'; import 'package:dpip/core/settings/setting_keys.dart'; import 'package:dpip/core/settings/settings_store.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:geolocator/geolocator.dart'; void main() { TestWidgetsFlutterBinding.ensureInitialized(); @@ -149,6 +152,36 @@ void main() { } }); + test('a location refusal ends the stream without a crash report', () async { + final sink = _Sink(); + Log.crashSink = sink; + addTearDown(() => Log.crashSink = null); + final controllers = >[]; + final reporter = DeviceLocationReporter( + positions: () { + final controller = StreamController(); + controllers.add(controller); + return controller.stream; + }, + settings: await settings(), + onMoved: (fix) async => true, + )..start(); + + controllers[0].addError(const LocationServiceDisabledException()); + await pumpEventQueue(); + expect(sink.handled, isEmpty); + + reporter.restart(); + controllers[1].addError(StateError('gps')); + await pumpEventQueue(); + expect(sink.handled.single, isA()); + + reporter.stop(); + for (final c in controllers) { + await c.close(); + } + }); + test('shares accepted fixes on [fixes] for the township tracker', () async { final positions = StreamController(); final reporter = DeviceLocationReporter( @@ -301,3 +334,17 @@ void main() { }); }); } + +class _Sink implements CrashSink { + final handled = []; + + @override + void report( + Object error, + StackTrace stackTrace, { + String? context, + bool fatal = false, + }) { + handled.add(error); + } +} diff --git a/test/core/geo/location_service_test.dart b/test/core/geo/location_service_test.dart index b96876ad6..e2603f4ce 100644 --- a/test/core/geo/location_service_test.dart +++ b/test/core/geo/location_service_test.dart @@ -4,6 +4,8 @@ import 'package:dpip/core/geo/location_service.dart'; import 'package:dpip/core/geo/location_status.dart'; import 'package:dpip/core/geo/town_boundaries.dart'; import 'package:dpip/core/geo/town_directory.dart'; +import 'package:dpip/core/logging/crash_sink.dart'; +import 'package:dpip/core/logging/log.dart'; import 'package:dpip/core/permissions/permission_outcome.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -90,11 +92,41 @@ void main() { }); test('a lastKnownFix fault degrades to null, never throws', () async { + final sink = _Sink(); + Log.crashSink = sink; + addTearDown(() => Log.crashSink = null); final service = LocationService( _dir(), lastKnown: () async => throw Exception('platform fault'), ); expect(await service.lastKnownFix(), isNull); + expect(sink.handled, hasLength(1)); + }); + + test('a refused last-known fix is null and not a crash report', () async { + final sink = _Sink(); + Log.crashSink = sink; + addTearDown(() => Log.crashSink = null); + final denied = LocationService( + _dir(), + lastKnown: () async => throw const PermissionDeniedException( + "User denied permissions to access the device's location.", + ), + ); + expect(await denied.lastKnownFix(), isNull); + final off = LocationService( + _dir(), + lastKnown: () async => throw const LocationServiceDisabledException(), + ); + expect(await off.lastKnownFix(), isNull); + final live = LocationService( + _dir(), + isAvailable: () async => true, + fix: () async => throw const LocationServiceDisabledException(), + ); + expect(await live.currentFix(), isNull); + expect(await live.currentTown(), isNull); + expect(sink.handled, isEmpty); }); test('a GPS timeout is a null fix, not a crash', () async { @@ -440,6 +472,20 @@ void _platformCases() { }); } +class _Sink implements CrashSink { + final handled = []; + + @override + void report( + Object error, + StackTrace stackTrace, { + String? context, + bool fatal = false, + }) { + handled.add(error); + } +} + class _FakeGeolocator extends GeolocatorPlatform { bool services = true; bool throwServices = false;