From c8246fc070b6d9145ab79b8965163a3769f06c75 Mon Sep 17 00:00:00 2001 From: Distronode <254259913+distronode-com@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:20:28 -0400 Subject: [PATCH] db: classify constraint violations by SQLite's codes, not its prose Thirteen call sites decided what a database error meant by matching English: strings.Contains(err.Error(), "UNIQUE constraint failed"). That is invisible to every gate, and it cannot answer the question it is being asked. SQLite reports a PRIMARY KEY collision as SQLITE_CONSTRAINT_PRIMARYKEY (1555) and an ordinary unique violation as SQLITE_CONSTRAINT_UNIQUE (2067), and it uses the SAME message for both. So the text match cannot distinguish them, and a code match written from the message alone would match 2067 and silently stop recognising every primary-key collision. idempotency_keys.idempotency_key is a bare PRIMARY KEY, so the replay path depends on 1555 being read as a unique violation. IsUniqueViolation, IsCheckViolation and IsForeignKeyViolation answer from the driver's code and fall back to the message only when the concrete driver type is no longer attached. A driver error whose code does not match returns false rather than falling through: falling through would classify by whether the message happened to contain an English phrase, and would readmit 1555 by its text after excluding it by code. Each class is provoked against the real schema rather than constructed, so the test asserts what the driver does rather than what its author believed. Removing 1555 from IsUniqueViolation fails the primary-key case, which is what makes that case worth having. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 19 +++ internal/booking/service.go | 12 +- internal/db/constraint.go | 85 +++++++++++++ internal/db/constraint_test.go | 186 ++++++++++++++++++++++++++++ internal/handler/availability.go | 6 +- internal/handler/booking_handler.go | 8 +- internal/handler/event_type.go | 9 +- internal/handler/idempotency.go | 5 +- internal/handler/override.go | 4 +- internal/handler/teams.go | 7 +- 10 files changed, 313 insertions(+), 28 deletions(-) create mode 100644 internal/db/constraint.go create mode 100644 internal/db/constraint_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b4c8bb2..7e602e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/internal/booking/service.go b/internal/booking/service.go index da604f9..665dc1f 100644 --- a/internal/booking/service.go +++ b/internal/booking/service.go @@ -13,6 +13,7 @@ import ( "strings" "time" + "github.com/calnode/calnode/internal/db" "github.com/calnode/calnode/internal/uid" ) @@ -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) @@ -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) @@ -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) @@ -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") -} diff --git a/internal/db/constraint.go b/internal/db/constraint.go new file mode 100644 index 0000000..33b2909 --- /dev/null +++ b/internal/db/constraint.go @@ -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) +} diff --git a/internal/db/constraint_test.go b/internal/db/constraint_test.go new file mode 100644 index 0000000..174063d --- /dev/null +++ b/internal/db/constraint_test.go @@ -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) + } + } +} diff --git a/internal/handler/availability.go b/internal/handler/availability.go index 6577260..3eb766e 100644 --- a/internal/handler/availability.go +++ b/internal/handler/availability.go @@ -4,8 +4,8 @@ import ( "database/sql" "encoding/json" "net/http" - "strings" + "github.com/calnode/calnode/internal/db" "github.com/calnode/calnode/internal/uid" ) @@ -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 } @@ -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 } diff --git a/internal/handler/booking_handler.go b/internal/handler/booking_handler.go index 7f0c5d9..53f7a8e 100644 --- a/internal/handler/booking_handler.go +++ b/internal/handler/booking_handler.go @@ -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" @@ -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 } @@ -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 { diff --git a/internal/handler/event_type.go b/internal/handler/event_type.go index 40a9e98..d2da9c8 100644 --- a/internal/handler/event_type.go +++ b/internal/handler/event_type.go @@ -7,6 +7,7 @@ import ( "net/http" "strings" + "github.com/calnode/calnode/internal/db" "github.com/calnode/calnode/internal/uid" ) @@ -315,11 +316,11 @@ func (h *Handler) CreateEventType(w http.ResponseWriter, r *http.Request) { routingMode, bufBefore, bufAfter, minNotice, maxFuture, maxActive, showTaken, defaultMsgConfirmation, defaultMsgCancellation, defaultMsgReschedule, defaultMsgReminder) if err != nil { - if strings.Contains(err.Error(), "UNIQUE constraint failed") { + if db.IsUniqueViolation(err) { h.writeError(w, http.StatusConflict, "slug already in use") return } - if strings.Contains(err.Error(), "CHECK constraint failed") { + if db.IsCheckViolation(err) { h.writeError(w, http.StatusBadRequest, "invalid location_type or routing_mode value") return } @@ -701,7 +702,7 @@ func (h *Handler) PatchEventType(w http.ResponseWriter, r *http.Request) { "UPDATE event_types SET "+strings.Join(setClauses, ", ")+" WHERE slug = ? AND user_id = ?", // #nosec G202 -- setClauses is built by set()/the literal col list above; every column name is a hardcoded string, every value is bound via args... args...) if err != nil { - if strings.Contains(err.Error(), "CHECK constraint failed") { + if db.IsCheckViolation(err) { h.writeError(w, http.StatusBadRequest, "invalid location_type or routing_mode value") return } @@ -780,7 +781,7 @@ func (h *Handler) DeleteEventType(w http.ResponseWriter, r *http.Request) { res, err := h.db.ExecContext(r.Context(), `DELETE FROM event_types WHERE slug = ? AND user_id = ?`, slug, user.ID) if err != nil { - if strings.Contains(err.Error(), "FOREIGN KEY constraint failed") { + if db.IsForeignKeyViolation(err) { h.writeError(w, http.StatusConflict, "this event type has bookings in its history (including cancelled ones) and can't be deleted — deactivate it instead") return } diff --git a/internal/handler/idempotency.go b/internal/handler/idempotency.go index 91717f9..8991666 100644 --- a/internal/handler/idempotency.go +++ b/internal/handler/idempotency.go @@ -5,8 +5,9 @@ import ( "crypto/sha256" "database/sql" "encoding/hex" - "strings" "time" + + "github.com/calnode/calnode/internal/db" ) // idempotencyRecord is a previously-seen Idempotency-Key's stored outcome. @@ -38,7 +39,7 @@ func (h *Handler) claimIdempotencyKey(ctx context.Context, key, reqHash string) if err == nil { return nil, false, nil } - if !strings.Contains(err.Error(), "UNIQUE constraint failed") { + if !db.IsUniqueViolation(err) { return nil, false, err } diff --git a/internal/handler/override.go b/internal/handler/override.go index aaa2274..d34db3a 100644 --- a/internal/handler/override.go +++ b/internal/handler/override.go @@ -4,9 +4,9 @@ import ( "database/sql" "encoding/json" "net/http" - "strings" "time" + "github.com/calnode/calnode/internal/db" "github.com/calnode/calnode/internal/uid" ) @@ -155,7 +155,7 @@ func (h *Handler) CreateAvailabilityOverride(w http.ResponseWriter, r *http.Requ INSERT INTO availability_overrides (id, user_id, date, is_available, reason, start_time, end_time) VALUES (?, ?, ?, ?, ?, ?, ?)`, id, user.ID, req.Date, isAvailInt, req.Reason, req.StartTime, req.EndTime); err != nil { - if strings.Contains(err.Error(), "UNIQUE constraint failed") { + if db.IsUniqueViolation(err) { h.writeError(w, http.StatusConflict, "an override already exists for this date; delete it first") return } diff --git a/internal/handler/teams.go b/internal/handler/teams.go index ff32173..c096625 100644 --- a/internal/handler/teams.go +++ b/internal/handler/teams.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "github.com/calnode/calnode/internal/db" "github.com/calnode/calnode/internal/uid" ) @@ -86,7 +87,7 @@ func (h *Handler) CreateTeam(w http.ResponseWriter, r *http.Request) { if _, err := h.db.ExecContext(r.Context(), `INSERT INTO teams (id, name, slug, created_at) VALUES (?, ?, ?, ?)`, id, req.Name, slug, now); err != nil { - if strings.Contains(err.Error(), "UNIQUE constraint failed") { + if db.IsUniqueViolation(err) { h.writeError(w, http.StatusConflict, "a team with that slug already exists") return } @@ -222,7 +223,7 @@ func (h *Handler) PatchTeam(w http.ResponseWriter, r *http.Request) { res, err := h.db.ExecContext(r.Context(), "UPDATE teams SET "+strings.Join(sets, ", ")+" WHERE id = ?", args...) // #nosec G202 -- sets is built above from hardcoded "col = ?" literals only; every value is bound via args... if err != nil { - if strings.Contains(err.Error(), "UNIQUE constraint failed") { + if db.IsUniqueViolation(err) { h.writeError(w, http.StatusConflict, "a team with that slug already exists") return } @@ -306,7 +307,7 @@ func (h *Handler) AddTeamMember(w http.ResponseWriter, r *http.Request) { INSERT INTO team_members (id, team_id, user_id, role, routing_priority) VALUES (?, ?, ?, 'member', ?)`, uid.New(), teamID, req.UserID, req.RoutingPriority); err != nil { - if strings.Contains(err.Error(), "UNIQUE constraint failed") { + if db.IsUniqueViolation(err) { h.writeError(w, http.StatusConflict, "that user is already in this team") return }