DEBUG turns debug logging on for any non-empty value (closes #159)
check / check (push) Waiting to run

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
This commit is contained in:
2026-10-19 02:01:41 +00:00
parent 648c225919
commit 4e660530cb
4 changed files with 65 additions and 22 deletions
+7 -6
View File
@@ -219,7 +219,7 @@ image, `make run` builds it and runs it on port 8080.
| `PORT` | `8080` | HTTP listen port | | `PORT` | `8080` | HTTP listen port |
| `DATA_DIR` | `./data/reports` | Directory for compressed reports | | `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) | | `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 | | `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) | | `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) | | `CORS_ALLOWED_ORIGINS` | empty | Comma-separated origins whose pages may call the API; see [CORS](#cors) |
@@ -242,10 +242,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 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). container image does not use this default; see [Deployment](#deployment).
A variable set to a value the server cannot use, such as `PORT=abc`, A variable set to a value the server cannot use, such as `PORT=abc` or a
`DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from `BIND_ADDRESS` that is not an IP address, stops it from starting, with an error
starting, with an error naming the variable. An empty variable counts as unset, naming the variable. An empty variable counts as unset, except
except `TRUSTED_PROXIES`. `TRUSTED_PROXIES`. No value of `DEBUG` stops the start.
#### Report storage #### Report storage
@@ -508,7 +508,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 report files may take; the oldest are deleted to stay under it
- `CORS_ALLOWED_ORIGINS`, default empty: other origins whose pages may call - `CORS_ALLOWED_ORIGINS`, default empty: other origins whose pages may call
the API 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 - `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 `/`. 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 The container also stops if the path goes through a symbolic link that
+5
View File
@@ -21,6 +21,11 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
# Completed Steps # 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; only `true`, `false`, `1`, `0` and their short forms were
accepted before. `README.md` no longer gives `false` as its default
- 2026-10-19: the three unit tests of a check whose result is discarded (its - 2026-10-19: the three unit tests of a check whose result is discarded (its
round given up, the user paused, or the first round) also check that the round given up, the user paused, or the first round) also check that the
host's history is still empty, its status still `pending`, and nothing was host's history is still empty, its status still `pending`, and nothing was
+8 -11
View File
@@ -41,7 +41,6 @@ var (
"must be an origin, scheme://host with an optional port", "must be an origin, scheme://host with an optional port",
) )
errNotPort = errors.New("must be a port number, 1 to 65535") 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") errNotIP = errors.New("must be an IP address, or empty")
errMetricsCredentials = errors.New( errMetricsCredentials = errors.New(
@@ -101,7 +100,7 @@ func New(
viper.SetDefault("CORS_ALLOWED_ORIGINS", "") viper.SetDefault("CORS_ALLOWED_ORIGINS", "")
viper.SetDefault("DATA_DIR", "./data/reports") viper.SetDefault("DATA_DIR", "./data/reports")
viper.SetDefault("DATA_DIR_MAX_BYTES", defaultDataDirMaxBytes) viper.SetDefault("DATA_DIR_MAX_BYTES", defaultDataDirMaxBytes)
viper.SetDefault("DEBUG", "false") viper.SetDefault("DEBUG", "")
// An empty BIND_ADDRESS listens on every interface. // An empty BIND_ADDRESS listens on every interface.
viper.SetDefault("BIND_ADDRESS", "") viper.SetDefault("BIND_ADDRESS", "")
viper.SetDefault("PORT", "8080") viper.SetDefault("PORT", "8080")
@@ -120,19 +119,17 @@ func New(
} }
} }
// Read with strconv: viper's GetInt and GetBool would read a value // Read with strconv: viper's GetInt would read a value it cannot
// they cannot parse as 0 or false instead of failing. // parse as 0 instead of failing.
port, err := strconv.Atoi(viper.GetString("PORT")) port, err := strconv.Atoi(viper.GetString("PORT"))
if err != nil || port < 1 || port > math.MaxUint16 { if err != nil || port < 1 || port > math.MaxUint16 {
return nil, fmt.Errorf("PORT %q: %w", return nil, fmt.Errorf("PORT %q: %w",
viper.GetString("PORT"), errNotPort) viper.GetString("PORT"), errNotPort)
} }
debug, err := strconv.ParseBool(viper.GetString("DEBUG")) // Any non-empty value turns debug logging on, "false" and "0"
if err != nil { // included, as the code style guide says.
return nil, fmt.Errorf("DEBUG %q: %w", debug := viper.GetString("DEBUG") != ""
viper.GetString("DEBUG"), errNotBool)
}
// An empty TRUSTED_PROXIES trusts no proxy, but viper reads an // An empty TRUSTED_PROXIES trusts no proxy, but viper reads an
// empty environment variable as unset and would give the default, // 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 // 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, // the server cannot use. New checks PORT as it reads it, and the
// and the middleware checks TRUSTED_PROXIES as it parses it. // middleware checks TRUSTED_PROXIES as it parses it.
func (s *Config) check() error { func (s *Config) check() error {
// viper reads a value that is not a number as 0, so this also // viper reads a value that is not a number as 0, so this also
// catches a mistyped setting. // catches a mistyped setting.
+45 -5
View File
@@ -127,12 +127,52 @@ func TestPortMustBeAPortNumber(t *testing.T) {
} }
} }
// TestDebugMustBeTrueOrFalse: viper reads any other value, such as // TestDebugIsOnForAnyNonEmptyValue: unset or empty, debug logging is
// "yes", as false. // off; any other value turns it on, "false" and "0" included, and none
func TestDebugMustBeTrueOrFalse(t *testing.T) { // stops the start.
t.Setenv("DEBUG", "yes") 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 // TestBindAddressMustBeAnIPAddress: a host name would be looked up