Skip to content

Commit fd87f39

Browse files
authored
fix: fall back to stderr (not stdout) when log-dir is unwritable (#5773)
When `awmg` cannot open log files in `--log-dir`, it was falling back to `os.Stdout` — corrupting the stdout JSON channel that `start_mcp_gateway.cjs` parses to get the gateway config. The caller sees a `SyntaxError` with no actionable error from `awmg`, since log lines like `[2026-...] [INFO] [startup]` are valid-looking JSON array starts. ## Changes - **`internal/logger/file_logger.go`** — `handleFileLoggerError`: `os.Stdout` → `os.Stderr`; `GetWriter()` fallback return: `os.Stdout` → `os.Stderr`; fallback warning message updated accordingly - **`internal/logger/common.go`** — Update doc comment describing the FileLogger fallback strategy - **`internal/logger/file_logger_test.go`** — Update test subcase and assertion to expect `os.Stderr` ```go // Before func handleFileLoggerError(err error, logDir, fileName string) (*FileLogger, error) { logFallbackWarnings(err, "Failed to initialize log file", "Falling back to stdout for logging") fl := &FileLogger{..., logger: log.New(os.Stdout, "", 0)} return fl, nil } // After func handleFileLoggerError(err error, logDir, fileName string) (*FileLogger, error) { logFallbackWarnings(err, "Failed to initialize log file", "Falling back to stderr for logging") fl := &FileLogger{..., logger: log.New(os.Stderr, "", 0)} return fl, nil } ```
2 parents ea3c8ab + da0b55e commit fd87f39

3 files changed

Lines changed: 17 additions & 13 deletions

File tree

internal/logger/common.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,21 +98,23 @@ import (
9898
//
9999
// Different logger types implement different fallback strategies based on their purpose:
100100
//
101-
// 1. FileLogger - Stdout Fallback:
101+
// 1. FileLogger - Stderr Fallback:
102102
// - Purpose: Operational logs must always be visible
103-
// - Fallback: Redirects to stdout if log directory/file creation fails
103+
// - Fallback: Redirects to stderr if log directory/file creation fails
104+
// (stderr is used, not stdout, to avoid corrupting the stdout
105+
// JSON channel that callers use to receive gateway config output)
104106
// - Error: Returns nil (never fails, always provides output)
105107
// - Use case: Critical operational messages that must be seen
106108
//
107109
// Example error handler:
108110
// func(err error, logDir, fileName string) (*FileLogger, error) {
109111
// log.Printf("WARNING: Failed to initialize log file: %v", err)
110-
// log.Printf("WARNING: Falling back to stdout for logging")
112+
// log.Printf("WARNING: Falling back to stderr for logging")
111113
// return &FileLogger{
112114
// logDir: logDir,
113115
// fileName: fileName,
114116
// useFallback: true,
115-
// logger: log.New(os.Stdout, "", 0),
117+
// logger: log.New(os.Stderr, "", 0),
116118
// }, nil
117119
// }
118120
//

internal/logger/file_logger.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"sync"
99
)
1010

11-
// FileLogger manages logging to a file with fallback to stdout
11+
// FileLogger manages logging to a file with fallback to stderr
1212
type FileLogger struct {
1313
lockable
1414
logFile *os.File
@@ -35,14 +35,16 @@ func setupFileLogger(file *os.File, logDir, fileName string) (*FileLogger, error
3535
return fl, nil
3636
}
3737

38-
// handleFileLoggerError falls back to stdout when the log file cannot be opened.
38+
// handleFileLoggerError falls back to stderr when the log file cannot be opened.
39+
// Stderr is used (not stdout) to avoid corrupting the stdout JSON channel that
40+
// callers use to receive the gateway configuration output.
3941
func handleFileLoggerError(err error, logDir, fileName string) (*FileLogger, error) {
40-
logFallbackWarnings(err, "Failed to initialize log file", "Falling back to stdout for logging")
42+
logFallbackWarnings(err, "Failed to initialize log file", "Falling back to stderr for logging")
4143
fl := &FileLogger{
4244
logDir: logDir,
4345
fileName: fileName,
4446
useFallback: true,
45-
logger: log.New(os.Stdout, "", 0),
47+
logger: log.New(os.Stderr, "", 0),
4648
}
4749
return fl, nil
4850
}
@@ -54,7 +56,7 @@ var fileLoggerFactory = loggerFactory[*FileLogger]{
5456
}
5557

5658
// InitFileLogger initializes the global file logger
57-
// If the log directory doesn't exist and can't be created, falls back to stdout
59+
// If the log directory doesn't exist and can't be created, falls back to stderr
5860
func InitFileLogger(logDir, fileName string) error {
5961
logger, err := initLogger(logDir, fileName, os.O_APPEND, fileLoggerFactory)
6062
initGlobalLogger(&globalLoggerMu, &globalFileLogger, logger)
@@ -103,7 +105,7 @@ func (fl *FileLogger) GetWriter() io.Writer {
103105
if fl.logFile != nil {
104106
return fl.logFile
105107
}
106-
return os.Stdout
108+
return os.Stderr
107109
}
108110

109111
// Global logging functions that use the global file logger

internal/logger/file_logger_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ func TestFileLoggerFlushes(t *testing.T) {
200200
}
201201

202202
// TestFileLogger_GetWriter verifies GetWriter returns the underlying file for a real
203-
// logger and os.Stdout for the fallback logger.
203+
// logger and os.Stderr for the fallback logger.
204204
func TestFileLogger_GetWriter(t *testing.T) {
205205
t.Run("real logger returns file writer", func(t *testing.T) {
206206
tmpDir := t.TempDir()
@@ -221,7 +221,7 @@ func TestFileLogger_GetWriter(t *testing.T) {
221221
assert.True(t, isFile, "GetWriter should return *os.File for real logger")
222222
})
223223

224-
t.Run("fallback logger returns stdout", func(t *testing.T) {
224+
t.Run("fallback logger returns stderr", func(t *testing.T) {
225225
err := InitFileLogger("/root/nonexistent/directory", "test.log")
226226
require.NoError(t, err)
227227
defer CloseGlobalLogger()
@@ -233,7 +233,7 @@ func TestFileLogger_GetWriter(t *testing.T) {
233233
require.NotNil(t, logger)
234234
if logger.useFallback {
235235
w := logger.GetWriter()
236-
assert.Equal(t, os.Stdout, w, "Fallback logger GetWriter should return os.Stdout")
236+
assert.Equal(t, os.Stderr, w, "Fallback logger GetWriter should return os.Stderr")
237237
} else {
238238
t.Skip("System has permissions to write to /root; cannot test fallback path")
239239
}

0 commit comments

Comments
 (0)