Some bug fixes:

- logging: LogToFile shadowed the package-level fileLogger, so
  IsFileLogger was always false and Shutdown never closed the file
- logging: replace IsJSONHandler's unsafe reflection into slog
  internals with a *slog.JSONHandler type check
- cli: fail if the log file can't be opened instead of silently
  logging to the terminal; return the log shutdown error from Finish
- build.sh: -X flags used the wrong module path, so the version, build
  time and commit were never stamped
This commit is contained in:
bwilliams committed 2026-10-09 17:58:48 +01:00
1 parent 399eccf2de
commit dcb714dc19
7 files changed
+24 -45

No files matched your search

+4 -3
View File
@@ -1,6 +1,7 @@
package cli package cli
import ( import (
"fmt"
"log/slog" "log/slog"
"git.avsdev.uk/Go/job-search/internal/config" "git.avsdev.uk/Go/job-search/internal/config"
@@ -58,8 +59,8 @@ func (o *Options) RunSetup(
if !terminalLog { if !terminalLog {
if len(conf.Logging.File) == 0 { if len(conf.Logging.File) == 0 {
slog.Warn("No log file configured, remaining in console logging mode!") slog.Warn("No log file configured, remaining in console logging mode!")
} else { } else if err := logging.LogToFile(conf.Logging.File); err != nil {
logging.LogToFile(conf.Logging.File) return nil, fmt.Errorf("opening log file: %w", err)
} }
} }
} }
@@ -70,7 +71,7 @@ func (o *Options) RunSetup(
// Finish shuts down any open file logger // Finish shuts down any open file logger
func (o *Options) Finish() error { func (o *Options) Finish() error {
if logging.IsFileLogger() { if logging.IsFileLogger() {
logging.Shutdown() return logging.Shutdown()
} }
return nil return nil
+1 -1
View File
@@ -84,7 +84,7 @@ func (conf JobSearchConfig) String() string {
var sb strings.Builder var sb strings.Builder
for k, v := range conf.Flatten() { for k, v := range conf.Flatten() {
typ, _ := conf.TypeOf(k) typ, _ := conf.TypeOf(k)
sb.WriteString(fmt.Sprint(k, " = ", v, " <", typ, ">\n")) fmt.Fprint(&sb, k, " = ", v, " <", typ, ">\n")
} }
return sb.String() return sb.String()
} }
+1 -1
View File
@@ -32,7 +32,7 @@ func initDir(t *testing.T, env []string, config string) string {
func writeFile(t *testing.T, path, contents string) { func writeFile(t *testing.T, path, contents string) {
t.Helper() t.Helper()
if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { if err := os.WriteFile(path, []byte(contents), 0o600); err != nil {
t.Fatal(err) t.Fatal(err)
} }
} }
+5 -1
View File
@@ -16,7 +16,7 @@ func useConfigFile(t *testing.T, contents string) string {
t.Helper() t.Helper()
resetViper(t) resetViper(t)
path := filepath.Join(t.TempDir(), "config.yaml") path := filepath.Join(t.TempDir(), "config.yaml")
if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { if err := os.WriteFile(path, []byte(contents), 0o600); err != nil {
t.Fatal(err) t.Fatal(err)
} }
viper.SetConfigFile(path) viper.SetConfigFile(path)
@@ -92,6 +92,7 @@ func TestStoreValueErrors(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
path := useConfigFile(t, "logging:\n level: warn\n") path := useConfigFile(t, "logging:\n level: warn\n")
//nolint:gosec // G304: reading a file the test created in t.TempDir
before, _ := os.ReadFile(path) before, _ := os.ReadFile(path)
conf := defaultConfig(t) conf := defaultConfig(t)
@@ -102,6 +103,7 @@ func TestStoreValueErrors(t *testing.T) {
if !strings.Contains(err.Error(), tt.wantErr) { if !strings.Contains(err.Error(), tt.wantErr) {
t.Errorf("err = %v; want it to mention %q", err, tt.wantErr) t.Errorf("err = %v; want it to mention %q", err, tt.wantErr)
} }
//nolint:gosec // G304: reading a file the test created in t.TempDir
if after, _ := os.ReadFile(path); string(after) != string(before) { if after, _ := os.ReadFile(path); string(after) != string(before) {
t.Errorf("config file changed:\n%s", after) t.Errorf("config file changed:\n%s", after)
} }
@@ -135,12 +137,14 @@ func TestPurgeValue(t *testing.T) {
func TestPurgeValueErrors(t *testing.T) { func TestPurgeValueErrors(t *testing.T) {
path := useConfigFile(t, "logging:\n level: debug\n") path := useConfigFile(t, "logging:\n level: debug\n")
//nolint:gosec // G304: reading a file the test created in t.TempDir
before, _ := os.ReadFile(path) before, _ := os.ReadFile(path)
conf := defaultConfig(t) conf := defaultConfig(t)
if err := PurgeValue(&conf, "nope"); err == nil { if err := PurgeValue(&conf, "nope"); err == nil {
t.Error("expected an error for an unknown key") t.Error("expected an error for an unknown key")
} }
//nolint:gosec // G304: reading a file the test created in t.TempDir
if after, _ := os.ReadFile(path); string(after) != string(before) { if after, _ := os.ReadFile(path); string(after) != string(before) {
t.Errorf("config file changed:\n%s", after) t.Errorf("config file changed:\n%s", after)
} }
+5 -5
View File
@@ -19,14 +19,14 @@ type testEmbedded struct {
type testSection struct { type testSection struct {
testEmbedded testEmbedded
Str string `cfg:"str" default:"hello"` Str string `cfg:"str" default:"hello"`
Int int `cfg:"int" default:"-42"` Int int `cfg:"int" default:"-42"`
Int8 int8 `cfg:"int8"` Int8 int8 `cfg:"int8"`
Uint uint `cfg:"uint" default:"7"` Uint uint `cfg:"uint" default:"7"`
Uint16 uint16 `cfg:"uint16"` Uint16 uint16 `cfg:"uint16"`
Float float64 `cfg:"float" default:"1.5"` Float float64 `cfg:"float" default:"1.5"`
Float32 float32 `cfg:"float32"` Float32 float32 `cfg:"float32"`
Dur time.Duration `cfg:"dur" default:"90s"` Dur time.Duration `cfg:"dur" default:"90s"`
When time.Time `cfg:"when"` When time.Time `cfg:"when"`
Slice []string `cfg:"slice"` Slice []string `cfg:"slice"`
Inner testInner `cfg:"inner"` Inner testInner `cfg:"inner"`
+5 -31
View File
@@ -3,8 +3,6 @@ package logging
import ( import (
"log/slog" "log/slog"
"os" "os"
"reflect"
"unsafe"
"github.com/lmittmann/tint" "github.com/lmittmann/tint"
) )
@@ -33,7 +31,8 @@ func LogToFile(logfile string) error {
} }
// Open the log file // Open the log file
fileLogger, err := os.OpenFile(logfile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) var err error
fileLogger, err = os.OpenFile(logfile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600)
if err != nil { if err != nil {
return err return err
} }
@@ -67,35 +66,10 @@ func LogToTerminal() {
slog.SetDefault(logger) slog.SetDefault(logger)
} }
// IsJSONHandler reports whether h writes JSON, as the file logger does.
func IsJSONHandler(h slog.Handler) bool { func IsJSONHandler(h slog.Handler) bool {
val := reflect.ValueOf(h) _, ok := h.(*slog.JSONHandler)
return ok
// Ensure we are working with a pointer to a struct (*JSONHandler or *TextHandler or derived)
if val.Kind() != reflect.Pointer || val.Elem().Kind() != reflect.Struct {
return false
}
// Both JSONHandler and TextHandler wrap an unexported field named 'commonHandler'
commonHandlerField := val.Elem().FieldByName("commonHandler")
if !commonHandlerField.IsValid() {
return false
}
// Get the underlying *commonHandler struct pointer
// commonHandlerField is unexported, so we must use unsafe to bypass Go's safety checks
ptr := reflect.NewAt(
commonHandlerField.Type().Elem(),
unsafe.Pointer(commonHandlerField.Pointer()),
)
// Find the unexported 'json' boolean field inside commonHandler
jsonField := ptr.Elem().FieldByName("json")
if !jsonField.IsValid() {
return false
}
// Read the boolean value using unsafe
return *(*bool)(unsafe.Pointer(jsonField.UnsafeAddr()))
} }
func IsFileLogger() bool { func IsFileLogger() bool {
+3 -3
View File
@@ -31,12 +31,12 @@ if [ "$VERSION" != "$VERSION_TAG" ]; then
fi fi
LD_FLAGS=${LD_FLAGS:-""} LD_FLAGS=${LD_FLAGS:-""}
LD_FLAGS="$LD_FLAGS -X git.avsdev.uk/Go/JobSearch/internal/version.Version=${VERSION}" LD_FLAGS="$LD_FLAGS -X git.avsdev.uk/Go/job-search/internal/version.Version=${VERSION}"
if [[ $DO_RELEASE -ne 1 ]]; then if [[ $DO_RELEASE -ne 1 ]]; then
LD_FLAGS="$LD_FLAGS -X git.avsdev.uk/Go/JobSearch/internal/version.BuildTime=${BUILD_TIME}" LD_FLAGS="$LD_FLAGS -X git.avsdev.uk/Go/job-search/internal/version.BuildTime=${BUILD_TIME}"
fi fi
if [ "$GIT_COMMIT" != "" ]; then if [ "$GIT_COMMIT" != "" ]; then
LD_FLAGS="$LD_FLAGS -X git.avsdev.uk/Go/JobSearch/internal/version.GitCommit=${GIT_COMMIT}" LD_FLAGS="$LD_FLAGS -X git.avsdev.uk/Go/job-search/internal/version.GitCommit=${GIT_COMMIT}"
fi fi
# Process any generators # Process any generators