Report a zero-length or missing database that starts empty (closes #290)
check / check (push) Successful in 3m26s
check / check (push) Successful in 3m26s
Audit of the configuration and state read from files. Three cases carried on silently: a zero-length webhooker.db booted as a first start without the "created a new, empty database" warning, and a missing or zero-length per-webhook database was replaced by an empty one with only an INFO line, losing that webhook's events and pending deliveries unannounced. All three now log that warning with the file's path; CreateDB, which makes a new webhook's file, does not. The README now says how .env, the main, per-webhook and archive databases treat an empty file, and that a damaged per-webhook database fails that webhook alone. Model: opus-5-5
This commit is contained in:
@@ -79,7 +79,8 @@ directory, read once at startup before anything else looks at the
|
||||
environment.
|
||||
|
||||
The file is optional and having none is the normal case for a
|
||||
deployment. A file that is there but cannot be parsed aborts startup
|
||||
deployment. An empty file is the same as none: it has nothing in it to
|
||||
apply. A file that is there but cannot be parsed aborts startup
|
||||
with a message naming it, because a single malformed line makes none
|
||||
of the file apply: every variable in it silently reverts to its
|
||||
default, which is exactly the failure [Invalid values abort
|
||||
@@ -564,7 +565,8 @@ its Argon2id hash. There is no second account and no forgot-password
|
||||
flow, so the banner and the reset command below are the only two ways
|
||||
in.
|
||||
|
||||
A start that finds no `webhooker.db` in `DATA_DIR` also logs
|
||||
A start that finds no `webhooker.db` in `DATA_DIR`, or a zero-length
|
||||
one (which SQLite opens as an empty database), also logs
|
||||
`created a new, empty database` at `WARN`, with the file's path,
|
||||
shortly before the banner. On a deployment that has run before, that
|
||||
line means `DATA_DIR` was empty, most often because its volume is not
|
||||
@@ -1939,10 +1941,17 @@ encryption key is generated and stored, and an `admin` user is created.
|
||||
the deliveries per target, kept through retention
|
||||
|
||||
Per-webhook databases are created automatically when a webhook is
|
||||
created (and lazily on first access for webhooks that predate this
|
||||
feature). They are managed by the `WebhookDBManager` component, which
|
||||
created. They are managed by the `WebhookDBManager` component, which
|
||||
handles connection pooling, lazy opening, migrations, and cleanup.
|
||||
|
||||
A per-webhook database that is missing or zero-length later means its
|
||||
webhook's events and pending deliveries are gone. The next time it is
|
||||
opened, an empty one is created in its place, so the webhook keeps
|
||||
receiving, and `created a new, empty database` is logged at `WARN` with
|
||||
the file's path. A file there that SQLite cannot open fails that
|
||||
webhook alone, with an `ERROR` naming the webhook on every access and a
|
||||
500 to its senders, so one damaged file does not stop the others.
|
||||
|
||||
This separation provides:
|
||||
|
||||
- **Isolation** — a high-volume webhook won't cause lock contention or
|
||||
@@ -2007,7 +2016,9 @@ After each write the archive handle is closed
|
||||
and reopened, debounced to at most once per second, so an operator can
|
||||
move the archive file away for offline archiving without stopping the
|
||||
service; a moved or removed archive file is recreated automatically on
|
||||
the next write. An optional `expiry` in the target's config JSON (e.g.
|
||||
the next write. A zero-length archive file is written to as a new
|
||||
archive: SQLite opens it as an empty database, so it holds nothing to
|
||||
lose. An optional `expiry` in the target's config JSON (e.g.
|
||||
`{"expiry":"720h"}`) is validated when the target is created — the
|
||||
default (unset or the literal `never`) keeps rows forever — and rows
|
||||
older than the expiry are pruned each time the archive is (re)opened. An
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -119,3 +120,26 @@ func TestNewDatabase_IsLoggedWithItsPath(t *testing.T) {
|
||||
t, second, created, "an existing database is not new",
|
||||
)
|
||||
}
|
||||
|
||||
// TestZeroLengthDatabase_IsLoggedAsNew covers what
|
||||
// https://git.eeqj.de/sneak/webhooker/issues/290 found: SQLite opens a
|
||||
// zero-length file as an empty database, so a start on one is a first
|
||||
// start, and it must say so exactly as a start with no file does.
|
||||
func TestZeroLengthDatabase_IsLoggedAsNew(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, database.MainDBFileName)
|
||||
require.NoError(t, os.WriteFile(path, nil, database.SQLiteFilePerm))
|
||||
|
||||
var out bytes.Buffer
|
||||
|
||||
db, err := database.Open(dir, slog.New(slog.NewTextHandler(&out, nil)))
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, db.Close())
|
||||
|
||||
assert.Contains(
|
||||
t, out.String(),
|
||||
`level=WARN msg="created a new, empty database" path=`+path,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -8,7 +8,6 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/fs"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -203,8 +202,7 @@ func (d *Database) connectTo(dataDir string) error {
|
||||
// Checked before opening, which creates the file. A DATA_DIR that
|
||||
// is unexpectedly empty -- its volume not mounted, say -- looks
|
||||
// exactly like a first start, so a new database is a warning.
|
||||
_, statErr := os.Stat(dbPath)
|
||||
created := errors.Is(statErr, fs.ErrNotExist)
|
||||
created := missingOrEmpty(dbPath)
|
||||
|
||||
// Opened through OpenSQLite so this handle carries the same WAL
|
||||
// journaling, busy timeout, immediate-transaction locking, and pool
|
||||
|
||||
@@ -152,6 +152,20 @@ func reserveSQLiteFile(path string, create bool) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// missingOrEmpty reports whether opening path in SQLiteModeCreate
|
||||
// would start a new, empty database: the file is not there, or it is
|
||||
// zero-length, which SQLite opens as an empty database. A file left at
|
||||
// zero length by an interrupted first start or a truncated copy holds
|
||||
// as little as a missing one, and must be reported the same way.
|
||||
func missingOrEmpty(path string) bool {
|
||||
info, err := os.Stat(path)
|
||||
if errors.Is(err, fs.ErrNotExist) {
|
||||
return true
|
||||
}
|
||||
|
||||
return err == nil && info.Size() == 0
|
||||
}
|
||||
|
||||
// sqliteSidecarPaths returns the files SQLite maintains beside a
|
||||
// database under WAL. They carry the same rows as the database itself,
|
||||
// so a fix that tightens only the main file has fixed nothing.
|
||||
|
||||
@@ -98,34 +98,18 @@ func NewWebhookDBManager(
|
||||
return m, nil
|
||||
}
|
||||
|
||||
// GetDB returns the database connection for a webhook,
|
||||
// creating the database file lazily if it doesn't exist.
|
||||
// GetDB returns the database connection for a webhook, opening it on
|
||||
// first use.
|
||||
//
|
||||
// The file is made by CreateDB when the webhook is created. One that is
|
||||
// missing or zero-length here means the webhook's events and pending
|
||||
// deliveries are gone: an empty database is created in its place so
|
||||
// the webhook keeps receiving, and that is logged as a warning naming
|
||||
// the file, as a new main database is.
|
||||
func (m *WebhookDBManager) GetDB(
|
||||
webhookID string,
|
||||
) (*gorm.DB, error) {
|
||||
// Fast path: already open
|
||||
if val, ok := m.dbs.Load(webhookID); ok {
|
||||
return asGormDB(val, webhookID)
|
||||
}
|
||||
|
||||
// Slow path: open the database under the lock, looking in the
|
||||
// cache again first. A caller that raced another one here then
|
||||
// waits for its handle instead of opening a second one.
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
if val, ok := m.dbs.Load(webhookID); ok {
|
||||
return asGormDB(val, webhookID)
|
||||
}
|
||||
|
||||
db, err := m.openDB(webhookID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
m.dbs.Store(webhookID, db)
|
||||
|
||||
return db, nil
|
||||
return m.getDB(webhookID, false)
|
||||
}
|
||||
|
||||
// asGormDB returns a value read from the cache as the database
|
||||
@@ -143,12 +127,12 @@ func asGormDB(val any, webhookID string) (*gorm.DB, error) {
|
||||
return db, nil
|
||||
}
|
||||
|
||||
// CreateDB explicitly creates a new per-webhook database file
|
||||
// and runs migrations.
|
||||
// CreateDB creates a new webhook's database file and runs
|
||||
// migrations.
|
||||
func (m *WebhookDBManager) CreateDB(
|
||||
webhookID string,
|
||||
) error {
|
||||
_, err := m.GetDB(webhookID)
|
||||
_, err := m.getDB(webhookID, true)
|
||||
|
||||
return err
|
||||
}
|
||||
@@ -266,6 +250,48 @@ func (m *WebhookDBManager) DBPath(
|
||||
return m.dbPath(webhookID)
|
||||
}
|
||||
|
||||
// getDB is GetDB, and CreateDB when isNew is true: the webhook has just
|
||||
// been created, so a missing file is expected rather than lost.
|
||||
func (m *WebhookDBManager) getDB(
|
||||
webhookID string, isNew bool,
|
||||
) (*gorm.DB, error) {
|
||||
// Fast path: already open
|
||||
if val, ok := m.dbs.Load(webhookID); ok {
|
||||
return asGormDB(val, webhookID)
|
||||
}
|
||||
|
||||
// Slow path: open the database under the lock, looking in the
|
||||
// cache again first. A caller that raced another one here then
|
||||
// waits for its handle instead of opening a second one.
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
if val, ok := m.dbs.Load(webhookID); ok {
|
||||
return asGormDB(val, webhookID)
|
||||
}
|
||||
|
||||
// Checked before opening, which creates the file. See GetDB.
|
||||
path := m.dbPath(webhookID)
|
||||
replaced := !isNew && missingOrEmpty(path)
|
||||
|
||||
db, err := m.openDB(webhookID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if replaced {
|
||||
m.log.Warn(
|
||||
"created a new, empty database",
|
||||
"webhook_id", webhookID,
|
||||
"path", path,
|
||||
)
|
||||
}
|
||||
|
||||
m.dbs.Store(webhookID, db)
|
||||
|
||||
return db, nil
|
||||
}
|
||||
|
||||
func (m *WebhookDBManager) dbPath(
|
||||
webhookID string,
|
||||
) string {
|
||||
|
||||
@@ -289,6 +289,75 @@ func TestWebhookDBManager_LazyCreation(t *testing.T) {
|
||||
assert.True(t, mgr.DBExists(webhookID))
|
||||
}
|
||||
|
||||
// A webhook's database is made by CreateDB along with the webhook. One
|
||||
// that GetDB finds missing or zero-length has lost the webhook's events
|
||||
// and pending deliveries, so the empty database made in its place is
|
||||
// logged as a warning naming the file
|
||||
// (https://git.eeqj.de/sneak/webhooker/issues/290). CreateDB, and
|
||||
// reopening a database that is there, log no such warning.
|
||||
func TestWebhookDBManager_LostDatabaseIsLogged(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const created = `level=WARN msg="created a new, empty database"`
|
||||
|
||||
open := func(
|
||||
t *testing.T, prepare func(*database.WebhookDBManager, string),
|
||||
) (string, string) {
|
||||
t.Helper()
|
||||
|
||||
var logs bytes.Buffer
|
||||
|
||||
mgr := database.NewTestWebhookDBManagerWithLogger(
|
||||
t.TempDir(),
|
||||
slog.New(slog.NewTextHandler(&logs, nil)),
|
||||
)
|
||||
|
||||
webhookID := uuid.New().String()
|
||||
prepare(mgr, webhookID)
|
||||
|
||||
_, err := mgr.GetDB(webhookID)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, mgr.CloseAll())
|
||||
|
||||
return logs.String(),
|
||||
" webhook_id=" + webhookID + " path=" + mgr.DBPath(webhookID)
|
||||
}
|
||||
|
||||
t.Run("missing", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
logs, fields := open(
|
||||
t, func(*database.WebhookDBManager, string) {},
|
||||
)
|
||||
assert.Contains(t, logs, created+fields)
|
||||
})
|
||||
|
||||
t.Run("zero-length", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
logs, fields := open(
|
||||
t, func(mgr *database.WebhookDBManager, webhookID string) {
|
||||
require.NoError(t, os.WriteFile(
|
||||
mgr.DBPath(webhookID), nil, database.SQLiteFilePerm,
|
||||
))
|
||||
},
|
||||
)
|
||||
assert.Contains(t, logs, created+fields)
|
||||
})
|
||||
|
||||
t.Run("created with the webhook, then reopened", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
logs, _ := open(
|
||||
t, func(mgr *database.WebhookDBManager, webhookID string) {
|
||||
require.NoError(t, mgr.CreateDB(webhookID))
|
||||
require.NoError(t, mgr.CloseAll())
|
||||
},
|
||||
)
|
||||
assert.NotContains(t, logs, created)
|
||||
})
|
||||
}
|
||||
|
||||
func TestWebhookDBManager_DeliveryWorkflow(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user