Skip to content

Commit 25f14d6

Browse files
Thomas StrombergThomas Stromberg
authored andcommitted
Save logs to disk
1 parent df32794 commit 25f14d6

File tree

5 files changed

+197
-36
lines changed

5 files changed

+197
-36
lines changed

cmd/goose/github.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,10 +446,17 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
446446
// Collect results and update PRs directly
447447
turnSuccesses := 0
448448
turnFailures := 0
449+
actualAPICalls := 0
450+
cacheHits := 0
449451

450452
for result := range results {
451453
if result.err == nil && result.turnData != nil && result.turnData.PRState.UnblockAction != nil {
452454
turnSuccesses++
455+
if result.wasFromCache {
456+
cacheHits++
457+
} else {
458+
actualAPICalls++
459+
}
453460

454461
// Check if user needs to review and get action reason
455462
needsReview := false
@@ -487,6 +494,17 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
487494
}
488495
}
489496

490-
slog.Info("[TURN] Turn API queries completed",
491-
"duration", time.Since(turnStart), "succeeded", turnSuccesses, "total", turnSuccesses+turnFailures)
497+
// Only log if there were actual API calls or failures
498+
if actualAPICalls > 0 || turnFailures > 0 {
499+
slog.Info("[TURN] API queries completed",
500+
"duration", time.Since(turnStart),
501+
"api_calls", actualAPICalls,
502+
"cache_hits", cacheHits,
503+
"failures", turnFailures,
504+
"total", turnSuccesses+turnFailures)
505+
} else if cacheHits > 0 {
506+
slog.Debug("[TURN] All data served from cache",
507+
"cache_hits", cacheHits,
508+
"duration", time.Since(turnStart))
509+
}
492510
}

cmd/goose/main.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ type App struct {
7878
turnClient *turn.Client
7979
previousBlockedPRs map[string]bool
8080
authError string
81+
lastFetchError string
8182
cacheDir string
8283
targetUser string
8384
lastMenuTitles []string
@@ -208,6 +209,30 @@ func main() {
208209
os.Exit(1)
209210
}
210211

212+
// Set up file-based logging alongside cache
213+
logDir := filepath.Join(cacheDir, "logs")
214+
if err := os.MkdirAll(logDir, dirPerm); err != nil {
215+
slog.Error("Failed to create log directory", "error", err)
216+
// Continue without file logging
217+
} else {
218+
// Create log file with daily rotation
219+
logPath := filepath.Join(logDir, fmt.Sprintf("goose-%s.log", time.Now().Format("2006-01-02")))
220+
logFile, err := os.OpenFile(logPath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600)
221+
if err != nil {
222+
slog.Error("Failed to open log file", "error", err)
223+
} else {
224+
// Update logger to write to both stderr and file
225+
multiHandler := &MultiHandler{
226+
handlers: []slog.Handler{
227+
slog.NewTextHandler(os.Stderr, opts),
228+
slog.NewTextHandler(logFile, opts),
229+
},
230+
}
231+
slog.SetDefault(slog.New(multiHandler))
232+
slog.Info("Logs are being written to", "path", logPath)
233+
}
234+
}
235+
211236
startTime := time.Now()
212237
app := &App{
213238
cacheDir: cacheDir,
@@ -376,6 +401,7 @@ func (app *App) updatePRs(ctx context.Context) {
376401
app.mu.Lock()
377402
app.consecutiveFailures++
378403
failureCount := app.consecutiveFailures
404+
app.lastFetchError = err.Error()
379405
app.mu.Unlock()
380406

381407
// Progressive degradation based on failure count
@@ -431,6 +457,7 @@ func (app *App) updatePRs(ctx context.Context) {
431457
app.mu.Lock()
432458
app.lastSuccessfulFetch = time.Now()
433459
app.consecutiveFailures = 0
460+
app.lastFetchError = ""
434461
app.mu.Unlock()
435462

436463
// Update state atomically
@@ -513,6 +540,7 @@ func (app *App) updatePRsWithWait(ctx context.Context) {
513540
app.mu.Lock()
514541
app.consecutiveFailures++
515542
failureCount := app.consecutiveFailures
543+
app.lastFetchError = err.Error()
516544
app.mu.Unlock()
517545

518546
// Progressive degradation based on failure count
@@ -535,7 +563,7 @@ func (app *App) updatePRsWithWait(ctx context.Context) {
535563
systray.SetTitle(title)
536564
systray.SetTooltip(tooltip)
537565

538-
// Still create initial menu even on error
566+
// Create or update menu to show error state
539567
if !app.menuInitialized {
540568
// Create initial menu despite error
541569
app.rebuildMenu(ctx)
@@ -547,6 +575,9 @@ func (app *App) updatePRsWithWait(ctx context.Context) {
547575
app.lastMenuTitles = menuTitles
548576
app.mu.Unlock()
549577
// Menu initialization complete
578+
} else if failureCount == 1 {
579+
// On first failure, rebuild menu to show error at top
580+
app.rebuildMenu(ctx)
550581
}
551582
return
552583
}
@@ -555,6 +586,7 @@ func (app *App) updatePRsWithWait(ctx context.Context) {
555586
app.mu.Lock()
556587
app.lastSuccessfulFetch = time.Now()
557588
app.consecutiveFailures = 0
589+
app.lastFetchError = ""
558590
app.mu.Unlock()
559591

560592
// Update state

cmd/goose/multihandler.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
)
7+
8+
// MultiHandler implements slog.Handler to write logs to multiple destinations.
9+
type MultiHandler struct {
10+
handlers []slog.Handler
11+
}
12+
13+
// Enabled returns true if at least one handler is enabled.
14+
func (h *MultiHandler) Enabled(ctx context.Context, level slog.Level) bool {
15+
for _, handler := range h.handlers {
16+
if handler.Enabled(ctx, level) {
17+
return true
18+
}
19+
}
20+
return false
21+
}
22+
23+
// Handle writes the record to all handlers.
24+
func (h *MultiHandler) Handle(ctx context.Context, record slog.Record) error {
25+
for _, handler := range h.handlers {
26+
if handler.Enabled(ctx, record.Level) {
27+
handler.Handle(ctx, record) //nolint:errcheck // Continue logging to other destinations
28+
}
29+
}
30+
return nil
31+
}
32+
33+
// WithAttrs returns a new handler with additional attributes.
34+
func (h *MultiHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
35+
handlers := make([]slog.Handler, len(h.handlers))
36+
for i, handler := range h.handlers {
37+
handlers[i] = handler.WithAttrs(attrs)
38+
}
39+
return &MultiHandler{handlers: handlers}
40+
}
41+
42+
// WithGroup returns a new handler with a group name.
43+
func (h *MultiHandler) WithGroup(name string) slog.Handler {
44+
handlers := make([]slog.Handler, len(h.handlers))
45+
for i, handler := range h.handlers {
46+
handlers[i] = handler.WithGroup(name)
47+
}
48+
return &MultiHandler{handlers: handlers}
49+
}

cmd/goose/settings.go

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,62 +16,43 @@ type Settings struct {
1616
EnableAutoBrowser bool `json:"enable_auto_browser"`
1717
}
1818

19-
// settingsDir returns the configuration directory for settings.
20-
func settingsDir() (string, error) {
21-
configDir, err := os.UserConfigDir()
22-
if err != nil {
23-
return "", err
24-
}
25-
return filepath.Join(configDir, "review-goose"), nil
26-
}
27-
2819
// loadSettings loads settings from disk or returns defaults.
2920
func (app *App) loadSettings() {
30-
settingsDir, err := settingsDir()
21+
// Set defaults first
22+
app.enableAudioCues = true
23+
app.hideStaleIncoming = true
24+
app.enableAutoBrowser = false
25+
app.hiddenOrgs = make(map[string]bool)
26+
27+
configDir, err := os.UserConfigDir()
3128
if err != nil {
3229
slog.Error("Failed to get settings directory", "error", err)
33-
// Use defaults
34-
app.enableAudioCues = true
35-
app.hideStaleIncoming = true
36-
app.enableAutoBrowser = false
37-
app.hiddenOrgs = make(map[string]bool)
3830
return
3931
}
4032

41-
settingsPath := filepath.Join(settingsDir, "settings.json")
42-
33+
settingsPath := filepath.Join(configDir, "review-goose", "settings.json")
4334
data, err := os.ReadFile(settingsPath)
4435
if err != nil {
4536
if !os.IsNotExist(err) {
4637
slog.Debug("Failed to read settings", "error", err)
4738
}
48-
// Use defaults
49-
app.enableAudioCues = true
50-
app.hideStaleIncoming = true
51-
app.enableAutoBrowser = false
52-
app.hiddenOrgs = make(map[string]bool)
5339
return
5440
}
5541

5642
var settings Settings
5743
if err := json.Unmarshal(data, &settings); err != nil {
5844
slog.Error("Failed to parse settings", "error", err)
59-
// Use defaults
60-
app.enableAudioCues = true
61-
app.hideStaleIncoming = true
62-
app.enableAutoBrowser = false
63-
app.hiddenOrgs = make(map[string]bool)
6445
return
6546
}
6647

48+
// Override defaults with loaded values
6749
app.enableAudioCues = settings.EnableAudioCues
6850
app.hideStaleIncoming = settings.HideStale
6951
app.enableAutoBrowser = settings.EnableAutoBrowser
7052
if settings.HiddenOrgs != nil {
7153
app.hiddenOrgs = settings.HiddenOrgs
72-
} else {
73-
app.hiddenOrgs = make(map[string]bool)
7454
}
55+
7556
slog.Info("Loaded settings",
7657
"audio_cues", app.enableAudioCues,
7758
"hide_stale", app.hideStaleIncoming,
@@ -81,11 +62,12 @@ func (app *App) loadSettings() {
8162

8263
// saveSettings saves current settings to disk.
8364
func (app *App) saveSettings() {
84-
settingsDir, err := settingsDir()
65+
configDir, err := os.UserConfigDir()
8566
if err != nil {
8667
slog.Error("Failed to get settings directory", "error", err)
8768
return
8869
}
70+
settingsDir := filepath.Join(configDir, "review-goose")
8971

9072
app.mu.RLock()
9173
settings := Settings{

cmd/goose/ui.go

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"os/exec"
1111
"runtime"
1212
"sort"
13+
"strings"
1314
"time"
1415

1516
"github.com/energye/systray" // needed for MenuItem type
@@ -407,16 +408,23 @@ func (app *App) rebuildMenu(ctx context.Context) {
407408
// Clear all existing menu items
408409
app.systrayInterface.ResetMenu()
409410

410-
// Check for auth error first
411-
if app.authError != "" {
411+
// Check for errors (auth or connection failures)
412+
app.mu.RLock()
413+
authError := app.authError
414+
failureCount := app.consecutiveFailures
415+
lastFetchError := app.lastFetchError
416+
app.mu.RUnlock()
417+
418+
// Show auth error if present
419+
if authError != "" {
412420
// Show authentication error message
413421
errorTitle := app.systrayInterface.AddMenuItem("⚠️ Authentication Error", "")
414422
errorTitle.Disable()
415423

416424
app.systrayInterface.AddSeparator()
417425

418426
// Add error details
419-
errorMsg := app.systrayInterface.AddMenuItem(app.authError, "Click to see setup instructions")
427+
errorMsg := app.systrayInterface.AddMenuItem(authError, "Click to see setup instructions")
420428
errorMsg.Click(func() {
421429
if err := openURL(ctx, "https://cli.github.com/manual/gh_auth_login"); err != nil {
422430
slog.Error("failed to open setup instructions", "error", err)
@@ -449,6 +457,78 @@ func (app *App) rebuildMenu(ctx context.Context) {
449457
return
450458
}
451459

460+
// Show connection error if we have consecutive failures
461+
if failureCount > 0 && lastFetchError != "" {
462+
var errorMsg string
463+
switch {
464+
case failureCount == 1:
465+
errorMsg = "⚠️ Connection Error"
466+
case failureCount <= 3:
467+
errorMsg = fmt.Sprintf("⚠️ Connection Issues (%d failures)", failureCount)
468+
case failureCount <= 10:
469+
errorMsg = "❌ Multiple Connection Failures"
470+
default:
471+
errorMsg = "💀 Service Degraded"
472+
}
473+
474+
errorTitle := app.systrayInterface.AddMenuItem(errorMsg, "")
475+
errorTitle.Disable()
476+
477+
// Determine hostname and error type
478+
hostname := "api.github.com"
479+
for _, h := range []struct{ match, host string }{
480+
{"ready-to-review.dev", "dash.ready-to-review.dev"},
481+
{"api.github.com", "api.github.com"},
482+
{"github.com", "github.com"},
483+
} {
484+
if strings.Contains(lastFetchError, h.match) {
485+
hostname = h.host
486+
break
487+
}
488+
}
489+
490+
errorType := "Connection failed"
491+
for _, e := range []struct{ match, errType string }{
492+
{"timeout", "Request timeout"},
493+
{"context deadline", "Request timeout (context deadline)"},
494+
{"rate limit", "Rate limit exceeded"},
495+
{"401", "Authentication failed"},
496+
{"unauthorized", "Authentication failed"},
497+
{"403", "Access forbidden"},
498+
{"forbidden", "Access forbidden"},
499+
{"404", "Resource not found"},
500+
{"connection refused", "Connection refused"},
501+
{"no such host", "DNS resolution failed"},
502+
{"TLS", "TLS/Certificate error"},
503+
{"x509", "TLS/Certificate error"},
504+
} {
505+
if strings.Contains(lastFetchError, e.match) {
506+
errorType = e.errType
507+
break
508+
}
509+
}
510+
511+
// Show technical details
512+
techDetails := app.systrayInterface.AddMenuItem(fmt.Sprintf("Host: %s", hostname), "")
513+
techDetails.Disable()
514+
515+
errorTypeItem := app.systrayInterface.AddMenuItem(fmt.Sprintf("Error: %s", errorType), "")
516+
errorTypeItem.Disable()
517+
518+
// Show truncated raw error for debugging (max 80 chars)
519+
rawError := lastFetchError
520+
if len(rawError) > 80 {
521+
rawError = rawError[:77] + "..."
522+
}
523+
rawErrorItem := app.systrayInterface.AddMenuItem(fmt.Sprintf("Details: %s", rawError), "Click to copy full error")
524+
rawErrorItem.Click(func() {
525+
// Would need clipboard support to implement copy
526+
slog.Info("Full error", "error", lastFetchError)
527+
})
528+
529+
app.systrayInterface.AddSeparator()
530+
}
531+
452532
// Update tray title
453533
app.setTrayTitle()
454534

0 commit comments

Comments
 (0)