--install --idempotent: non-SQL errors during the idempotency check fall through to a destructive install

[twiesing]

Description

The idempotency check in --install --idempotent only aborts on errors that are a *pq.Error. Any other error type — which is what connection-level failures surface as — is treated like "settings table does not exist", and installation proceeds. Since schema.sql drops every table before recreating it, this turns a transient connectivity blip into a full wipe of a populated database.

This is the flag from #247, intended for declarative/containerized deployments where the check must be reliable precisely in the situation where it is weakest: containers restarting, DB briefly unreachable.

The check

cmd/install.go (v6.2.0):

// If idempotence is on, check if the DB is already setup.
if idempotent {
    if _, err := db.Exec("SELECT count(*) FROM settings"); err != nil {
        // If "settings" doesn't exist, assume it's a fresh install.
        if pqErr, ok := err.(*pq.Error); ok && pqErr.Code != "42P01" {
            lo.Fatalf("error checking existing DB schema: %v", err)
        }
    } else {
        lo.Println("skipping install as database appears to be already setup")
        os.Exit(0)
    }
}

Only *pq.Error with a code other than 42P01 aborts. A dial failure (connection refused/reset, no route to host) surfaces — after database/sql exhausts its ErrBadConn retries — as e.g. a *net.OpError, not a *pq.Error. The type assertion fails, the Fatalf branch is skipped, and control falls through to installSchema() as if the database were fresh.

Why the consequence is severe

schema.sql prefixes every object with a drop:

DROP TYPE IF EXISTS list_type CASCADE;
...
DROP TABLE IF EXISTS subscribers CASCADE;
DROP TABLE IF EXISTS lists CASCADE;
...
DROP TABLE IF EXISTS settings CASCADE;

So a mistakenly-started install does not error out on an existing schema — it succeeds, replacing all data with the sample dataset. The idempotent path also skips the explicit "This will wipe existing listmonk tables" warning that a plain --install run prints.

Suggested fix

Let only the positive "table does not exist" signal trigger installation; abort on everything else. isTableNotExistErr() from cmd/upgrade.go already implements the right predicate (it returns false for non-pq errors):

if _, err := db.Exec("SELECT count(*) FROM settings"); err != nil {
    if !isTableNotExistErr(err) {
        lo.Fatalf("error checking existing DB schema: %v", err)
    }
    // 42P01: settings missing -> genuinely fresh, proceed with install.
} else {
    lo.Println("skipping install as database appears to be already setup")
    os.Exit(0)
}

Two related observations:

• The idempotency check probes settings, while checkSchema() (used before normal startup) probes templates. A partial schema containing one but not the other either crash-loops with "Run --install" while --install --idempotent keeps skipping, or triggers the destructive install. Using the same predicate in both places would make them consistent.
• Defense in depth: the idempotent path could refuse to execute schema.sql when any listmonk relation exists, since the schema is destructive by design.

Background: what we observed (reconstruction, for context)

We run listmonk 6.2.0 in a container stack with ./listmonk --install --idempotent --yes --config '' && ./listmonk --upgrade --yes --config '' && ./listmonk --config ''. During a host maintenance, both containers were restarted and container networking took a moment to converge. Our logs:

22:39:13  postgres: database system is ready to accept connections
22:39:16  listmonk: error connecting to DB: dial tcp 100.121.67.249:5432: connect: no route to host
22:39:21  listmonk (restarted): connecting to db: listmonk-db:5432/listmonk
22:39:21  ** first time (idempotent) installation **
22:41:37  install.go: setup complete

On every previous restart the same setup logged skipping install as database appears to be already setup. After this one, the database contained only the sample data and default settings; we restored from a nightly dump. We cannot prove from the logs alone that the check hit a non-pq error at 22:39 (the banner prints before the check runs), so we present this as a plausible reconstruction — the code path above is the only one we found that matches the observed outcome. Full listmonk/PostgreSQL logs are available if useful.