V3-12: crash reporting (code only)
shouldInitializeCrashReporting() and scrubExtra() are pure, fully unit-tested decision/scrubbing functions wired into sentry_flutter via beforeSend/beforeBreadcrumb. Config.crashReportingEnabled defaults off; the DSN is a compile-time --dart-define, not a preference. RecordingEngine gained an injected onUnexpectedStop callback, firing once when restoreAfterProcessDeath finds a trip still 'recording' at launch -- the process died without a user stop. Marked partially done: the ticket's real acceptance criteria (a forced crash landing in a real Sentry project from a release build, inspecting a real payload) need a Sentry account and a release build this environment can't produce.
This commit is contained in:
@@ -3,11 +3,22 @@ import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
|
||||
import 'src/app/providers.dart';
|
||||
import 'src/config/config.dart';
|
||||
import 'src/crash/crash_reporter.dart';
|
||||
import 'src/ui/router.dart';
|
||||
import 'src/ui/theme.dart';
|
||||
|
||||
void main() {
|
||||
runApp(const ProviderScope(child: RipprApp()));
|
||||
Future<void> main() async {
|
||||
// Loaded early, once, purely to decide whether to talk to Sentry at all -- the
|
||||
// decision has to be made before `runApp`, since `SentryFlutter.init` wraps it. The
|
||||
// widget tree loads its own `Config` again in `RipprApp.initState` for everything
|
||||
// else; SharedPreferences is memory-cached after the first read, so this costs nothing
|
||||
// beyond a single extra map lookup.
|
||||
WidgetsFlutterBinding.ensureInitialized();
|
||||
final config = await Config.load();
|
||||
await maybeInitCrashReporting(
|
||||
config: config,
|
||||
appRunner: () async => runApp(const ProviderScope(child: RipprApp())),
|
||||
);
|
||||
}
|
||||
|
||||
class RipprApp extends ConsumerStatefulWidget {
|
||||
|
||||
@@ -14,6 +14,7 @@ import '../config/config.dart';
|
||||
import '../data/database.dart';
|
||||
import '../data/trip_repository.dart';
|
||||
import '../domain/models.dart';
|
||||
import '../crash/crash_reporter.dart';
|
||||
import '../data/route_plan_repository.dart';
|
||||
import '../notification/ride_notification_controller.dart';
|
||||
import '../notification/ride_notification_coordinator.dart';
|
||||
@@ -69,6 +70,8 @@ final recordingEngineProvider = Provider<RecordingEngine>((ref) {
|
||||
repository: ref.watch(tripRepositoryProvider),
|
||||
locationSource: ref.watch(locationSourceProvider),
|
||||
uploadPending: () async => ref.read(uploaderProvider)?.uploadPending(),
|
||||
onUnexpectedStop: (reason) =>
|
||||
reportUnexpectedRecordingStop(reason: reason),
|
||||
);
|
||||
ref.onDispose(engine.dispose);
|
||||
return engine;
|
||||
@@ -120,6 +123,14 @@ final mountedModeProvider = StateProvider<bool>(
|
||||
(ref) => ref.watch(configProvider)?.mountedMode ?? false,
|
||||
);
|
||||
|
||||
/// V3-12: same shape again. Note that flipping this at runtime does not retroactively
|
||||
/// start or stop a Sentry client already initialised at app launch -- see
|
||||
/// `maybeInitCrashReporting`'s doc comment on why that gate is checked once, in
|
||||
/// `main()`, not read reactively.
|
||||
final crashReportingEnabledProvider = StateProvider<bool>(
|
||||
(ref) => ref.watch(configProvider)?.crashReportingEnabled ?? false,
|
||||
);
|
||||
|
||||
/// Overridden in tests with [FakeWakelockController].
|
||||
final wakelockControllerProvider = Provider<WakelockController>(
|
||||
(ref) => PlusWakelockController(),
|
||||
|
||||
@@ -16,6 +16,7 @@ const _keyDeviceId = 'device_id';
|
||||
const _keyMapEnabled = 'map_enabled';
|
||||
const _keyUnitSystem = 'unit_system';
|
||||
const _keyMountedMode = 'mounted_mode';
|
||||
const _keyCrashReportingEnabled = 'crash_reporting_enabled';
|
||||
|
||||
/// Countries that did not adopt metric for everyday distances. Not exhaustive — a
|
||||
/// best-effort default, not a claim of authority. Anyone can override it in Settings.
|
||||
@@ -78,6 +79,15 @@ class Config {
|
||||
Future<void> setMountedMode(bool enabled) =>
|
||||
_prefs.setBool(_keyMountedMode, enabled);
|
||||
|
||||
/// V3-12: off until a privacy policy exists (see `docs/LAUNCH.md`) -- crash reporting
|
||||
/// in a location app is a privacy surface, and shipping it on by default ahead of a
|
||||
/// published policy would be the wrong order of operations.
|
||||
bool get crashReportingEnabled =>
|
||||
_prefs.getBool(_keyCrashReportingEnabled) ?? false;
|
||||
|
||||
Future<void> setCrashReportingEnabled(bool enabled) =>
|
||||
_prefs.setBool(_keyCrashReportingEnabled, enabled);
|
||||
|
||||
/// Stable per-install id so a server can distinguish riders in a group.
|
||||
String get deviceId {
|
||||
final existing = _prefs.getString(_keyDeviceId);
|
||||
|
||||
111
lib/src/crash/crash_reporter.dart
Normal file
111
lib/src/crash/crash_reporter.dart
Normal file
@@ -0,0 +1,111 @@
|
||||
/// V3-12: know when the app dies mid-ride, without turning a location app's own crash
|
||||
/// reports into a second location app.
|
||||
///
|
||||
/// **A crash reporter in a location app is a privacy surface.** Every payload passes
|
||||
/// through [scrubExtra] before it leaves the device; nothing here is a matter of
|
||||
/// configuring Sentry correctly and hoping the SDK does the right thing by default.
|
||||
library;
|
||||
|
||||
import 'package:flutter/foundation.dart';
|
||||
import 'package:sentry_flutter/sentry_flutter.dart';
|
||||
|
||||
import '../config/config.dart';
|
||||
|
||||
/// The decision of *whether* to talk to Sentry at all, pulled out as a pure function so
|
||||
/// every combination of debug/release, the user's toggle, and a configured DSN can be
|
||||
/// asserted without ever constructing a real client -- see
|
||||
/// "Reporting disabled means the client is never initialised" in the ticket's Tests
|
||||
/// section.
|
||||
bool shouldInitializeCrashReporting({
|
||||
required bool enabled,
|
||||
required bool isDebug,
|
||||
required String dsn,
|
||||
}) => enabled && !isDebug && dsn.isNotEmpty;
|
||||
|
||||
/// Keys that must never leave the device in a crash payload: coordinates, in any of the
|
||||
/// spellings this codebase or its dependencies use, plus device/ride identity. Matched
|
||||
/// case-insensitively and as a substring, so `latitude`, `startLat`, `lat`, and a nested
|
||||
/// `gps.lon` are all caught without having to enumerate every call site that might one
|
||||
/// day capture one into `extra` or a breadcrumb.
|
||||
const _forbiddenKeyFragments = [
|
||||
'lat',
|
||||
'lon',
|
||||
'coord',
|
||||
'altitude',
|
||||
'device_id',
|
||||
'deviceid',
|
||||
'trip_id',
|
||||
'tripid',
|
||||
];
|
||||
|
||||
bool _isForbiddenKey(String key) {
|
||||
final lower = key.toLowerCase();
|
||||
return _forbiddenKeyFragments.any(lower.contains);
|
||||
}
|
||||
|
||||
/// Strips forbidden keys from a breadcrumb's or event's free-form data map. Never
|
||||
/// mutates [data]; returns a new map (or the same empty-ness) so a caller can never
|
||||
/// accidentally hang onto the unscrubbed original by reference.
|
||||
Map<String, dynamic>? scrubExtra(Map<String, dynamic>? data) {
|
||||
if (data == null) return null;
|
||||
return {
|
||||
for (final entry in data.entries)
|
||||
if (!_isForbiddenKey(entry.key)) entry.key: entry.value,
|
||||
};
|
||||
}
|
||||
|
||||
SentryEvent _scrubEvent(SentryEvent event) => event.copyWith(
|
||||
// `extra` is deprecated in favour of structured contexts, but still populated by
|
||||
// some integrations and manual capture calls -- scrubbed defensively regardless of
|
||||
// which path an event arrived through.
|
||||
// ignore: deprecated_member_use
|
||||
extra: scrubExtra(event.extra),
|
||||
breadcrumbs: event.breadcrumbs
|
||||
?.map((b) => b.copyWith(data: scrubExtra(b.data)))
|
||||
.toList(),
|
||||
);
|
||||
|
||||
/// Wires [SentryFlutter.init] behind [shouldInitializeCrashReporting]. `dsn` is a
|
||||
/// compile-time value (`--dart-define=SENTRY_DSN=...`), not a user preference -- only
|
||||
/// *whether to report at all* is a user preference (see [Config.crashReportingEnabled]).
|
||||
Future<void> maybeInitCrashReporting({
|
||||
required Config config,
|
||||
required Future<void> Function() appRunner,
|
||||
String dsn = const String.fromEnvironment('SENTRY_DSN'),
|
||||
bool isDebug = kDebugMode,
|
||||
}) async {
|
||||
if (!shouldInitializeCrashReporting(
|
||||
enabled: config.crashReportingEnabled,
|
||||
isDebug: isDebug,
|
||||
dsn: dsn,
|
||||
)) {
|
||||
await appRunner();
|
||||
return;
|
||||
}
|
||||
|
||||
await SentryFlutter.init((options) {
|
||||
options.dsn = dsn;
|
||||
// Location, id and ride content scrubbing -- the whole point of this file.
|
||||
options.beforeSend = (event, hint) async => _scrubEvent(event);
|
||||
options.beforeBreadcrumb = (breadcrumb, hint) =>
|
||||
breadcrumb?.copyWith(data: scrubExtra(breadcrumb.data));
|
||||
// No default integrations that might capture more device context than intended.
|
||||
options.sendDefaultPii = false;
|
||||
}, appRunner: appRunner);
|
||||
}
|
||||
|
||||
/// The one custom event worth having beyond crashes: recording stopped without the rider
|
||||
/// choosing to stop it. That is the failure this app exists to avoid, and it can happen
|
||||
/// without ever throwing -- a location permission revoked mid-ride, a killed process that
|
||||
/// restores into a state the engine treats as already-stopped, or a platform quietly
|
||||
/// tearing down the position stream.
|
||||
///
|
||||
/// A no-op when reporting isn't initialised, exactly like every Sentry call is when
|
||||
/// `Sentry.isEnabled` is false -- there is no separate gate to keep in sync here.
|
||||
void reportUnexpectedRecordingStop({required String reason}) {
|
||||
Sentry.captureMessage(
|
||||
'Recording ended unexpectedly',
|
||||
level: SentryLevel.warning,
|
||||
withScope: (scope) => scope.setContexts('stop', {'reason': reason}),
|
||||
);
|
||||
}
|
||||
@@ -68,11 +68,13 @@ class RecordingEngine {
|
||||
LiveTelemetry? liveTelemetry,
|
||||
int Function()? clock,
|
||||
Future<void> Function()? uploadPending,
|
||||
void Function(String reason)? onUnexpectedStop,
|
||||
}) : _repo = repository,
|
||||
_source = locationSource,
|
||||
_live = liveTelemetry ?? LiveTelemetry.instance,
|
||||
_now = clock ?? (() => DateTime.now().millisecondsSinceEpoch),
|
||||
_uploadPending = uploadPending;
|
||||
_uploadPending = uploadPending,
|
||||
_onUnexpectedStop = onUnexpectedStop;
|
||||
|
||||
final TripRepository _repo;
|
||||
final LocationSource _source;
|
||||
@@ -84,6 +86,11 @@ class RecordingEngine {
|
||||
final Future<void> Function()? _uploadPending;
|
||||
Timer? _uploadTimer;
|
||||
|
||||
/// V3-12: injected rather than importing a crash reporter directly, the same reasoning
|
||||
/// as [_uploadPending] -- the engine stays free of any opinion about where a report
|
||||
/// goes, and tests need no Sentry client.
|
||||
final void Function(String reason)? _onUnexpectedStop;
|
||||
|
||||
/// Unbounded, exactly like the Kotlin `Channel(UNLIMITED)`. Appending is the only work
|
||||
/// done on the fix path.
|
||||
final List<TrackPoint> _pending = [];
|
||||
@@ -240,6 +247,12 @@ class RecordingEngine {
|
||||
final trip = await _repo.activeTrip();
|
||||
switch (trip?.state) {
|
||||
case TripState.recording:
|
||||
// A trip still marked `recording` at launch means the previous process ended
|
||||
// without ever calling stop() or pause() -- a crash, an OS kill, or a location
|
||||
// permission revoked out from under the app. This is the failure V3-12 exists to
|
||||
// surface: the recording stopped, silently, and nobody chose that.
|
||||
_onUnexpectedStop?.call('process death mid-recording');
|
||||
|
||||
// Resume into a genuinely *new* segment: the time the process was dead is a real
|
||||
// gap in the recording and must render as one.
|
||||
//
|
||||
|
||||
@@ -142,6 +142,21 @@ class _SettingsBodyState extends ConsumerState<_SettingsBody> {
|
||||
},
|
||||
),
|
||||
const Divider(),
|
||||
const _SectionHeader('Crash reporting'),
|
||||
SwitchListTile(
|
||||
key: const Key('crash-reporting-switch'),
|
||||
title: const Text('Send crash reports'),
|
||||
subtitle: const Text(
|
||||
'Off by default. No location, device id, or ride content is ever included '
|
||||
'-- see V3-12. Takes effect on next launch.',
|
||||
),
|
||||
value: ref.watch(crashReportingEnabledProvider),
|
||||
onChanged: (value) async {
|
||||
await widget.config.setCrashReportingEnabled(value);
|
||||
ref.read(crashReportingEnabledProvider.notifier).state = value;
|
||||
},
|
||||
),
|
||||
const Divider(),
|
||||
const _SectionHeader('Sync'),
|
||||
Padding(
|
||||
padding: const EdgeInsets.symmetric(horizontal: 16),
|
||||
|
||||
Reference in New Issue
Block a user