refactor: thread context.Context into both SQLite handles #452

New issue

Open

opened 2026-07-25 23:32:21 +02:00 by dries · 0 comments

dries commented

2026-07-25 23:32:21 +02:00

Owner

Code-quality audit (P2). Neither SQLite handle uses context-aware database/sql calls.

In internal/db/ and internal/state/: QueryContext / QueryRowContext / ExecContext appear 0 times in non-test code; Query / QueryRow / Exec appear 376 times.

Why it matters

Two concrete consequences:

No cancellation. An abandoned HTTP request keeps burning a multi-second aggregate query to completion. The session-list aggregate (internal/db/sessions.go:22) runs correlated subqueries over the whole message table; a client that navigates away cannot stop it.
Broken tracing. AGENTS.md advertises otelsql instrumentation on both handles, but otelsql derives the DB span from the context passed to the call. With Query/Exec there is no context, so every DB span is orphaned — it never attaches to the otelhttp server span. The tracing feature is effectively non-functional for DB work.

Suggested fix

Thread context.Context as the first parameter through the query layer and switch to the *Context variants. Do it incrementally, request-path first:

internal/db/sessions.go:122 and the other aggregates reachable from /api/sessions.
Remaining internal/db readers.
internal/state writers.

Handlers pass r.Context(); background loops pass their own ctx.

Acceptance criteria

Request-path DB calls take a ctx and use QueryContext / QueryRowContext / ExecContext.
A cancelled request aborts the in-flight query (test with a cancelled ctx asserting context.Canceled).
With OTEL enabled, a DB span appears as a child of the HTTP server span.

Effort: L.

Code-quality audit (P2). Neither SQLite handle uses context-aware `database/sql` calls. In `internal/db/` and `internal/state/`: `QueryContext` / `QueryRowContext` / `ExecContext` appear **0** times in non-test code; `Query` / `QueryRow` / `Exec` appear **376** times. ## Why it matters Two concrete consequences: 1. **No cancellation.** An abandoned HTTP request keeps burning a multi-second aggregate query to completion. The session-list aggregate (`internal/db/sessions.go:22`) runs correlated subqueries over the whole `message` table; a client that navigates away cannot stop it. 2. **Broken tracing.** AGENTS.md advertises `otelsql` instrumentation on both handles, but `otelsql` derives the DB span from the context passed to the call. With `Query`/`Exec` there is no context, so every DB span is orphaned — it never attaches to the `otelhttp` server span. The tracing feature is effectively non-functional for DB work. ## Suggested fix Thread `context.Context` as the first parameter through the query layer and switch to the `*Context` variants. Do it incrementally, request-path first: 1. `internal/db/sessions.go:122` and the other aggregates reachable from `/api/sessions`. 2. Remaining `internal/db` readers. 3. `internal/state` writers. Handlers pass `r.Context()`; background loops pass their own ctx. ## Acceptance criteria - [ ] Request-path DB calls take a `ctx` and use `QueryContext` / `QueryRowContext` / `ExecContext`. - [ ] A cancelled request aborts the in-flight query (test with a cancelled ctx asserting `context.Canceled`). - [ ] With OTEL enabled, a DB span appears as a child of the HTTP server span. Effort: L.