From f5aa81f024fb50761fe495edf957315a08584905 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 19 Oct 2026 05:48:20 +0200 Subject: [PATCH] DEBUG turns debug logging on for any non-empty value (closes #159) DEBUG was read with strconv.ParseBool, so a value such as "yes" stopped the start. It now follows the code style guide: unset or empty is off, and any other value, "false" and "0" included, turns debug logging on. No value of DEBUG stops the start. The test that "yes" fails is replaced by one covering unset, empty, "false", "0" and "yes". README.md no longer gives false as the default, and says which values turn it on. Model: opus-5-5 --- README.md | 13 +++++---- TODO.md | 6 ++++ internal/config/config.go | 19 ++++++------- internal/config/config_test.go | 50 ++++++++++++++++++++++++++++++---- 4 files changed, 66 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 3ab3c67..13ccd83 100644 --- a/README.md +++ b/README.md @@ -231,7 +231,7 @@ image, `make run` builds it and runs it on port 8080. | `PORT` | `8080` | HTTP listen port | | `DATA_DIR` | `./data/reports` | Directory for compressed reports | | `DATA_DIR_MAX_BYTES` | `1073741824` (1 GiB) | Most bytes of report files kept in `DATA_DIR`, oldest deleted first; see [Report limits](#report-limits) | -| `DEBUG` | `false` | Enable debug logging | +| `DEBUG` | empty | Debug logging; any non-empty value, `false` and `0` included, turns it on | | `TRUSTED_PROXIES` | RFC 1918 ranges | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution | | `REPORTS_PER_MINUTE` | `60` | Reports each client address may send a minute; see [Report limits](#report-limits) | | `CORS_ALLOWED_ORIGINS` | empty | Comma-separated origins whose pages may call the API; see [CORS](#cors) | @@ -254,10 +254,10 @@ an entry that is not an address at all, such as `unknown`, the direct peer is used. `X-Real-IP` is read only when `X-Forwarded-For` is missing or empty. The container image does not use this default; see [Deployment](#deployment). -A variable set to a value the server cannot use, such as `PORT=abc`, -`DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from -starting, with an error naming the variable. An empty variable counts as unset, -except `TRUSTED_PROXIES`. +A variable set to a value the server cannot use, such as `PORT=abc` or a +`BIND_ADDRESS` that is not an IP address, stops it from starting, with an error +naming the variable. An empty variable counts as unset, except +`TRUSTED_PROXIES`. No value of `DEBUG` stops the start. #### Report storage @@ -520,7 +520,8 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs: report files may take; the oldest are deleted to stay under it - `CORS_ALLOWED_ORIGINS`, default empty: other origins whose pages may call the API - - `DEBUG`, default `false`: debug logging + - `DEBUG`, default empty: debug logging; any non-empty value, `false` and + `0` included, turns it on - `DATA_DIR`, default `/data/reports`: the directory the reports are kept in: `/data` or a path below it, with no `.` or `..` part and no extra `/`. The container also stops if the path goes through a symbolic link that diff --git a/TODO.md b/TODO.md index 7d26a5d..6fbeb9f 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,12 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of # Completed Steps +- 2026-10-19: `DEBUG` unset or empty leaves debug logging off, and any other + value, `false` and `0` included, turns it on, as the code style guide says + ([#159](https://git.eeqj.de/sneak/netwatch/issues/159)). No value of it stops + the start any more; before, only `1`, `t`, `T`, `TRUE`, `true`, `True`, `0`, + `f`, `F`, `FALSE`, `false` and `False` were accepted. `README.md` no longer + gives `false` as its default - 2026-10-19: check builds keep Go's modules and compiled packages between runs ([#118](https://git.eeqj.de/sneak/netwatch/issues/118)), on sneak's rulings in [prompts#113](https://git.eeqj.de/sneak/prompts/issues/113) and diff --git a/internal/config/config.go b/internal/config/config.go index 69cca58..888afa1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -41,7 +41,6 @@ var ( "must be an origin, scheme://host with an optional port", ) errNotPort = errors.New("must be a port number, 1 to 65535") - errNotBool = errors.New("must be true or false") errNotIP = errors.New("must be an IP address, or empty") errMetricsCredentials = errors.New( @@ -101,7 +100,7 @@ func New( viper.SetDefault("CORS_ALLOWED_ORIGINS", "") viper.SetDefault("DATA_DIR", "./data/reports") viper.SetDefault("DATA_DIR_MAX_BYTES", defaultDataDirMaxBytes) - viper.SetDefault("DEBUG", "false") + viper.SetDefault("DEBUG", "") // An empty BIND_ADDRESS listens on every interface. viper.SetDefault("BIND_ADDRESS", "") viper.SetDefault("PORT", "8080") @@ -120,19 +119,17 @@ func New( } } - // Read with strconv: viper's GetInt and GetBool would read a value - // they cannot parse as 0 or false instead of failing. + // Read with strconv: viper's GetInt would read a value it cannot + // parse as 0 instead of failing. port, err := strconv.Atoi(viper.GetString("PORT")) if err != nil || port < 1 || port > math.MaxUint16 { return nil, fmt.Errorf("PORT %q: %w", viper.GetString("PORT"), errNotPort) } - debug, err := strconv.ParseBool(viper.GetString("DEBUG")) - if err != nil { - return nil, fmt.Errorf("DEBUG %q: %w", - viper.GetString("DEBUG"), errNotBool) - } + // Any non-empty value turns debug logging on, "false" and "0" + // included, as the code style guide says. + debug := viper.GetString("DEBUG") != "" // An empty TRUSTED_PROXIES trusts no proxy, but viper reads an // empty environment variable as unset and would give the default, @@ -172,8 +169,8 @@ func New( } // check fails with an error naming the first setting here whose value -// the server cannot use. New checks PORT and DEBUG as it reads them, -// and the middleware checks TRUSTED_PROXIES as it parses it. +// the server cannot use. New checks PORT as it reads it, and the +// middleware checks TRUSTED_PROXIES as it parses it. func (s *Config) check() error { // viper reads a value that is not a number as 0, so this also // catches a mistyped setting. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7de411a..43a15a5 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -127,12 +127,52 @@ func TestPortMustBeAPortNumber(t *testing.T) { } } -// TestDebugMustBeTrueOrFalse: viper reads any other value, such as -// "yes", as false. -func TestDebugMustBeTrueOrFalse(t *testing.T) { - t.Setenv("DEBUG", "yes") +// TestDebugIsOnForAnyNonEmptyValue: unset or empty, debug logging is +// off; any other value turns it on, "false" and "0" included, and none +// stops the start. +func TestDebugIsOnForAnyNonEmptyValue(t *testing.T) { + for _, tc := range []struct { + name string + value string + unset bool + want bool + }{ + {name: "unset", unset: true, want: false}, + {name: "empty", value: "", want: false}, + {name: "false", value: "false", want: true}, + {name: "0", value: "0", want: true}, + {name: "yes", value: "yes", want: true}, + } { + t.Run(tc.name, func(t *testing.T) { + // t.Setenv first, so the variable is put back as it was + // after the test. + t.Setenv("DEBUG", tc.value) - requireConfigError(t, "DEBUG") + if tc.unset { + err := os.Unsetenv("DEBUG") + if err != nil { + t.Fatal(err) + } + } + + var cfg *config.Config + + app := fx.New( + fx.NopLogger, + fx.Provide(globals.New, logger.New, config.New), + fx.Populate(&cfg), + ) + + err := app.Err() + if err != nil { + t.Fatalf("config error = %v", err) + } + + if cfg.Debug != tc.want { + t.Fatalf("Debug = %t, want %t", cfg.Debug, tc.want) + } + }) + } } // TestBindAddressMustBeAnIPAddress: a host name would be looked up