Check unique constraint errors when manually inserting migrations (#2712)

This should avoid unnecessary logging on startup if the migration (were
we need `InsertMigration`) was already executed.
This now checks for "unique constraint errors" for SQLite and Postgres
and fails the startup process if the migration couldn't be manually
inserted for some other reason.

internal/sqlutil/migrate.go

@@ -155,5 +155,10 @@ func InsertMigration(ctx context.Context, db *sql.DB, migrationName string) erro
 		time.Now().Format(time.RFC3339),
 		internal.VersionString(),
 	)
+	// If the migration was already executed, we'll get a unique constraint error,
+	// return nil instead, to avoid unnecessary logging.
+	if IsUniqueConstraintViolationErr(err) {
+		return nil
+	}
 	return err
 }

internal/sqlutil/unique_constraint.go

@@ -12,12 +12,27 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//go:build wasm
+//go:build !wasm
-// +build wasm
+// +build !wasm
 
 package sqlutil
 
-// IsUniqueConstraintViolationErr no-ops for this architecture
+import (
+	"github.com/lib/pq"
+	"github.com/mattn/go-sqlite3"
+)
+
+// IsUniqueConstraintViolationErr returns true if the error is an unique_violation error
 func IsUniqueConstraintViolationErr(err error) bool {
+	switch e := err.(type) {
+	case *pq.Error:
+		return e.Code == "23505"
+	case pq.Error:
+		return e.Code == "23505"
+	case *sqlite3.Error:
+		return e.Code == sqlite3.ErrConstraint
+	case sqlite3.Error:
+		return e.Code == sqlite3.ErrConstraint
+	}
 	return false
 }

internal/sqlutil/unique_constraint_wasm.go

@@ -12,15 +12,20 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//go:build !wasm
+//go:build wasm
-// +build !wasm
+// +build wasm
 
 package sqlutil
 
-import "github.com/lib/pq"
+import "github.com/mattn/go-sqlite3"
 
-// IsUniqueConstraintViolationErr returns true if the error is a postgresql unique_violation error
+// IsUniqueConstraintViolationErr returns true if the error is an unique_violation error
 func IsUniqueConstraintViolationErr(err error) bool {
-	pqErr, ok := err.(*pq.Error)
+	switch e := err.(type) {
-	return ok && pqErr.Code == "23505"
+	case *sqlite3.Error:
+		return e.Code == sqlite3.ErrConstraint
+	case sqlite3.Error:
+		return e.Code == sqlite3.ErrConstraint
+	}
+	return false
 }

keyserver/storage/postgres/key_changes_table.go

@@ -18,8 +18,7 @@ import (
 	"context"
 	"database/sql"
 	"errors"
-
+	"fmt"
-	"github.com/sirupsen/logrus"
 
 	"github.com/matrix-org/dendrite/internal"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
@@ -81,8 +80,7 @@ func executeMigration(ctx context.Context, db *sql.DB) error {
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) { // migration was already executed, as the column was removed
 			if err = sqlutil.InsertMigration(ctx, db, migrationName); err != nil {
-				// not a fatal error, log and continue
+				return fmt.Errorf("unable to manually insert migration '%s': %w", migrationName, err)
-				logrus.WithError(err).Warnf("unable to manually insert migration '%s'", migrationName)
 			}
 			return nil
 		}

keyserver/storage/sqlite3/key_changes_table.go

@@ -18,8 +18,7 @@ import (
 	"context"
 	"database/sql"
 	"errors"
-
+	"fmt"
-	"github.com/sirupsen/logrus"
 
 	"github.com/matrix-org/dendrite/internal"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
@@ -80,8 +79,7 @@ func executeMigration(ctx context.Context, db *sql.DB) error {
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) { // migration was already executed, as the column was removed
 			if err = sqlutil.InsertMigration(ctx, db, migrationName); err != nil {
-				// not a fatal error, log and continue
+				return fmt.Errorf("unable to manually insert migration '%s': %w", migrationName, err)
-				logrus.WithError(err).Warnf("unable to manually insert migration '%s'", migrationName)
 			}
 			return nil
 		}

roomserver/storage/postgres/storage.go

@@ -21,8 +21,6 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/sirupsen/logrus"
-
 	// Import the postgres database driver.
 	_ "github.com/lib/pq"
 
@@ -79,8 +77,7 @@ func executeMigration(ctx context.Context, db *sql.DB) error {
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) { // migration was already executed, as the column was removed
 			if err = sqlutil.InsertMigration(ctx, db, migrationName); err != nil {
-				// not a fatal error, log and continue
+				return fmt.Errorf("unable to manually insert migration '%s': %w", migrationName, err)
-				logrus.WithError(err).Warnf("unable to manually insert migration '%s'", migrationName)
 			}
 			return nil
 		}

roomserver/storage/sqlite3/storage.go

@@ -22,7 +22,6 @@ import (
 	"fmt"
 
 	"github.com/matrix-org/gomatrixserverlib"
-	"github.com/sirupsen/logrus"
 
 	"github.com/matrix-org/dendrite/internal/caching"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
@@ -87,8 +86,7 @@ func executeMigration(ctx context.Context, db *sql.DB) error {
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) { // migration was already executed, as the column was removed
 			if err = sqlutil.InsertMigration(ctx, db, migrationName); err != nil {
-				// not a fatal error, log and continue
+				return fmt.Errorf("unable to manually insert migration '%s': %w", migrationName, err)
-				logrus.WithError(err).Warnf("unable to manually insert migration '%s'", migrationName)
 			}
 			return nil
 		}