Skip to content

Commit ab925cf

Browse files
aofeiaofei
authored
refactor(Goproxy)!: redesign ErrorLogger as Logger using log/slog.Logger (#106)
Signed-off-by: Aofei Sheng <[email protected]>
1 parent 7973135 commit ab925cf

File tree

2 files changed

+60
-78
lines changed

2 files changed

+60
-78
lines changed

‎goproxy.go‎

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"fmt"
1010
"io"
1111
"io/fs"
12-
"log"
12+
"log/slog"
1313
"net/http"
1414
"net/url"
1515
"os"
@@ -67,15 +67,17 @@ type Goproxy struct {
6767
// If Transport is nil, [http.DefaultTransport] is used.
6868
Transport http.RoundTripper
6969

70-
// ErrorLogger is used to log errors that occur during proxying.
70+
// Logger is used to log messages that occur during proxying. It is
71+
// currently used only for error messages.
7172
//
72-
// If ErrorLogger is nil, [log.Default] is used.
73-
ErrorLogger *log.Logger
73+
// If Logger is nil, [slog.Default] is used.
74+
Logger *slog.Logger
7475

7576
initOnce sync.Once
7677
fetcher Fetcher
7778
proxiedSumDBs map[string]*url.URL
7879
httpClient *http.Client
80+
logger *slog.Logger
7981
}
8082

8183
// init initializes the g.
@@ -104,6 +106,11 @@ func (g *Goproxy) init() {
104106
}
105107

106108
g.httpClient = &http.Client{Transport: g.Transport}
109+
110+
g.logger = g.Logger
111+
if g.logger == nil {
112+
g.logger = slog.Default().WithGroup("goproxy")
113+
}
107114
}
108115

109116
// ServeHTTP implements [http.Handler].
@@ -203,7 +210,7 @@ func (g *Goproxy) serveFetchQuery(rw http.ResponseWriter, req *http.Request, tar
203210
version, time, err := g.fetcher.Query(req.Context(), modulePath, moduleQuery)
204211
if err != nil {
205212
g.serveCache(rw, req, target, contentType, cacheControlMaxAge, func() {
206-
g.logErrorf("failed to query module version: %s: %v", target, err)
213+
g.logger.Error("failed to query module version", "error", err, "target", target)
207214
responseError(rw, req, err, true)
208215
})
209216
return
@@ -224,7 +231,7 @@ func (g *Goproxy) serveFetchList(rw http.ResponseWriter, req *http.Request, targ
224231
versions, err := g.fetcher.List(req.Context(), modulePath)
225232
if err != nil {
226233
g.serveCache(rw, req, target, contentType, cacheControlMaxAge, func() {
227-
g.logErrorf("failed to list module versions: %s: %v", target, err)
234+
g.logger.Error("failed to list module versions", "error", err, "target", target)
228235
responseError(rw, req, err, true)
229236
})
230237
return
@@ -256,14 +263,14 @@ func (g *Goproxy) serveFetchDownload(rw http.ResponseWriter, req *http.Request,
256263
responseSuccess(rw, req, content, contentType, cacheControlMaxAge)
257264
return
258265
} else if !errors.Is(err, fs.ErrNotExist) {
259-
g.logErrorf("failed to get cached module file: %s: %v", target, err)
266+
g.logger.Error("failed to get cached module file", "error", err, "target", target)
260267
responseInternalServerError(rw, req)
261268
return
262269
}
263270

264271
info, mod, zip, err := g.fetcher.Download(req.Context(), modulePath, moduleVersion)
265272
if err != nil {
266-
g.logErrorf("failed to download module version: %s: %v", target, err)
273+
g.logger.Error("failed to download module version", "error", err, "target", target)
267274
responseError(rw, req, err, false)
268275
return
269276
}
@@ -283,7 +290,7 @@ func (g *Goproxy) serveFetchDownload(rw http.ResponseWriter, req *http.Request,
283290
{".zip", zip},
284291
} {
285292
if err := g.putCache(req.Context(), targetWithoutExt+cache.ext, cache.content); err != nil {
286-
g.logErrorf("failed to cache module file: %s: %v", target, err)
293+
g.logger.Error("failed to cache module file", "error", err, "target", target)
287294
responseInternalServerError(rw, req)
288295
return
289296
}
@@ -299,7 +306,7 @@ func (g *Goproxy) serveFetchDownload(rw http.ResponseWriter, req *http.Request,
299306
content = zip
300307
}
301308
if _, err := content.Seek(0, io.SeekStart); err != nil {
302-
g.logErrorf("failed to seek: %v", err)
309+
g.logger.Error("failed to seek content", "error", err)
303310
responseInternalServerError(rw, req)
304311
return
305312
}
@@ -345,7 +352,7 @@ func (g *Goproxy) serveSumDB(rw http.ResponseWriter, req *http.Request, target s
345352

346353
tempDir, err := os.MkdirTemp(g.TempDir, tempDirPattern)
347354
if err != nil {
348-
g.logErrorf("failed to create temporary directory: %v", err)
355+
g.logger.Error("failed to create temporary directory", "error", err)
349356
responseInternalServerError(rw, req)
350357
return
351358
}
@@ -354,7 +361,7 @@ func (g *Goproxy) serveSumDB(rw http.ResponseWriter, req *http.Request, target s
354361
file, err := httpGetTemp(req.Context(), g.httpClient, u.JoinPath(path).String(), tempDir)
355362
if err != nil {
356363
g.serveCache(rw, req, target, contentType, cacheControlMaxAge, func() {
357-
g.logErrorf("failed to proxy checksum database: %s: %v", target, err)
364+
g.logger.Error("failed to proxy checksum database", "error", err, "target", target)
358365
responseError(rw, req, err, true)
359366
})
360367
return
@@ -374,7 +381,7 @@ func (g *Goproxy) serveCache(rw http.ResponseWriter, req *http.Request, name, co
374381
}
375382
return
376383
}
377-
g.logErrorf("failed to get cached module file: %s: %v", name, err)
384+
g.logger.Error("failed to get cached module file", "error", err, "name", name)
378385
responseInternalServerError(rw, req)
379386
return
380387
}
@@ -385,12 +392,12 @@ func (g *Goproxy) serveCache(rw http.ResponseWriter, req *http.Request, name, co
385392
// servePutCache serves requests after putting the content to the g.Cacher.
386393
func (g *Goproxy) servePutCache(rw http.ResponseWriter, req *http.Request, name, contentType string, cacheControlMaxAge int, content io.ReadSeeker) {
387394
if err := g.putCache(req.Context(), name, content); err != nil {
388-
g.logErrorf("failed to cache module file: %s: %v", name, err)
395+
g.logger.Error("failed to cache module file", "error", err, "name", name)
389396
responseInternalServerError(rw, req)
390397
return
391398
}
392399
if _, err := content.Seek(0, io.SeekStart); err != nil {
393-
g.logErrorf("failed to seek: %v", err)
400+
g.logger.Error("failed to seek content", "error", err)
394401
responseInternalServerError(rw, req)
395402
return
396403
}
@@ -402,7 +409,7 @@ func (g *Goproxy) servePutCache(rw http.ResponseWriter, req *http.Request, name,
402409
func (g *Goproxy) servePutCacheFile(rw http.ResponseWriter, req *http.Request, name, contentType string, cacheControlMaxAge int, file string) {
403410
f, err := os.Open(file)
404411
if err != nil {
405-
g.logErrorf("failed to open file: %v", err)
412+
g.logger.Error("failed to open file", "error", err)
406413
responseInternalServerError(rw, req)
407414
return
408415
}
@@ -436,16 +443,6 @@ func (g *Goproxy) putCacheFile(ctx context.Context, name, file string) error {
436443
return g.putCache(ctx, name, f)
437444
}
438445

439-
// logErrorf formats according to a format specifier and writes to the g.ErrorLogger.
440-
func (g *Goproxy) logErrorf(format string, v ...any) {
441-
msg := "goproxy: " + fmt.Sprintf(format, v...)
442-
if g.ErrorLogger != nil {
443-
g.ErrorLogger.Output(2, msg)
444-
} else {
445-
log.Output(2, msg)
446-
}
447-
}
448-
449446
// cleanPath returns the canonical path for the p.
450447
func cleanPath(p string) string {
451448
if p == "" {

‎goproxy_test.go‎

Lines changed: 37 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"fmt"
99
"io"
1010
"io/fs"
11-
"log"
11+
"log/slog"
1212
"net/http"
1313
"net/http/httptest"
1414
"os"
@@ -139,9 +139,9 @@ func TestGoproxyServeHTTP(t *testing.T) {
139139
Env: []string{"GOPROXY=" + proxyServer.URL, "GOSUMDB=off"},
140140
TempDir: t.TempDir(),
141141
},
142-
Cacher: DirCacher(t.TempDir()),
143-
TempDir: t.TempDir(),
144-
ErrorLogger: log.New(io.Discard, "", 0),
142+
Cacher: DirCacher(t.TempDir()),
143+
TempDir: t.TempDir(),
144+
Logger: slog.New(slogDiscardHandler{}),
145145
}
146146
rec := httptest.NewRecorder()
147147
g.ServeHTTP(rec, httptest.NewRequest(tt.method, tt.path, nil))
@@ -430,9 +430,9 @@ func TestGoproxyServeFetch(t *testing.T) {
430430
Env: []string{"GOPROXY=" + proxyServer.URL, "GOSUMDB=off"},
431431
TempDir: t.TempDir(),
432432
},
433-
Cacher: tt.cacher,
434-
TempDir: t.TempDir(),
435-
ErrorLogger: log.New(io.Discard, "", 0),
433+
Cacher: tt.cacher,
434+
TempDir: t.TempDir(),
435+
Logger: slog.New(slogDiscardHandler{}),
436436
}
437437
g.initOnce.Do(g.init)
438438
req := httptest.NewRequest("", "/", nil)
@@ -518,9 +518,9 @@ func TestGoproxyServeFetchQuery(t *testing.T) {
518518
Env: []string{"GOPROXY=" + proxyServer.URL, "GOSUMDB=off"},
519519
TempDir: t.TempDir(),
520520
},
521-
Cacher: tt.cacher,
522-
TempDir: t.TempDir(),
523-
ErrorLogger: log.New(io.Discard, "", 0),
521+
Cacher: tt.cacher,
522+
TempDir: t.TempDir(),
523+
Logger: slog.New(slogDiscardHandler{}),
524524
}
525525
g.initOnce.Do(g.init)
526526
rec := httptest.NewRecorder()
@@ -594,9 +594,9 @@ func TestGoproxyServeFetchList(t *testing.T) {
594594
Env: []string{"GOPROXY=" + proxyServer.URL, "GOSUMDB=off"},
595595
TempDir: t.TempDir(),
596596
},
597-
Cacher: tt.cacher,
598-
TempDir: t.TempDir(),
599-
ErrorLogger: log.New(io.Discard, "", 0),
597+
Cacher: tt.cacher,
598+
TempDir: t.TempDir(),
599+
Logger: slog.New(slogDiscardHandler{}),
600600
}
601601
g.initOnce.Do(g.init)
602602
rec := httptest.NewRecorder()
@@ -777,9 +777,9 @@ func TestGoproxyServeFetchDownload(t *testing.T) {
777777
Env: []string{"GOPROXY=" + proxyServer.URL, "GOSUMDB=off"},
778778
TempDir: t.TempDir(),
779779
},
780-
Cacher: tt.cacher,
781-
TempDir: t.TempDir(),
782-
ErrorLogger: log.New(io.Discard, "", 0),
780+
Cacher: tt.cacher,
781+
TempDir: t.TempDir(),
782+
Logger: slog.New(slogDiscardHandler{}),
783783
}
784784
g.initOnce.Do(g.init)
785785
escapedModulePath, after, ok := strings.Cut(tt.target, "/@v/")
@@ -936,7 +936,7 @@ func TestGoproxyServeSumDB(t *testing.T) {
936936
ProxiedSumDBs: []string{"sumdb.example.com " + sumdbServer.URL},
937937
Cacher: tt.cacher,
938938
TempDir: tt.tempDir,
939-
ErrorLogger: log.New(io.Discard, "", 0),
939+
Logger: slog.New(slogDiscardHandler{}),
940940
}
941941
g.initOnce.Do(g.init)
942942
rec := httptest.NewRecorder()
@@ -1005,9 +1005,9 @@ func TestGoproxyServeCache(t *testing.T) {
10051005
tt.cacher = DirCacher(t.TempDir())
10061006
}
10071007
g := &Goproxy{
1008-
Cacher: tt.cacher,
1009-
TempDir: t.TempDir(),
1010-
ErrorLogger: log.New(io.Discard, "", 0),
1008+
Cacher: tt.cacher,
1009+
TempDir: t.TempDir(),
1010+
Logger: slog.New(slogDiscardHandler{}),
10111011
}
10121012
g.initOnce.Do(g.init)
10131013
req := httptest.NewRequest("", "/", nil)
@@ -1066,9 +1066,9 @@ func TestGoproxyServePutCache(t *testing.T) {
10661066
},
10671067
} {
10681068
g := &Goproxy{
1069-
Cacher: DirCacher(t.TempDir()),
1070-
TempDir: t.TempDir(),
1071-
ErrorLogger: log.New(io.Discard, "", 0),
1069+
Cacher: DirCacher(t.TempDir()),
1070+
TempDir: t.TempDir(),
1071+
Logger: slog.New(slogDiscardHandler{}),
10721072
}
10731073
g.initOnce.Do(g.init)
10741074
rec := httptest.NewRecorder()
@@ -1106,9 +1106,9 @@ func TestGoproxyServePutCacheFile(t *testing.T) {
11061106
},
11071107
} {
11081108
g := &Goproxy{
1109-
Cacher: DirCacher(t.TempDir()),
1110-
TempDir: t.TempDir(),
1111-
ErrorLogger: log.New(io.Discard, "", 0),
1109+
Cacher: DirCacher(t.TempDir()),
1110+
TempDir: t.TempDir(),
1111+
Logger: slog.New(slogDiscardHandler{}),
11121112
}
11131113
g.initOnce.Do(g.init)
11141114
rec := httptest.NewRecorder()
@@ -1205,32 +1205,6 @@ func TestGoproxyPutCacheFile(t *testing.T) {
12051205
}
12061206
}
12071207

1208-
func TestGoproxyLogErrorf(t *testing.T) {
1209-
for _, tt := range []struct {
1210-
n int
1211-
errorLogger *log.Logger
1212-
}{
1213-
{1, log.New(io.Discard, "", log.Ldate)},
1214-
{2, nil},
1215-
} {
1216-
var errorLoggerBuffer bytes.Buffer
1217-
g := &Goproxy{TempDir: t.TempDir(), ErrorLogger: tt.errorLogger}
1218-
g.initOnce.Do(g.init)
1219-
if g.ErrorLogger != nil {
1220-
g.ErrorLogger.SetOutput(&errorLoggerBuffer)
1221-
} else {
1222-
log.SetFlags(log.Ldate)
1223-
defer log.SetFlags(log.LstdFlags)
1224-
log.SetOutput(&errorLoggerBuffer)
1225-
defer log.SetOutput(os.Stderr)
1226-
}
1227-
g.logErrorf("not found: %s", "invalid version")
1228-
if got, want := errorLoggerBuffer.String(), fmt.Sprintf("%s goproxy: not found: invalid version\n", time.Now().Format("2006/01/02")); got != want {
1229-
t.Errorf("test(%d): got %q, want %q", tt.n, got, want)
1230-
}
1231-
}
1232-
}
1233-
12341208
func TestCleanPath(t *testing.T) {
12351209
for _, tt := range []struct {
12361210
n int
@@ -1347,3 +1321,14 @@ func (c *testCacher) Put(ctx context.Context, name string, content io.ReadSeeker
13471321
}
13481322
return c.Cacher.Put(ctx, name, content)
13491323
}
1324+
1325+
// slogDiscardHandler implements [slog.Handler] by discarding all logs.
1326+
//
1327+
// TODO: Remove slogDiscardHandler when the minimum supported Go version is
1328+
// 1.24. See https://go.dev/doc/go1.24#logslogpkglogslog.
1329+
type slogDiscardHandler struct{}
1330+
1331+
func (h slogDiscardHandler) Enabled(context.Context, slog.Level) bool { return false }
1332+
func (h slogDiscardHandler) Handle(context.Context, slog.Record) error { return nil }
1333+
func (h slogDiscardHandler) WithAttrs([]slog.Attr) slog.Handler { return h }
1334+
func (h slogDiscardHandler) WithGroup(string) slog.Handler { return h }

0 commit comments

Comments
 (0)