diff --git a/stronglift_replacement/workout_app/lib/screens/workout_screen.dart b/stronglift_replacement/workout_app/lib/screens/workout_screen.dart index c0ee22c..d484313 100644 --- a/stronglift_replacement/workout_app/lib/screens/workout_screen.dart +++ b/stronglift_replacement/workout_app/lib/screens/workout_screen.dart @@ -3,6 +3,7 @@ library; import 'dart:async'; +import 'dart:developer'; import 'package:audioplayers/audioplayers.dart'; import 'package:flutter/material.dart'; import 'package:vibration/vibration.dart'; @@ -464,7 +465,17 @@ class _WorkoutScreenState extends State { await storage.clearActiveSession(); final syncResult = await _sync.writeWorkoutResult(session); - unawaited(WorkoutSyncService().push(session)); + // Not awaited: a slow or unreachable backend must not delay the summary + // dialog. But the result is no longer discarded -- a failed push logs at + // error level inside push(), so an unpushed workout is diagnosable + // instead of silently absent from every other device. + unawaited( + WorkoutSyncService().push(session).then((result) { + if (!result.pushed) { + log('Workout not synced: ${result.reason}', level: 1000); + } + }), + ); if (!mounted) return; unawaited( diff --git a/stronglift_replacement/workout_app/lib/services/workout_sync_service.dart b/stronglift_replacement/workout_app/lib/services/workout_sync_service.dart index adb08a8..2984854 100644 --- a/stronglift_replacement/workout_app/lib/services/workout_sync_service.dart +++ b/stronglift_replacement/workout_app/lib/services/workout_sync_service.dart @@ -1,8 +1,9 @@ -/// Pushes completed workout sessions to the shared GitHub sync repo. +/// Pushes completed workout sessions to the shared sync backend. library; import 'dart:convert'; +import 'dart:developer'; import 'package:crdt_sync/crdt_sync.dart'; import 'package:flutter/foundation.dart'; import 'package:http/http.dart' as http; @@ -23,11 +24,36 @@ Log _decode(String text) => (jsonDecode(text) as Map).map( (id, data) => MapEntry(id, Record.fromJson(data as Map)), ); +/// The outcome of a [WorkoutSyncService.push] / [WorkoutSyncService.pushManual]. +/// +/// A bare `Future` used to hide every failure: the caller could not +/// tell "nothing to do" from "it broke", and the only trace was a +/// `debugPrint` that goes nowhere in a release build. That is the exact +/// silent-failure shape `CLAUDE.md` forbids, and it is why a workout could +/// go unpushed with nothing anywhere saying so. +@immutable +class PushResult { + /// Creates a result. [reason] should read like a sentence a human can act on. + const PushResult({required this.pushed, required this.reason}); + + /// Whether the sync tick completed without error. + final bool pushed; + + /// Why it did or did not happen. + final String reason; + + @override + String toString() => 'PushResult(pushed: $pushed, reason: $reason)'; +} + /// Mirrors the PC-side `_workout_sync.py`: GitHub is used purely as dumb /// file storage via the Contents API, one [Record] per completed session, -/// merged onto whatever this phone has already pushed. A missing token or a -/// failed push is swallowed here -- sync being unconfigured or unreachable -/// must never crash or delay the workout-completion flow that calls [push]. +/// merged onto whatever this phone has already pushed. +/// +/// A failed push never throws into the workout-completion flow -- sync being +/// unreachable must not cost the user their session -- but it is never +/// silent either: every path returns a [PushResult] whose `reason` says what +/// happened, and failures log at error level. class WorkoutSyncService { /// Creates a [WorkoutSyncService]. [owner]/[repo]/[httpClient] default to /// the real `syncs` repo (`screen-locker-sync/` subdirectory) and a fresh @@ -74,15 +100,22 @@ class WorkoutSyncService { final String repo; final http.Client? _httpClient; - /// Pushes [session] to `devices/phone/log.json`, merging with whatever - /// this device has already pushed. No-ops silently if sync isn't - /// configured; logs (but does not rethrow) any [GitHubSyncError]. - Future push(WorkoutSession session) async { + /// Pushes [session] to this device's log, merging with whatever it has + /// already pushed. + /// + /// Never throws: returns a [PushResult] saying whether the tick happened + /// and why. Failures are logged at error level so an unpushed workout + /// leaves a trace rather than vanishing. + Future push(WorkoutSession session) async { final settings = await SyncSettings.load(); // Either backend counts. Gating on the GitHub token alone silently // dropped every push from a device connected only to Firebase -- and // once the mirror is retired that would be every device. - if (!settings.isConfigured && !await _hasFirebaseAccount()) return; + if (!settings.isConfigured && !await _hasFirebaseAccount()) { + const reason = 'sync not configured (no Firebase account, no token)'; + log('WorkoutSyncService.push skipped: $reason', level: 900); + return const PushResult(pushed: false, reason: reason); + } final github = GitHubClient( owner: owner, @@ -120,20 +153,37 @@ class WorkoutSyncService { // Firebase free tier's monthly budget depends on not happening. stateStore: await _openStateStore(), ); - } on GitHubSyncError catch (error) { - debugPrint('WorkoutSyncService.push failed: $error'); + return const PushResult(pushed: true, reason: 'pushed'); + } on Object catch (error, stackTrace) { + // Deliberately broad: a GitHubSyncError was the only case handled + // before, so a Firebase error, an auth failure or a dropped connection + // vanished with no trace at all. Still swallowed rather than rethrown + // -- a sync failure must not cost the user their finished workout -- + // but now it is reported, not hidden. + final reason = 'push failed: $error'; + log( + 'WorkoutSyncService.push failed', + level: 1000, + error: error, + stackTrace: stackTrace, + ); + return PushResult(pushed: false, reason: reason); } finally { github.close(); firebase?.close(); } } - /// Pushes a pre-built manual-workout [record] to `devices/phone/log.json`, - /// merging with whatever this device has already pushed. Same swallow-on- - /// failure contract as [push]. - Future pushManual(Record record) async { + /// Pushes a pre-built manual-workout [record] to this device's log. + /// + /// Same never-throw, always-report contract as [push]. + Future pushManual(Record record) async { final settings = await SyncSettings.load(); - if (!settings.isConfigured && !await _hasFirebaseAccount()) return; + if (!settings.isConfigured && !await _hasFirebaseAccount()) { + const reason = 'sync not configured (no Firebase account, no token)'; + log('WorkoutSyncService.pushManual skipped: $reason', level: 900); + return const PushResult(pushed: false, reason: reason); + } final github = GitHubClient( owner: owner, @@ -165,8 +215,16 @@ class WorkoutSyncService { // Firebase free tier's monthly budget depends on not happening. stateStore: await _openStateStore(), ); - } on GitHubSyncError catch (error) { - debugPrint('WorkoutSyncService.pushManual failed: $error'); + return const PushResult(pushed: true, reason: 'pushed'); + } on Object catch (error, stackTrace) { + final reason = 'push failed: $error'; + log( + 'WorkoutSyncService.pushManual failed', + level: 1000, + error: error, + stackTrace: stackTrace, + ); + return PushResult(pushed: false, reason: reason); } finally { github.close(); firebase?.close(); diff --git a/stronglift_replacement/workout_app/test/screens/manual_workout_screen_test.dart b/stronglift_replacement/workout_app/test/screens/manual_workout_screen_test.dart index 58b2d51..a269fea 100644 --- a/stronglift_replacement/workout_app/test/screens/manual_workout_screen_test.dart +++ b/stronglift_replacement/workout_app/test/screens/manual_workout_screen_test.dart @@ -16,8 +16,9 @@ class _FakeSync extends WorkoutSyncService { payloads; @override - Future pushManual(Record record) async { + Future pushManual(Record record) async { pushed = record; + return const PushResult(pushed: true, reason: 'pushed'); } } diff --git a/stronglift_replacement/workout_app/test/services/workout_sync_service_test.dart b/stronglift_replacement/workout_app/test/services/workout_sync_service_test.dart index bedc8ee..69e59e9 100644 --- a/stronglift_replacement/workout_app/test/services/workout_sync_service_test.dart +++ b/stronglift_replacement/workout_app/test/services/workout_sync_service_test.dart @@ -104,9 +104,13 @@ void main() { installFakeSecureStorage(); final (:httpClient, :putCalls) = _mockGitHub(); - await _service(httpClient: httpClient).push(_session()); + final result = await _service(httpClient: httpClient).push(_session()); expect(putCalls, isEmpty); + // Not just "nothing happened" -- the caller can tell WHY, so an unpushed + // workout is diagnosable instead of silently absent everywhere else. + expect(result.pushed, isFalse); + expect(result.reason, contains('not configured')); }); test('pushes a new record when nothing has been synced yet', () async { @@ -165,16 +169,41 @@ void main() { ); }); - test('a GitHubSyncError is swallowed, not rethrown', () async { + test('a sync error is swallowed, but reported', () async { installFakeSecureStorage(initial: {'sync.token': 'tok'}); final httpClient = http_testing.MockClient( (request) async => http.Response('', 500), ); - await expectLater( - _service(httpClient: httpClient).push(_session()), - completes, + final result = await _service(httpClient: httpClient).push(_session()); + + // Swallowed: a failed push must never cost the user their finished + // workout. Reported: the previous contract logged to debugPrint, which + // goes nowhere in a release build -- a workout could go unpushed with + // nothing anywhere saying so. + expect(result.pushed, isFalse); + expect(result.reason, contains('push failed')); + }); + + test('PushResult stringifies both fields for logs', () { + // It exists to be readable in a log line, so the log line is the test. + const result = PushResult(pushed: false, reason: 'push failed: boom'); + + expect(result.toString(), contains('pushed: false')); + expect(result.toString(), contains('push failed: boom')); + }); + + test('a successful push reports that it happened', () async { + installFakeSecureStorage(initial: {'sync.token': 'tok'}); + final (:httpClient, :putCalls) = _mockGitHub( + contentResponses: {'screen-locker-sync/devices': _response(200, [])}, ); + + final result = await _service(httpClient: httpClient).push(_session()); + + expect(result.pushed, isTrue); + expect(result.reason, 'pushed'); + expect(putCalls, isNotEmpty); }); Record _manual(String id, String cost, int wallMs) => Record( @@ -234,7 +263,7 @@ void main() { ); }); - test('pushManual swallows a sync error', () async { + test('pushManual reports a failure instead of hiding it', () async { installFakeSecureStorage(initial: {'sync.token': 'tok'}); final httpClient = http_testing.MockClient( (request) async => http.Response('', 500),