Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,25 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad
copied price is how a paid meeting quietly starts selling for nothing. Bookings are not
copied.

### Fixed
- **Constraint violations are recognised by SQLite's error code rather than by its
English message.** Thirteen call sites asked `strings.Contains(err.Error(), "UNIQUE
constraint failed")`, and SQLite reports a PRIMARY KEY collision
(`SQLITE_CONSTRAINT_PRIMARYKEY`, 1555) with that exact message while giving it a
different code from an ordinary unique violation (`SQLITE_CONSTRAINT_UNIQUE`, 2067).
The text could not tell the two apart, so nothing that needed to distinguish them
could.

`db.IsUniqueViolation`, `db.IsCheckViolation` and `db.IsForeignKeyViolation` answer
from the driver's code, falling back to the message only for an error that arrives
without its driver type still attached. A driver error whose code does not match is a
definite no rather than a fall-through, so an error cannot be classified by whether
its text happened to contain an English phrase.

Each class is provoked against the real schema in a test rather than constructed by
hand, including the primary-key case, which is the one a code match written from the
message alone would get wrong.

## [0.8.0] - 2026-09-03

### Added
Expand Down
12 changes: 4 additions & 8 deletions internal/booking/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"
"time"

"github.com/calnode/calnode/internal/db"
"github.com/calnode/calnode/internal/uid"
)

Expand Down Expand Up @@ -141,7 +142,7 @@ func (s *Service) Create(ctx context.Context, p CreateParams) (*Booking, error)
VALUES (?, ?, ?, ?, ?, 'confirmed', ?, ?, ?)`,
bookingID, p.EventTypeID, chosenHost, startStr, endStr, p.LocationValue, now, now)
if err != nil {
if isUniqueViolation(err) {
if db.IsUniqueViolation(err) {
return nil, ErrDoubleBooked
}
return nil, fmt.Errorf("booking: insert: %w", err)
Expand Down Expand Up @@ -483,7 +484,7 @@ func (s *Service) Reschedule(ctx context.Context, bookingID string, newStart, ne
if _, err := tx.ExecContext(ctx, `
UPDATE bookings SET start_at = ?, end_at = ?, updated_at = ? WHERE id = ?`,
startStr, endStr, now, bookingID); err != nil {
if isUniqueViolation(err) {
if db.IsUniqueViolation(err) {
return nil, ErrDoubleBooked
}
return nil, fmt.Errorf("booking: reschedule update: %w", err)
Expand Down Expand Up @@ -540,7 +541,7 @@ func (s *Service) ReassignHost(ctx context.Context, bookingID, newHostID string)
if _, err := tx.ExecContext(ctx, `
UPDATE bookings SET host_id = ?, updated_at = ? WHERE id = ?`,
newHostID, now, bookingID); err != nil {
if isUniqueViolation(err) {
if db.IsUniqueViolation(err) {
return nil, ErrDoubleBooked
}
return nil, fmt.Errorf("booking: reassign update: %w", err)
Expand Down Expand Up @@ -651,8 +652,3 @@ func scanBooking(s scanner) (*Booking, error) {
}
return &b, nil
}

// isUniqueViolation reports whether err is a SQLite UNIQUE constraint failure.
func isUniqueViolation(err error) bool {
return strings.Contains(err.Error(), "UNIQUE constraint failed")
}
85 changes: 85 additions & 0 deletions internal/db/constraint.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package db

import (
"errors"
"slices"
"strings"

"modernc.org/sqlite"
)

// Constraint violations are the one class of database error Calnode routinely acts
// on rather than just reporting: a duplicate slug is a 409, an out-of-range value is
// a 400, a dangling reference is a 404. Deciding which is which was a substring match
// on SQLite's English message, which is invisible to every gate and, as the codes
// below show, cannot actually distinguish the two cases it is asked to.
//
// SQLite's extended result codes, as reported by (*sqlite.Error).Code().
//
// ⛔ SQLITE_CONSTRAINT_PRIMARYKEY is a SEPARATE code from SQLITE_CONSTRAINT_UNIQUE
// even though both carry the message "UNIQUE constraint failed". A text match cannot
// tell them apart at all, and matching only 2067 would silently stop recognising
// primary-key collisions. Calnode has one that matters:
// idempotency_keys.idempotency_key is a bare PRIMARY KEY, so every idempotent replay
// arrives as 1555. Both belong to IsUniqueViolation.
const (
sqliteConstraintCheck = 275 // SQLITE_CONSTRAINT_CHECK
sqliteConstraintForeignKey = 787 // SQLITE_CONSTRAINT_FOREIGNKEY
sqliteConstraintPrimaryKey = 1555 // SQLITE_CONSTRAINT_PRIMARYKEY
sqliteConstraintUnique = 2067 // SQLITE_CONSTRAINT_UNIQUE
)

// SQLite's message fragments, used only as a fallback — see violates.
const (
sqliteUniqueText = "UNIQUE constraint failed"
sqliteCheckText = "CHECK constraint failed"
sqliteForeignKeyText = "FOREIGN KEY constraint failed"
)

// IsUniqueViolation reports whether err is a unique-constraint violation — a
// duplicate slug, a replayed idempotency key, a second booking at one host's exact
// start time. A primary-key collision counts.
func IsUniqueViolation(err error) bool {
return violates(err, sqliteUniqueText, sqliteConstraintUnique, sqliteConstraintPrimaryKey)
}

// IsCheckViolation reports whether err is a CHECK-constraint violation, i.e. a value
// outside the set the column allows. Callers turn this into a 400, since the only way
// to reach it is a request carrying a value the handler did not validate.
func IsCheckViolation(err error) bool {
return violates(err, sqliteCheckText, sqliteConstraintCheck)
}

// IsForeignKeyViolation reports whether err is a foreign-key violation — a reference
// to a row that does not exist, or a delete that would orphan one.
func IsForeignKeyViolation(err error) bool {
return violates(err, sqliteForeignKeyText, sqliteConstraintForeignKey)
}

// violates classifies err: the driver's own error code when one is available, the
// message only when it is not.
//
// A driver error is a DEFINITE answer in both directions. A *sqlite.Error whose code
// does not match returns false and does not fall through to the text comparison —
// falling through would classify an error by whether its message happened to contain
// an English phrase, which is the fragility being removed. It would also reintroduce
// the primary-key trap in reverse: a 1555 error excluded by code would be readmitted
// by its "UNIQUE constraint failed" message.
//
// The text fallback is deliberate rather than vestigial. It covers an error that
// reaches here without the concrete driver type still attached — a driver release
// that changes its error type, a layer that reformats an error into a plain one
// instead of wrapping it. In that case the message is the only signal left, and
// answering from it beats answering "not a constraint violation" and returning a 500.
func violates(err error, sqliteText string, sqliteCodes ...int) bool {
if err == nil {
return false
}

var sqliteErr *sqlite.Error
if errors.As(err, &sqliteErr) {
return slices.Contains(sqliteCodes, sqliteErr.Code())
}

return strings.Contains(err.Error(), sqliteText)
}
186 changes: 186 additions & 0 deletions internal/db/constraint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
package db_test

import (
"database/sql"
"errors"
"testing"

"github.com/calnode/calnode/internal/db"
)

// openMigrated returns an in-memory database with the schema applied, so the
// constraints these tests provoke are the real ones.
func openMigrated(t *testing.T) *sql.DB {
t.Helper()
database, err := db.Open("sqlite://:memory:")
if err != nil {
t.Fatalf("db.Open: %v", err)
}
t.Cleanup(func() { database.Close() })
if err := db.Migrate(database); err != nil {
t.Fatalf("db.Migrate: %v", err)
}
return database
}

// TestConstraintPredicates provokes a real violation of each class against the real
// schema and asserts the predicate recognises it.
//
// Provoked rather than constructed: a hand-built error would only prove the
// predicate agrees with whatever the test author believed the driver returns, and
// the belief this replaces (that the message distinguishes the cases) was wrong.
func TestConstraintPredicates(t *testing.T) {
database := openMigrated(t)

// A user and an event type to hang the booking constraints off.
const userID = "u-constraint"
if _, err := database.Exec(
`INSERT INTO users (id, email, name, iana_timezone) VALUES (?, ?, 'Host', 'UTC')`,
userID, userID+"@example.com"); err != nil {
t.Fatalf("seed user: %v", err)
}
const etID = "et-constraint"
if _, err := database.Exec(
`INSERT INTO event_types (id, user_id, slug, name, duration_minutes)
VALUES (?, ?, ?, 'Call', 30)`, etID, userID, etID); err != nil {
t.Fatalf("seed event type: %v", err)
}

t.Run("unique", func(t *testing.T) {
// users.email is UNIQUE.
_, err := database.Exec(
`INSERT INTO users (id, email, name, iana_timezone) VALUES (?, ?, 'Clash', 'UTC')`,
"u-clash", userID+"@example.com")
assertOnly(t, err, "unique", db.IsUniqueViolation)
})

// ⛔ The case that distinguishes a correct implementation from a plausible one,
// and the reason the text match could not be kept.
//
// SQLite reports a PRIMARY KEY collision as SQLITE_CONSTRAINT_PRIMARYKEY (1555),
// NOT as SQLITE_CONSTRAINT_UNIQUE (2067) — while still saying "UNIQUE constraint
// failed" in the message. So a predicate matching only 2067 passes the subtest
// above and fails here, and the message cannot tell the two apart at all.
//
// This is not a theoretical shape: idempotency_keys.idempotency_key is a bare
// PRIMARY KEY, so claimIdempotencyKey's entire replay path depends on 1555 being
// classified as a unique violation.
t.Run("unique via primary key", func(t *testing.T) {
const key = "idem-key-constraint"
if _, err := database.Exec(
`INSERT INTO idempotency_keys (idempotency_key, request_hash, created_at) VALUES (?, 'h', ?)`,
key, "2026-06-01T00:00:00Z"); err != nil {
t.Fatalf("seed idempotency key: %v", err)
}
_, err := database.Exec(
`INSERT INTO idempotency_keys (idempotency_key, request_hash, created_at) VALUES (?, 'h', ?)`,
key, "2026-06-01T00:00:00Z")
assertOnly(t, err, "primary key", db.IsUniqueViolation)
})

t.Run("check", func(t *testing.T) {
// bookings.status has CHECK (status IN ('confirmed','cancelled')).
_, err := database.Exec(`
INSERT INTO bookings (id, event_type_id, host_id, start_at, end_at, status, created_at, updated_at)
VALUES (?, ?, ?, '2026-06-15T09:00:00Z', '2026-06-15T09:30:00Z', 'not-a-status', ?, ?)`,
"b-check", etID, userID, "2026-06-01T00:00:00Z", "2026-06-01T00:00:00Z")
assertOnly(t, err, "check", db.IsCheckViolation)
})

t.Run("foreign key", func(t *testing.T) {
// event_type_id references event_types(id). This needs foreign_keys=ON,
// which db.Open sets.
_, err := database.Exec(`
INSERT INTO bookings (id, event_type_id, host_id, start_at, end_at, status, created_at, updated_at)
VALUES (?, 'no-such-event-type', ?, '2026-06-15T10:00:00Z', '2026-06-15T10:30:00Z', 'confirmed', ?, ?)`,
"b-fk", userID, "2026-06-01T00:00:00Z", "2026-06-01T00:00:00Z")
assertOnly(t, err, "foreign key", db.IsForeignKeyViolation)
})

t.Run("unrelated error", func(t *testing.T) {
// A predicate that answered true for everything would satisfy every caller
// above and be badly wrong, so the negative cases carry as much weight.
_, err := database.Exec(`SELECT * FROM a_table_that_does_not_exist`)
if err == nil {
t.Fatal("expected an error from a missing table")
}
assertNone(t, err, "missing table")
assertNone(t, errors.New("some unrelated failure"), "plain error")
assertNone(t, nil, "nil")
})

t.Run("unhandled constraint class", func(t *testing.T) {
// A NOT NULL violation is a constraint violation of a class nothing here
// classifies (SQLITE_CONSTRAINT_NOTNULL, 1299). It must match none of the
// three, so a caller cannot turn it into a 409 by accident.
_, err := database.Exec(
`INSERT INTO users (id, email, name, iana_timezone) VALUES (?, ?, NULL, 'UTC')`,
"u-null", "u-null@example.com")
if err == nil {
t.Fatal("expected a NOT NULL violation")
}
assertNone(t, err, "not-null violation")
})
}

// TestConstraintTextFallback covers the branch the live driver never reaches: an
// error that arrives without its driver type still attached.
//
// The branch exists for a driver release that changes its error type, or a layer
// that reformats an error into a plain one rather than wrapping it — in which case
// the message is the only signal left, and answering from it beats returning a 500.
// Without this test the fallback would be unexecuted code that reads like an
// accident.
func TestConstraintTextFallback(t *testing.T) {
cases := []struct {
text string
want func(error) bool
name string
}{
{"boom: UNIQUE constraint failed: t.a", db.IsUniqueViolation, "unique"},
{"boom: CHECK constraint failed: n > 0", db.IsCheckViolation, "check"},
{"boom: FOREIGN KEY constraint failed", db.IsForeignKeyViolation, "foreign key"},
}
for _, c := range cases {
err := errors.New(c.text)
if !c.want(err) {
t.Errorf("%s: fallback did not recognise %q", c.name, c.text)
}
}
// And the fallback is still discriminating, not a catch-all.
assertNone(t, errors.New("NOT NULL constraint failed: t.a"), "not-null text")
}

// assertOnly checks that want recognises err and the other two predicates do not:
// the classes have to be distinguishable, or a CHECK violation becomes a 409.
func assertOnly(t *testing.T, err error, class string, want func(error) bool) {
t.Helper()
if err == nil {
t.Fatalf("%s: expected a constraint violation, got nil", class)
}
if !want(err) {
t.Errorf("%s: predicate did not recognise %v", class, err)
}
matches := 0
for _, p := range []func(error) bool{db.IsUniqueViolation, db.IsCheckViolation, db.IsForeignKeyViolation} {
if p(err) {
matches++
}
}
if matches != 1 {
t.Errorf("%s: %d of 3 predicates matched %v; want exactly 1", class, matches, err)
}
}

func assertNone(t *testing.T, err error, what string) {
t.Helper()
for name, p := range map[string]func(error) bool{
"IsUniqueViolation": db.IsUniqueViolation,
"IsCheckViolation": db.IsCheckViolation,
"IsForeignKeyViolation": db.IsForeignKeyViolation,
} {
if p(err) {
t.Errorf("%s returned true for %s (%v)", name, what, err)
}
}
}
6 changes: 3 additions & 3 deletions internal/handler/availability.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"database/sql"
"encoding/json"
"net/http"
"strings"

"github.com/calnode/calnode/internal/db"
"github.com/calnode/calnode/internal/uid"
)

Expand Down Expand Up @@ -47,7 +47,7 @@ func (h *Handler) CreateAvailabilityRule(w http.ResponseWriter, r *http.Request)
VALUES (?, ?, ?, ?, ?, ?)`,
id, user.ID, req.EventTypeID, req.DayOfWeek, req.StartTime, req.EndTime)
if err != nil {
if strings.Contains(err.Error(), "UNIQUE constraint failed") {
if db.IsUniqueViolation(err) {
h.writeError(w, http.StatusConflict, "a rule for this day and time already exists")
return
}
Expand Down Expand Up @@ -177,7 +177,7 @@ func (h *Handler) UpdateAvailabilityRule(w http.ResponseWriter, r *http.Request)
`UPDATE availability_rules SET day_of_week=?, start_time=?, end_time=? WHERE id=? AND user_id=?`,
current.DayOfWeek, current.StartTime, current.EndTime, id, user.ID)
if err != nil {
if strings.Contains(err.Error(), "UNIQUE constraint failed") {
if db.IsUniqueViolation(err) {
h.writeError(w, http.StatusConflict, "a rule for this day and time already exists")
return
}
Expand Down
8 changes: 2 additions & 6 deletions internal/handler/booking_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/calnode/calnode/internal/booking"
"github.com/calnode/calnode/internal/calendar"
"github.com/calnode/calnode/internal/db"
"github.com/calnode/calnode/internal/i18n"
"github.com/calnode/calnode/internal/mailer"
"github.com/calnode/calnode/internal/slots"
Expand Down Expand Up @@ -807,7 +808,7 @@ func (h *Handler) CreateBooking(w http.ResponseWriter, r *http.Request) {
}
// A question was deleted between validateAnswers and the INSERT — return a
// clean 422 rather than leaking a generic 500 for an FK constraint failure.
if isForeignKeyViolation(err) {
if db.IsForeignKeyViolation(err) {
h.writeError(w, http.StatusUnprocessableEntity, "one or more questions are no longer available")
return
}
Expand Down Expand Up @@ -1843,11 +1844,6 @@ func (h *Handler) loadHostPrefs(ctx context.Context, hostID string) (hostPrefs,
return p, nil
}

// isForeignKeyViolation reports whether err is a SQLite FOREIGN KEY constraint failure.
func isForeignKeyViolation(err error) bool {
return strings.Contains(err.Error(), "FOREIGN KEY constraint failed")
}

// enqueueReminder inserts a reminder.send job scheduled hoursBefore hours before startAt.
// If the computed run_at has already passed, the job fires on the next poll cycle.
func (h *Handler) enqueueReminder(ctx context.Context, bookingID string, startAt time.Time, hoursBefore int) error {
Expand Down
Loading
Loading