From 6f6d1ea5e026766fe69fdac9c9facfad5988e00a Mon Sep 17 00:00:00 2001 From: Christophe Vila Date: Mon, 27 Jul 2026 19:03:09 +0200 Subject: [PATCH] fix(sync): stop retrying a workout Garmin confirms is gone fillPendingWorkouts previously treated every get_workout_by_id failure identically -- log it, leave workout_raw_json null, retry next sync -- which is correct for a transient failure but means a definitive 404 (the workout deleted on Garmin's side after being linked to an activity) got silently retried forever, with the only symptom being an unexplained, permanent "1 more workout pending" nudge. Now checks errors.Is(err, garmin.ErrNotFound) and, only for that case, calls SetActivityWorkoutNotFound instead of retrying. --- backend/internal/sync/service.go | 14 +++++- backend/internal/sync/service_test.go | 62 +++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/backend/internal/sync/service.go b/backend/internal/sync/service.go index 4d3297d..b1c4a82 100644 --- a/backend/internal/sync/service.go +++ b/backend/internal/sync/service.go @@ -7,6 +7,7 @@ package sync import ( "context" "encoding/json" + "errors" "fmt" "log" "sync" @@ -363,7 +364,18 @@ func (s *Service) fillPendingWorkouts(ctx context.Context, limit int) error { } } if err := s.fillActivityWorkout(ctx, a, profile); err != nil { - log.Printf("sync: fill workout for activity %d failed, will retry next sync: %v", a.GarminActivityID, err) + if errors.Is(err, garmin.ErrNotFound) { + // A definitive 404 (the workout was deleted on Garmin's side + // after being linked to this activity) will never succeed on + // retry -- mark it so ActivitiesMissingWorkout stops + // surfacing it, instead of retrying forever. + log.Printf("sync: workout for activity %d not found on Garmin, marking as such (will not retry): %v", a.GarminActivityID, err) + if serr := s.db.SetActivityWorkoutNotFound(ctx, s.userID, a.ID); serr != nil { + return fmt.Errorf("mark activity %d workout not found: %w", a.GarminActivityID, serr) + } + } else { + log.Printf("sync: fill workout for activity %d failed, will retry next sync: %v", a.GarminActivityID, err) + } } s.setProgress(PhaseWorkouts, i+1, len(pending)) } diff --git a/backend/internal/sync/service_test.go b/backend/internal/sync/service_test.go index 236ad4d..ae6f92a 100644 --- a/backend/internal/sync/service_test.go +++ b/backend/internal/sync/service_test.go @@ -902,6 +902,68 @@ func TestFillPendingDetails_OneWorkoutFetchFailureDoesNotAbortTheWorkoutsPass(t } } +func TestFillPendingDetails_NotFoundWorkoutStopsBeingRetried(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userID := provisionTestUser(t, db) + + const missingWorkoutID = 300 + missingPtr := int64(missingWorkoutID) + m := &mock.Client{ + Activities: []garmin.Activity{ + {ActivityID: 1, ActivityType: garmin.ActivityType{TypeKey: "running"}, WorkoutID: &missingPtr, + StartTimeGMT: "2026-07-01 06:00:00", Distance: 5000, Duration: 1500}, + }, + Splits: map[int64]garmin.ActivitySplits{ + 1: {ActivityID: 1, Laps: []garmin.Lap{{LapIndex: 1, IntensityType: "ACTIVE"}}}, + }, + Details: map[int64]garmin.ActivityDetails{1: {ActivityID: 1}}, + Workouts: map[int64]garmin.Workout{}, + WorkoutErrByID: map[int64]error{missingWorkoutID: fmt.Errorf("API Error 404: %w", garmin.ErrNotFound)}, + } + svc := NewService(m, db, userID, Config{InterCallDelay: time.Millisecond}, + fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) + + if _, err := svc.backfillCore(ctx); err != nil { + t.Fatalf("backfillCore: %v", err) + } + if err := svc.FillPendingDetails(ctx, 10); err != nil { + t.Fatalf("FillPendingDetails should not return an error for a confirmed-404 workout: %v", err) + } + + remaining, err := db.CountActivitiesMissingWorkout(ctx, userID) + if err != nil { + t.Fatalf("CountActivitiesMissingWorkout: %v", err) + } + if remaining != 0 { + t.Fatalf("CountActivitiesMissingWorkout = %d, want 0 (confirmed-404 activity must not be retried)", remaining) + } + + activities, err := db.ListActivities(ctx, userID, store.ActivityFilter{}) + if err != nil { + t.Fatalf("ListActivities: %v", err) + } + if len(activities) != 1 { + t.Fatalf("ListActivities returned %d activities, want 1", len(activities)) + } + got, _, err := db.GetActivity(ctx, userID, activities[0].ID) + if err != nil { + t.Fatalf("GetActivity: %v", err) + } + if got.WorkoutNotFoundAt == nil { + t.Error("WorkoutNotFoundAt is nil, want it set") + } + if got.WorkoutRawJSON != nil { + t.Error("WorkoutRawJSON should stay nil -- not-found is recorded separately, not faked") + } + + // A second FillPendingDetails call must not attempt this workout again + // (it's no longer in ActivitiesMissingWorkout's result set at all). + if err := svc.FillPendingDetails(ctx, 10); err != nil { + t.Fatalf("second FillPendingDetails: %v", err) + } +} + func TestService_TwoUsersSyncIndependently(t *testing.T) { db := openTestDB(t) ctx := context.Background()