Skip to content

Commit 8a11305

Browse files
fix: surface config save errors instead of silently swallowing them (#89)
SaveConfig() previously only returned an error when both keyring and file writes failed. Now logs independently when one backend fails. Also fixes AddTurn()/AppendTurn() to return save errors instead of silently logging them.
1 parent 2073502 commit 8a11305

5 files changed

Lines changed: 29 additions & 13 deletions

File tree

internal/config/config.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ func loadFromEnv() *Config {
128128
}
129129

130130
// SaveConfig stores host and token in both the system keyring and file storage.
131-
// It returns an error only if both storage methods fail.
132131
func SaveConfig(host, token string) error {
133132
if host != "" {
134133
validHost, err := ValidateAndTransformHost(host)
@@ -163,11 +162,20 @@ func SaveConfig(host, token string) error {
163162
cfg.GleanToken = token
164163
}
165164

166-
if err := saveToFile(cfg); err != nil && keyringErr != nil {
167-
return fmt.Errorf("failed to save config: keyring error: %v, file error: %v", keyringErr, err)
168-
}
165+
fileErr := saveToFile(cfg)
169166

170-
return nil
167+
switch {
168+
case keyringErr != nil && fileErr != nil:
169+
return fmt.Errorf("failed to save config: keyring: %v, file: %v", keyringErr, fileErr)
170+
case fileErr != nil:
171+
cfgLog.Log("warning: config file write failed (keyring OK): %v", fileErr)
172+
return nil
173+
case keyringErr != nil:
174+
cfgLog.Log("keyring unavailable, config saved to file only: %v", keyringErr)
175+
return nil
176+
default:
177+
return nil
178+
}
171179
}
172180

173181
// SaveHostToFile persists only the host in ~/.glean/config.json without touching

internal/config/config_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,8 @@ func TestConfigOperations(t *testing.T) {
270270
err = SaveConfig("linkedin", "test-token")
271271
assert.Error(t, err)
272272
assert.Contains(t, err.Error(), "failed to save config")
273-
assert.Contains(t, err.Error(), "keyring error")
274-
assert.Contains(t, err.Error(), "file error")
273+
assert.Contains(t, err.Error(), "keyring:")
274+
assert.Contains(t, err.Error(), "file:")
275275

276276
// Reset mock error for other tests
277277
mock.err = nil

internal/tui/commands.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,9 @@ func (m *Model) handleSlashCommand(input string) (tea.Model, tea.Cmd) {
118118
// System turns are rendered in the viewport but never sent to the Glean API.
119119
func (m *Model) addSystemMessage(text string) {
120120
turn := Turn{Role: roleSystem, Content: text}
121-
m.session.AppendTurn(turn)
121+
if err := m.session.AppendTurn(turn); err != nil {
122+
sessionLog.Log("save failed: %v", err)
123+
}
122124
if !m.conversationActive {
123125
m.conversationActive = true
124126
m.viewport.Height = m.maxViewportHeight()

internal/tui/model.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,9 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
410410
}
411411

412412
// Display turn uses original question for clean viewport rendering.
413-
m.session.AddTurn(roleUser, question, nil)
413+
if err := m.session.AddTurn(roleUser, question, nil); err != nil {
414+
sessionLog.Log("save failed: %v", err)
415+
}
414416

415417
// API message carries file context when present.
416418
apiText := apiContent
@@ -489,7 +491,9 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
489491
Elapsed: msg.elapsed,
490492
}
491493
m.addTurnToConversation(turn)
492-
m.session.AppendTurn(turn) // preserves Elapsed for renderConversation
494+
if err := m.session.AppendTurn(turn); err != nil {
495+
sessionLog.Log("save failed: %v", err)
496+
}
493497
}
494498
m.viewport.SetContent(m.renderConversation())
495499
m.viewport.GotoBottom()

internal/tui/session.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,17 @@ func (s *Session) Save() error {
8181
}
8282

8383
// AddTurn appends a turn to the session and saves immediately.
84-
func (s *Session) AddTurn(role, content string, sources []Source) {
85-
s.AppendTurn(Turn{Role: role, Content: content, Sources: sources})
84+
func (s *Session) AddTurn(role, content string, sources []Source) error {
85+
return s.AppendTurn(Turn{Role: role, Content: content, Sources: sources})
8686
}
8787

8888
// AppendTurn appends a complete Turn (including Elapsed and any other fields)
8989
// to the session and saves immediately.
90-
func (s *Session) AppendTurn(turn Turn) {
90+
func (s *Session) AppendTurn(turn Turn) error {
9191
s.Turns = append(s.Turns, turn)
9292
if err := s.Save(); err != nil {
9393
sessionLog.Log("save failed: %v", err)
94+
return err
9495
}
96+
return nil
9597
}

0 commit comments

Comments
 (0)