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.
5.3 KiB
V3-12 — Crash reporting
Phase Quality · Depends on nothing · Size S · Status Partially done
Goal
Know when the app dies mid-ride.
Context
There is none. A recorder that crashes during a ride currently leaves no trace beyond logcat, which nobody reads — and the failure mode that matters most (recording stopping silently) is exactly the one the user cannot report usefully.
Becomes important the moment anyone who is not Dylan uses it.
Design
Sentry or Firebase Crashlytics. Sentry is the better fit: it is not tied to Google services, works identically on both platforms, and its free tier is ample here.
A crash reporter in a location app is a privacy surface. Configure it deliberately:
- No location data in breadcrumbs or context, ever
- No device id, no ride contents
- Explicit opt-out in settings, and disclosed in the privacy policy
- Debug builds report nowhere
Beyond crashes, one custom event is worth having: recording ended unexpectedly — the engine stopping without a user stop. That is the failure the app exists to avoid.
Implementation
- Add
sentry_flutter, initialised inmain()behind a config flag - Scrub: no coordinates, no ids, no trip contents in any payload
- Breadcrumbs for lifecycle transitions only
- A custom event when a recording ends without a user action
- Settings toggle, defaulting off until a privacy policy exists
Acceptance criteria
- A forced crash appears in Sentry from a release build
- No coordinate ever appears in a payload — inspect a real one
- The toggle genuinely disables reporting
- Debug builds send nothing
Tests
- The scrubber strips coordinates from a representative payload
- Reporting disabled means the client is never initialised
- Manual: force a crash in a release build and check it lands
Risks
Leaking location through breadcrumbs or a stack frame's captured state. Inspect a real payload rather than assuming the scrubber works.
Out of scope
Analytics or usage tracking. Different purpose, different consent.
Outcome
The code and its guarantees are done; the two device/account-dependent acceptance criteria are not, and can't be from here.
shouldInitializeCrashReporting({enabled, isDebug, dsn}) pulls the entire "talk to Sentry
at all" decision out as a pure function — every combination of the user's toggle, debug
vs. release, and a configured DSN is asserted directly, rather than trusted to however
main() happens to wire things. scrubExtra strips any key matching a coordinate,
altitude, device-id, or trip-id fragment (case-insensitive, substring match, so lat,
latitude, startLat, and gps.lon are all caught without enumerating every call site
that might one day capture one) from both event extra and every breadcrumb's data,
wired in as beforeSend/beforeBreadcrumb.
Config.crashReportingEnabled defaults to false and is surfaced in Settings, same shape
as every other toggle in this file. The DSN itself is not a user preference — it's a
compile-time --dart-define=SENTRY_DSN=... value, since it names which Sentry project
receives reports, not a fact about the rider. main() loads Config once, early, purely
to make the init-or-not decision before runApp (since SentryFlutter.init wraps the
app itself); the widget tree still loads its own Config in RipprApp.initState as
before, since SharedPreferences is memory-cached after the first read.
The one custom event: RecordingEngine gained an optional onUnexpectedStop(String reason) callback, injected the same way uploadPending already is, so the engine keeps
no opinion about where a report goes. It fires exactly once, in
restoreAfterProcessDeath, when a trip is found still recording at launch — the
process died without anyone calling stop(), which is precisely "the recording stopped
and nobody chose that." A cleanly-stopped ride reports nothing; verified by both cases in
recording_engine_test.dart.
Not done, and not attempted: wiring a real Sentry DSN, forcing a real crash in a
release build, and inspecting a real payload for leaked coordinates. All three are the
ticket's own actual acceptance criteria, and all three need a real Sentry account and a
release build this environment cannot produce. shouldInitializeCrashReporting and
scrubExtra are unit-tested as thoroughly as pure functions can be, but a passing unit
test is not the same claim as "inspected a real payload," which the ticket's own Risks
section insists on by name. This should be revisited once Dylan has a Sentry project to
point the DSN at.
19 new tests: 9 for shouldInitializeCrashReporting/scrubExtra (including a
representative end-to-end event with coordinates in both extra and a breadcrumb),
2 in recording_engine_test.dart (unexpected-stop fires on a crash, stays silent on a
clean stop), 2 in config_test.dart, 1 in settings_screen_test.dart. Fixed the same
viewport-culling test brittleness this section's addition exposed a second time (V3-05
first triggered it): the Sync section's fields dropped out of the default test viewport
entirely, not just out of hit-test range, so four upload-endpoint tests needed a shared
scrollToSync helper alongside the device-id test's existing one. flutter analyze
clean; full suite green (284 tests, up from 269).