Bug panic rethrow crashes program (#62)

Fix panic that crashes program

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Author: hughdbrown

internal/sync/incremental.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"runtime/debug"
 	"strconv"
 	"time"
 
@@ -13,9 +14,9 @@ import (
 
 // Incremental performs an incremental sync using the Gmail History API.
 // Falls back to full sync if history is too old (404 error).
-func (s *Syncer) Incremental(ctx context.Context, email string) (*gmail.SyncSummary, error) {
+func (s *Syncer) Incremental(ctx context.Context, email string) (summary *gmail.SyncSummary, err error) {
 	startTime := time.Now()
-	summary := &gmail.SyncSummary{StartTime: startTime}
+	summary = &gmail.SyncSummary{StartTime: startTime}
 
 	// Get source - must already exist for incremental sync
 	source, err := s.store.GetSourceByIdentifier(email)
@@ -42,11 +43,16 @@ func (s *Syncer) Incremental(ctx context.Context, email string) (*gmail.SyncSumm
 		return nil, fmt.Errorf("start sync: %w", err)
 	}
 
-	// Defer failure handling
+	// Defer failure handling — recover from panics and return as error
 	defer func() {
 		if r := recover(); r != nil {
-			_ = s.store.FailSync(syncID, fmt.Sprintf("panic: %v", r))
-			panic(r)
+			stack := debug.Stack()
+			s.logger.Error("sync panic recovered", "panic", r, "stack", string(stack))
+			if failErr := s.store.FailSync(syncID, fmt.Sprintf("panic: %v", r)); failErr != nil {
+				s.logger.Error("failed to record sync failure", "error", failErr)
+			}
+			summary = nil
+			err = fmt.Errorf("sync panicked: %v", r)
 		}
 	}()
 

internal/sync/sync.go

@@ -9,6 +9,7 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"runtime/debug"
 	"strconv"
 	"time"
 
@@ -214,9 +215,9 @@ func (s *Syncer) processBatch(ctx context.Context, sourceID int64, listResp *gma
 }
 
 // Full performs a full synchronization.
-func (s *Syncer) Full(ctx context.Context, email string) (*gmail.SyncSummary, error) {
+func (s *Syncer) Full(ctx context.Context, email string) (summary *gmail.SyncSummary, err error) {
 	startTime := time.Now()
-	summary := &gmail.SyncSummary{StartTime: startTime}
+	summary = &gmail.SyncSummary{StartTime: startTime}
 
 	// Get or create source
 	source, err := s.store.GetOrCreateSource("gmail", email)
@@ -232,11 +233,16 @@ func (s *Syncer) Full(ctx context.Context, email string) (*gmail.SyncSummary, er
 	summary.WasResumed = state.wasResumed
 	summary.ResumedFromToken = state.pageToken
 
-	// Defer failure handling
+	// Defer failure handling — recover from panics and return as error
 	defer func() {
 		if r := recover(); r != nil {
-			_ = s.store.FailSync(state.syncID, fmt.Sprintf("panic: %v", r))
-			panic(r)
+			stack := debug.Stack()
+			s.logger.Error("sync panic recovered", "panic", r, "stack", string(stack))
+			if failErr := s.store.FailSync(state.syncID, fmt.Sprintf("panic: %v", r)); failErr != nil {
+				s.logger.Error("failed to record sync failure", "error", failErr)
+			}
+			summary = nil
+			err = fmt.Errorf("sync panicked: %v", r)
 		}
 	}()
 

internal/sync/sync_test.go

@@ -1,17 +1,76 @@
 package sync
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/wesm/msgvault/internal/gmail"
 	"github.com/wesm/msgvault/internal/store"
 	testemail "github.com/wesm/msgvault/internal/testutil/email"
 )
 
+// panicOnBatchAPI wraps a MockAPI and panics when GetMessagesRawBatch is called.
+// Used to test that Full() recovers from panics gracefully.
+type panicOnBatchAPI struct {
+	*gmail.MockAPI
+}
+
+func (p *panicOnBatchAPI) GetMessagesRawBatch(_ context.Context, _ []string) ([]*gmail.RawMessage, error) {
+	panic("unexpected nil pointer in batch processing")
+}
+
+func TestFullSync_PanicReturnsError(t *testing.T) {
+	env := newTestEnv(t)
+	seedMessages(env, 1, 12345, "msg1")
+
+	// Replace the client with one that panics during batch fetch
+	env.Syncer = New(&panicOnBatchAPI{MockAPI: env.Mock}, env.Store, nil)
+
+	// Should return an error, NOT panic and crash the program
+	_, err := env.Syncer.Full(env.Context, testEmail)
+	if err == nil {
+		t.Fatal("expected error from panic recovery, got nil")
+	}
+	if !strings.Contains(err.Error(), "panic") {
+		t.Errorf("expected error to mention panic, got: %v", err)
+	}
+}
+
+// panicOnHistoryAPI wraps a MockAPI and panics when ListHistory is called.
+// Used to test that Incremental() recovers from panics gracefully.
+type panicOnHistoryAPI struct {
+	*gmail.MockAPI
+}
+
+func (p *panicOnHistoryAPI) ListHistory(_ context.Context, _ uint64, _ string) (*gmail.HistoryResponse, error) {
+	panic("unexpected nil pointer in history processing")
+}
+
+func TestIncrementalSync_PanicReturnsError(t *testing.T) {
+	env := newTestEnv(t)
+	env.CreateSourceWithHistory(t, "12340")
+
+	env.Mock.Profile.MessagesTotal = 10
+	env.Mock.Profile.HistoryID = 12350
+
+	// Replace the client with one that panics during history fetch
+	env.Syncer = New(&panicOnHistoryAPI{MockAPI: env.Mock}, env.Store, nil)
+
+	// Should return an error, NOT panic and crash the program
+	_, err := env.Syncer.Incremental(env.Context, testEmail)
+	if err == nil {
+		t.Fatal("expected error from panic recovery, got nil")
+	}
+	if !strings.Contains(err.Error(), "panic") {
+		t.Errorf("expected error to mention panic, got: %v", err)
+	}
+}
+
 func TestFullSync(t *testing.T) {
 	env := newTestEnv(t)
 	seedMessages(env, 3, 12345, "msg1", "msg2", "msg3")