Skip to content

Commit 4ddb61b

Browse files
authored
Merge branch 'development' into fix/subscriber-error-swallowed
2 parents abfe4ba + 6ee34c3 commit 4ddb61b

5 files changed

Lines changed: 80 additions & 8 deletions

File tree

pkg/gofr/gofr.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"io"
78
"net"
89
"net/http"
910
"os"
@@ -110,12 +111,13 @@ func (a *App) Shutdown(ctx context.Context) error {
110111
err = errors.Join(err, a.metricServer.Shutdown(ctx))
111112
}
112113

113-
if err != nil {
114-
return err
115-
}
116-
117114
a.container.Logger.Info("Application shutdown complete")
118115

116+
// Close logger file if applicable
117+
if closer, ok := a.container.Logger.(io.Closer); ok {
118+
err = errors.Join(err, closer.Close())
119+
}
120+
119121
return err
120122
}
121123

pkg/gofr/gofr_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,40 @@ func TestNewCMD(t *testing.T) {
4040
assert.Contains(t, outputWithoutArgs, "is not a valid command", "TEST Failed.\n%s", "Stderr output mismatch")
4141
}
4242

43+
func TestNewCMD_FileLoggerClosedAfterRun(t *testing.T) {
44+
tempFile, err := os.CreateTemp(t.TempDir(), "gofr_cmd_test_log_*.log")
45+
require.NoError(t, err)
46+
47+
tempFile.Close() // Close it since NewFileLogger will open it.
48+
t.Setenv("CMD_LOGS_FILE", tempFile.Name())
49+
50+
originalArgs := os.Args
51+
os.Args = []string{"", "test-log"}
52+
53+
t.Cleanup(func() { os.Args = originalArgs })
54+
55+
a := NewCMD()
56+
57+
a.SubCommand("test-log", func(c *Context) (any, error) {
58+
c.Logger.Info("test log message in cmd")
59+
return "handler called", nil
60+
})
61+
62+
a.Run()
63+
64+
logBytes, err := os.ReadFile(tempFile.Name())
65+
require.NoError(t, err)
66+
67+
assert.Contains(t, string(logBytes), "test log message in cmd")
68+
69+
// Verify the logger is closed by checking if another Close() call returns an error.
70+
closer, ok := a.container.Logger.(io.Closer)
71+
require.True(t, ok, "logger should implement io.Closer")
72+
73+
err = closer.Close()
74+
assert.ErrorIs(t, err, os.ErrClosed)
75+
}
76+
4377
func TestGofr_readConfig(t *testing.T) {
4478
app := App{}
4579

pkg/gofr/logging/logger.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ type logger struct {
4747
errorOut io.Writer
4848
isTerminal bool
4949
lock chan struct{}
50+
file *os.File
5051
}
5152

5253
type logEntry struct {
@@ -144,8 +145,8 @@ func (l *logger) Fatal(args ...any) {
144145
l.logf(FATAL, "", args...)
145146

146147
// Flush output before exiting
147-
if f, ok := l.errorOut.(*os.File); ok {
148-
_ = f.Sync() // Ignore sync error as we're about to exit
148+
if l.file != nil {
149+
_ = l.file.Sync() // Ignore sync error as we're about to exit
149150
}
150151

151152
//nolint:revive // exit status is 1 as it denotes failure as signified by Fatal log
@@ -156,8 +157,8 @@ func (l *logger) Fatalf(format string, args ...any) {
156157
l.logf(FATAL, format, args...)
157158

158159
// Flush output before exiting
159-
if f, ok := l.errorOut.(*os.File); ok {
160-
_ = f.Sync() // Ignore sync error as we're about to exit
160+
if l.file != nil {
161+
_ = l.file.Sync() // Ignore sync error as we're about to exit
161162
}
162163

163164
//nolint:revive // exit status is 1 as it denotes failure as signified by Fatal log
@@ -227,10 +228,19 @@ func NewFileLogger(path string) Logger {
227228

228229
l.normalOut = f
229230
l.errorOut = f
231+
l.file = f
230232

231233
return l
232234
}
233235

236+
func (l *logger) Close() error {
237+
if l.file != nil {
238+
return l.file.Close()
239+
}
240+
241+
return nil
242+
}
243+
234244
func checkIfTerminal(w io.Writer) bool {
235245
// Force JSON output in test environments
236246
if os.Getenv("GOFR_EXITER") == "1" {

pkg/gofr/logging/logger_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,3 +281,22 @@ func TestNewFileLogger_NilPath(t *testing.T) {
281281
assert.Equal(t, io.Discard, logger.normalOut)
282282
assert.Equal(t, io.Discard, logger.errorOut)
283283
}
284+
285+
func TestNewFileLogger_Close(t *testing.T) {
286+
tempFile, err := os.CreateTemp(t.TempDir(), "gofr_test_log_*.log")
287+
require.NoError(t, err)
288+
289+
tempFile.Close() // Close it since NewFileLogger will open it.
290+
291+
l := NewFileLogger(tempFile.Name())
292+
293+
closer, ok := l.(io.Closer)
294+
require.True(t, ok, "logger should implement io.Closer")
295+
296+
err = closer.Close()
297+
require.NoError(t, err)
298+
299+
// verify that subsequent Close calls do not panic
300+
err = closer.Close()
301+
assert.ErrorIs(t, err, os.ErrClosed)
302+
}

pkg/gofr/run.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package gofr
33
import (
44
"context"
55
"errors"
6+
"io"
67
"net/http"
78
"os"
89
"os/signal"
@@ -15,6 +16,12 @@ import (
1516
func (a *App) Run() {
1617
if a.cmd != nil {
1718
a.cmd.Run(a.container)
19+
20+
if closer, ok := a.container.Logger.(io.Closer); ok {
21+
closer.Close()
22+
}
23+
24+
return
1825
}
1926

2027
// Create a context that is canceled on receiving termination signals

0 commit comments

Comments
 (0)