Skip to content

Commit 1430a57

Browse files
authored
fix panic during tab-completion (#688)
Tab completion uses a nil logger with a command where the logger was optional, but in #680, it calls a method on the logger that it previously didn't, which causes a panic. This change makes two fixes: - The function is updated to handle a nil logger like before. - All methods of Logger are made nil-safe to prevent issues like this in the future Resolves #687
1 parent 4fec474 commit 1430a57

File tree

6 files changed

+114
-3
lines changed

6 files changed

+114
-3
lines changed

.changes/v0.14.1.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
## <a name="v0.14.1">v0.14.1</a> - 2025-05-30
2+
### Fixed
3+
- Fix panic when using shell tab completion for a branch name.

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
55
adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html),
66
and is generated by [Changie](https://github.com/miniscruff/changie).
77

8+
## <a name="v0.14.1">v0.14.1</a> - 2025-05-30
9+
### Fixed
10+
- Fix panic when using shell tab completion for a branch name.
11+
812
## <a name="v0.14.0">v0.14.0</a> - 2025-05-28
913
### Added
1014
- branch checkout: Add 'spice.branchCheckout.trackUntrackedPrompt' configuration option to disable prompting to track untracked branches upon checkout.

internal/silog/log.go

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,25 +109,35 @@ func New(w io.Writer, opts *Options) *Logger {
109109
// Clone returns a new logger with the same configuration
110110
// as the original logger.
111111
func (l *Logger) Clone() *Logger {
112+
if l == nil {
113+
return l
114+
}
112115
newL := *l
113116
return &newL
114117
}
115118

116119
// Level returns the current log level of the logger.
117120
func (l *Logger) Level() Level {
121+
if l == nil {
122+
return LevelFatal + 1
123+
}
124+
118125
return Level(l.lvl.Level())
119126
}
120127

121128
// SetLevel changes the log level of the logger
122129
// and all loggers cloned from it.
123130
func (l *Logger) SetLevel(lvl Level) {
131+
if l == nil {
132+
return
133+
}
124134
l.lvl.Set(lvl.Level())
125135
}
126136

127137
// WithLevel returns a copy of this logger
128138
// that will log at the given level.
129139
func (l *Logger) WithLevel(lvl Level) *Logger {
130-
if lvl == l.Level() {
140+
if l == nil || lvl == l.Level() {
131141
return l
132142
}
133143

@@ -140,6 +150,9 @@ func (l *Logger) WithLevel(lvl Level) *Logger {
140150

141151
// WithGroup returns a copy of the logger with the given group name added.
142152
func (l *Logger) WithGroup(name string) *Logger {
153+
if l == nil || name == "" {
154+
return l
155+
}
143156
newL := l.Clone()
144157
newL.sl = newL.sl.WithGroup(name)
145158
return newL
@@ -151,6 +164,9 @@ func (l *Logger) WithGroup(name string) *Logger {
151164
// If the prefix is empty, an existing prefix will be removed.
152165
// If the prefix is non-empty, a ": " delimiter will be added.
153166
func (l *Logger) WithPrefix(prefix string) *Logger {
167+
if l == nil {
168+
return l
169+
}
154170
newL := l.Clone()
155171
newL.sl = slog.New(newL.sl.Handler().(*logHandler).WithPrefix(prefix))
156172
return newL
@@ -164,14 +180,17 @@ func (l *Logger) WithPrefix(prefix string) *Logger {
164180
// will be downgraded to LevelDebug,
165181
// Levels at the minimum level will be discarded.
166182
func (l *Logger) Downgrade() *Logger {
183+
if l == nil {
184+
return l
185+
}
167186
newL := l.Clone()
168187
newL.numDowngrades++
169188
return newL
170189
}
171190

172191
// With returns a copy of the logger with the given attributes added.
173192
func (l *Logger) With(attrs ...any) *Logger {
174-
if len(attrs) == 0 {
193+
if l == nil || len(attrs) == 0 {
175194
return l
176195
}
177196

@@ -182,6 +201,13 @@ func (l *Logger) With(attrs ...any) *Logger {
182201

183202
// Log logs a message at the given level with the given key-value pairs.
184203
func (l *Logger) Log(lvl Level, msg string, kvs ...any) {
204+
if l == nil {
205+
if lvl >= LevelFatal {
206+
_osExit(1) // exit on fatal regardless
207+
}
208+
return
209+
}
210+
185211
if l.numDowngrades > 0 {
186212
lvl = lvl.Dec(l.numDowngrades)
187213
}
@@ -229,4 +255,6 @@ func (l *Logger) Errorf(format string, args ...any) { l.Logf(LevelError, format,
229255
// It also exits the program with a non-zero status code.
230256
func (l *Logger) Fatalf(format string, args ...any) { l.Logf(LevelFatal, format, args...) }
231257

232-
func exitOnFatal() { os.Exit(1) }
258+
var _osExit = os.Exit // for testing
259+
260+
func exitOnFatal() { _osExit(1) }

internal/silog/log_int_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package silog
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestLogger_nilSafeFatal(t *testing.T) {
10+
defer func(old func(int)) {
11+
_osExit = old
12+
}(_osExit)
13+
14+
var exited bool
15+
_osExit = func(int) {
16+
exited = true
17+
}
18+
19+
var logger *Logger
20+
logger.Fatal("foo")
21+
assert.True(t, exited)
22+
23+
exited = false
24+
logger.Fatalf("foo %s", "bar")
25+
assert.True(t, exited)
26+
}

internal/silog/log_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,54 @@ func TestLogger_WithLevel(t *testing.T) {
375375
assert.Empty(t, buffer.String())
376376
}
377377

378+
func TestLogger_nilSafe(t *testing.T) {
379+
var logger *silog.Logger
380+
381+
t.Run("Clone", func(*testing.T) {
382+
_ = logger.Clone()
383+
})
384+
385+
t.Run("Level", func(*testing.T) {
386+
_ = logger.Level()
387+
})
388+
389+
t.Run("SetLevel", func(*testing.T) {
390+
logger.SetLevel(silog.LevelDebug)
391+
})
392+
393+
t.Run("WithLevel", func(*testing.T) {
394+
_ = logger.WithLevel(silog.LevelDebug)
395+
})
396+
397+
t.Run("WithGroup", func(*testing.T) {
398+
_ = logger.WithGroup("test")
399+
})
400+
401+
t.Run("WithPrefix", func(*testing.T) {
402+
_ = logger.WithPrefix("test")
403+
})
404+
405+
t.Run("Downgrade", func(*testing.T) {
406+
_ = logger.Downgrade()
407+
})
408+
409+
t.Run("With", func(*testing.T) {
410+
_ = logger.With("key", "value")
411+
})
412+
413+
t.Run("Log", func(*testing.T) { logger.Log(silog.LevelInfo, "test") })
414+
t.Run("Debug", func(*testing.T) { logger.Debug("test") })
415+
t.Run("Info", func(*testing.T) { logger.Info("test") })
416+
t.Run("Warn", func(*testing.T) { logger.Warn("test") })
417+
t.Run("Error", func(*testing.T) { logger.Error("test") })
418+
419+
t.Run("Logf", func(*testing.T) { logger.Logf(silog.LevelInfo, "test %s", "message") })
420+
t.Run("Debugf", func(*testing.T) { logger.Debugf("test %s", "message") })
421+
t.Run("Infof", func(*testing.T) { logger.Infof("test %s", "message") })
422+
t.Run("Warnf", func(*testing.T) { logger.Warnf("test %s", "message") })
423+
t.Run("Errorf", func(*testing.T) { logger.Errorf("test %s", "message") })
424+
}
425+
378426
type testStringer struct{ v string }
379427

380428
func (s *testStringer) String() string { return s.v }

repo_init.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package main
22

33
import (
4+
"cmp"
45
"context"
56
"errors"
67
"fmt"
@@ -126,6 +127,7 @@ const (
126127
)
127128

128129
func newRepoStorage(repo *git.Repository, log *silog.Logger) *storage.DB {
130+
log = cmp.Or(log, silog.Nop())
129131
return storage.NewDB(storage.NewGitBackend(storage.GitConfig{
130132
Repo: repo.WithLogger(log.Downgrade()),
131133
Ref: _dataRef,

0 commit comments

Comments
 (0)