diff --git a/TODO.md b/TODO.md index ec56ac3..172a02c 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,17 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of # Completed Steps +- 2026-10-18: when the backend runs on its own, a `DATA_DIR` that cannot be + created or read, or where no file can be created, stops the start with an + error naming `DATA_DIR` and its value, as the other settings' errors do + ([#148](https://git.eeqj.de/sneak/netwatch/issues/148)); the first two named + only the path, and the third was found only at the first write of a report + file. At start the server creates a file in `DATA_DIR` and removes it. Unit + tests start the buffer with `DATA_DIR` below a regular file and with + `DATA_DIR` set to `/proc`, and check the error. The report file number is an + `atomic.Int64`, as the Go style guide asks for int types + ([#161](https://git.eeqj.de/sneak/netwatch/issues/161)); file names are + unchanged - 2026-10-18: a browser that cannot format the header's clocks still probes ([#149](https://git.eeqj.de/sneak/netwatch/issues/149)). The clocks name the local time zone through `Intl.DateTimeFormat`'s `shortGeneric` option, which diff --git a/internal/reportbuf/reportbuf.go b/internal/reportbuf/reportbuf.go index f4e925b..eadf3c1 100644 --- a/internal/reportbuf/reportbuf.go +++ b/internal/reportbuf/reportbuf.go @@ -88,7 +88,7 @@ type Buffer struct { now func() time.Time // seq numbers the report files, so that two named in the same // millisecond still get different names. - seq atomic.Uint64 + seq atomic.Int64 stopOnce sync.Once // usedBytes is what Append checks against maxBytes: the size // of the report files in dataDir, plus the reports waiting to @@ -127,16 +127,33 @@ func New( lc.Append(fx.Hook{ OnStart: func(_ context.Context) error { + // The errors name DATA_DIR, as config.New's do for the + // settings it checks. err := os.MkdirAll(b.dataDir, dirPerms) if err != nil { - return fmt.Errorf("create data dir: %w", err) + return fmt.Errorf("DATA_DIR %q: %w", b.dataDir, err) + } + + // Creating and removing a file stops the start when no + // file can be created in DATA_DIR, instead of the first + // write of a report file failing after reports were + // answered 200. A full disk is not caught: the empty + // file is still created. + f, err := os.CreateTemp(b.dataDir, "") + if err != nil { + return fmt.Errorf("DATA_DIR %q: %w", b.dataDir, err) + } + + err = errors.Join(f.Close(), os.Remove(f.Name())) + if err != nil { + return fmt.Errorf("DATA_DIR %q: %w", b.dataDir, err) } // Report files left by earlier runs count too, and are // the first to be deleted to make room. files, err := reportFiles(b.dataDir) if err != nil { - return err + return fmt.Errorf("DATA_DIR %q: %w", b.dataDir, err) } b.mu.Lock() diff --git a/internal/reportbuf/reportbuf_test.go b/internal/reportbuf/reportbuf_test.go index 9e277f4..b49bff9 100644 --- a/internal/reportbuf/reportbuf_test.go +++ b/internal/reportbuf/reportbuf_test.go @@ -108,6 +108,68 @@ func TestFailedFinalFlushFailsStop(t *testing.T) { } } +// TestUncreatableDataDirStopsStart sets DATA_DIR below a regular file, +// where no directory can be created, even by root, as the tests run in +// the backend image. The start must fail with an error naming DATA_DIR +// and its value, as the errors for the other settings do. +func TestUncreatableDataDirStopsStart(t *testing.T) { + file := filepath.Join(t.TempDir(), "file") + writeBytes(t, file, 1) + + dir := filepath.Join(file, "reports") + t.Setenv("DATA_DIR", dir) + + app := fxtest.New(t, + fx.Provide( + globals.New, + logger.New, + config.New, + reportbuf.New, + ), + fx.Invoke(func(*reportbuf.Buffer) {}), + ) + + err := app.Start(t.Context()) + if err == nil { + t.Fatal("start succeeded with DATA_DIR below a regular file") + } + + want := fmt.Sprintf("DATA_DIR %q: ", dir) + if !strings.Contains(err.Error(), want) { + t.Fatalf("start error = %q, want it to contain %q", err, want) + } +} + +// TestUnwritableDataDirStopsStart sets DATA_DIR to /proc, which exists +// and can be read, but where no file can be created, even by root, as +// the tests run in the backend image: root ignores a directory the test +// makes read-only. The start must fail with an error naming DATA_DIR and +// its value, rather than the first write of a report file. +func TestUnwritableDataDirStopsStart(t *testing.T) { + dir := "/proc" + t.Setenv("DATA_DIR", dir) + + app := fxtest.New(t, + fx.Provide( + globals.New, + logger.New, + config.New, + reportbuf.New, + ), + fx.Invoke(func(*reportbuf.Buffer) {}), + ) + + err := app.Start(t.Context()) + if err == nil { + t.Fatal("start succeeded with DATA_DIR /proc") + } + + want := fmt.Sprintf("DATA_DIR %q: ", dir) + if !strings.Contains(err.Error(), want) { + t.Fatalf("start error = %q, want it to contain %q", err, want) + } +} + // TestStopWaitsForPeriodicWrite holds open the write the periodic flush // starts, then stops the app. The stop must wait until the write is // released, and the report must then be in its file. The test runs in