mirror
/
hydra
mirror of https://github.com/ory/hydra
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

chore: improve error reporting to help diagnose flaky test 

GitOrigin-RevId: d08aa2c570dd17df5a5e14c2975cebe80aeb2e66
This commit is contained in:
Philippe Gaultier 2025-11-20 12:56:35 +01:00 committed by ory-bot
parent c28a6c82fc
commit 9023ef4d11
7 changed files with 56 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
oryx/popx/match.go
View File

@ -4,9 +4,10 @@
package popx
import (
"fmt"
"regexp"
"github.com/pkg/errors"
"github.com/ory/pop/v6"
)
@ -43,18 +44,18 @@ func parseMigrationFilename(filename string) (*match, error) {
} else {
dbType = pop.CanonicalDialect(m[3][1:])
if !pop.DialectSupported(dbType) {
return nil, fmt.Errorf("unsupported dialect %s", dbType)
return nil, errors.Errorf("unsupported dialect %s", dbType)
}
}
if m[6] == "fizz" && dbType != "all" {
return nil, fmt.Errorf("invalid database type %q, expected \"all\" because fizz is database type independent", dbType)
return nil, errors.Errorf("invalid database type %q, expected \"all\" because fizz is database type independent", dbType)
}
if m[4] == ".autocommit" {
autocommit = true
} else if m[4] != "" {
return nil, fmt.Errorf("invalid autocommit flag %q", m[4])
return nil, errors.Errorf("invalid autocommit flag %q", m[4])
}
return &match{

14
oryx/popx/migration_box.go
View File

@ -89,7 +89,7 @@ func WithTestdata(t *testing.T, testdata fs.FS) MigrationBoxOption {
return func(m *MigrationBox) {
require.NoError(t, fs.WalkDir(testdata, ".", func(path string, info fs.DirEntry, err error) error {
if err != nil {
return err
return errors.WithStack(err)
}
if !info.Type().IsRegular() {
t.Logf("skipping testdata entry that is not a file: %s", path)
@ -120,13 +120,13 @@ func WithTestdata(t *testing.T, testdata fs.FS) MigrationBoxOption {
Runner: func(m Migration, c *pop.Connection) error {
b, err := fs.ReadFile(testdata, m.Path)
if err != nil {
return err
return errors.WithStack(err)
}
if isMigrationEmpty(string(b)) {
return nil
}
_, err = c.Store.SQLDB().Exec(string(b))
return err
return errors.WithStack(err)
},
})
@ -230,7 +230,7 @@ func (mb *MigrationBox) findMigrations(
}
if details == nil {
return errors.WithStack(fmt.Errorf("Found a migration file that does not match the file pattern: filename=%s pattern=%s", info.Name(), MigrationFileRegexp))
return errors.Errorf("Found a migration file that does not match the file pattern: filename=%s pattern=%s", info.Name(), MigrationFileRegexp)
}
content, err := fs.ReadFile(dir, p)
@ -269,7 +269,7 @@ func (mb *MigrationBox) findMigrations(
// Sort ascending.
sort.Sort(mb.migrationsUp)
return err
return errors.WithStack(err)
}
// hasDownMigrationWithVersion checks if there is a migration with the given
@ -293,12 +293,12 @@ func (mb *MigrationBox) check() error {
for _, n := range mb.migrationsUp {
if err := n.Valid(); err != nil {
return err
return errors.WithStack(err)
}
}
for _, n := range mb.migrationsDown {
if err := n.Valid(); err != nil {
return err
return errors.WithStack(err)
}
}
return nil

48
oryx/popx/migrator.go
View File

@ -22,6 +22,7 @@ import (
"github.com/ory/x/cmdx"
"github.com/ory/x/logrusx"
"github.com/ory/x/otelx"
"github.com/ory/x/sqlcon"
)
const (
@ -37,7 +38,7 @@ func (mb *MigrationBox) shouldNotUseTransaction(m Migration) bool {
// Up runs pending "up" migrations and applies them to the database.
func (mb *MigrationBox) Up(ctx context.Context) error {
_, err := mb.UpTo(ctx, 0)
return err
return errors.WithStack(err)
}
// UpTo runs up to step "up" migrations and applies them to the database.
@ -83,7 +84,7 @@ func (mb *MigrationBox) UpTo(ctx context.Context, step int) (applied int, err er
err := conn.RawQuery(fmt.Sprintf("INSERT INTO %s (version) VALUES (?)", mtn), mi.Version).Exec()
return errors.Wrapf(err, "problem inserting migration version %s", mi.Version)
}); err != nil {
return err
return errors.WithStack(err)
}
continue
}
@ -91,13 +92,14 @@ func (mb *MigrationBox) UpTo(ctx context.Context, step int) (applied int, err er
l.Info("Migration has not yet been applied, running migration.")
if err := mi.Valid(); err != nil {
return err
return errors.WithStack(err)
}
noTx := mb.shouldNotUseTransaction(mi)
if noTx {
l.Info("NOT running migrations inside a transaction")
if err := mi.Runner(mi, c); err != nil {
return err
return errors.WithStack(err)
}
// #nosec G201 - mtn is a system-wide const
@ -107,7 +109,7 @@ func (mb *MigrationBox) UpTo(ctx context.Context, step int) (applied int, err er
} else {
if err := mb.isolatedTransaction(ctx, "up", func(conn *pop.Connection) error {
if err := mi.Runner(mi, conn); err != nil {
return err
return errors.WithStack(err)
}
// #nosec G201 - mtn is a system-wide const
@ -116,7 +118,7 @@ func (mb *MigrationBox) UpTo(ctx context.Context, step int) (applied int, err er
}
return nil
}); err != nil {
return err
return errors.WithStack(err)
}
}
@ -133,7 +135,7 @@ func (mb *MigrationBox) UpTo(ctx context.Context, step int) (applied int, err er
}
return nil
})
return
return applied, errors.WithStack(err)
}
// Down runs pending "down" migrations and rolls back the
@ -148,7 +150,7 @@ func (mb *MigrationBox) Down(ctx context.Context, steps int) (err error) {
}
c := mb.c.WithContext(ctx)
return mb.exec(ctx, func() (err error) {
return errors.WithStack(mb.exec(ctx, func() (err error) {
mtn := sanitizedMigrationTableName(c)
count, err := c.Count(mtn)
if err != nil {
@ -203,7 +205,7 @@ func (mb *MigrationBox) Down(ctx context.Context, steps int) (err error) {
if mb.shouldNotUseTransaction(mi) {
err := mi.Runner(mi, c)
if err != nil {
return err
return errors.WithStack(err)
}
// #nosec G201 - mtn is a system-wide const
@ -232,7 +234,7 @@ func (mb *MigrationBox) Down(ctx context.Context, steps int) (err error) {
reverted++
}
return nil
})
}))
}
func (mb *MigrationBox) createTransactionalMigrationTable(ctx context.Context, c *pop.Connection, l *logrusx.Logger) error {
@ -243,7 +245,7 @@ func (mb *MigrationBox) createTransactionalMigrationTable(ctx context.Context, c
fmt.Sprintf(`CREATE UNIQUE INDEX %s_version_idx ON %s (version)`, mtn, mtn),
fmt.Sprintf(`CREATE INDEX %s_version_self_idx ON %s (version_self)`, mtn, mtn),
}); err != nil {
return err
return errors.WithStack(err)
}
l.WithField("migration_table", mtn).Debug("Transactional migration table created successfully.")
@ -277,7 +279,7 @@ func (mb *MigrationBox) migrateToTransactionalMigrationTable(ctx context.Context
}
if err := mb.createMigrationStatusTableTransaction(ctx, workload...); err != nil {
return err
return errors.WithStack(err)
}
l.WithField("migration_table", mtn).Debug("Successfully migrated legacy schema_migration to new transactional schema_migration table.")
@ -296,7 +298,7 @@ func (mb *MigrationBox) isolatedTransaction(ctx context.Context, direction strin
}
return Transaction(ctx, mb.c.WithContext(ctx), func(ctx context.Context, connection *pop.Connection) error {
return fn(connection)
return errors.WithStack(fn(connection))
})
}
@ -319,7 +321,7 @@ func (mb *MigrationBox) createMigrationStatusTableTransaction(ctx context.Contex
}
return nil
}); err != nil {
return err
return errors.WithStack(err)
}
}
}
@ -341,14 +343,14 @@ func (mb *MigrationBox) CreateSchemaMigrations(ctx context.Context) error {
if err != nil {
mb.l.WithError(err).WithField("migration_table", mtn).Debug("An error occurred while checking for the legacy migration table, maybe it does not exist yet? Trying to create.")
// This means that the legacy pop migrator has not yet been applied
return mb.createTransactionalMigrationTable(ctx, c, mb.l)
return errors.WithStack(mb.createTransactionalMigrationTable(ctx, c, mb.l))
}
mb.l.WithField("migration_table", mtn).Debug("A migration table exists, checking if it is a transactional migration table.")
_, err = c.Store.Exec(fmt.Sprintf("select version, version_self from %s", mtn))
if err != nil {
mb.l.WithError(err).WithField("migration_table", mtn).Debug("An error occurred while checking for the transactional migration table, maybe it does not exist yet? Trying to create.")
return mb.migrateToTransactionalMigrationTable(ctx, c, mb.l)
return errors.WithStack(mb.migrateToTransactionalMigrationTable(ctx, c, mb.l))
}
mb.l.WithField("migration_table", mtn).Debug("Migration tables exist and are up to date.")
@ -436,7 +438,7 @@ func (mb *MigrationBox) Status(ctx context.Context) (MigrationStatuses, error) {
// It also means that we can ignore this state and act as if no migrations have been applied yet.
} else {
// On any other error, we fail.
return nil, errors.Wrapf(err, "problem with migration")
return nil, errors.Wrap(err, "problem with migration")
}
}
@ -471,12 +473,12 @@ func (mb *MigrationBox) DumpMigrationSchema(ctx context.Context) error {
schema := "schema.sql"
f, err := os.Create(schema) //#nosec:G304) //#nosec:G304
if err != nil {
return err
return errors.WithStack(err)
}
err = c.Dialect.DumpSchema(f)
if err != nil {
_ = os.RemoveAll(schema)
return err
return errors.WithStack(err)
}
return nil
}
@ -501,24 +503,24 @@ func (mb *MigrationBox) exec(ctx context.Context, fn func() error) error {
if mb.c.Dialect.Name() == "sqlite3" {
if err := mb.c.RawQuery("PRAGMA foreign_keys=OFF").Exec(); err != nil {
return err
return sqlcon.HandleError(err)
}
}
if mb.c.Dialect.Name() == "cockroach" {
outer := fn
fn = func() error {
return crdb.Execute(outer)
return errors.WithStack(crdb.Execute(outer))
}
}
if err := fn(); err != nil {
return err
return errors.WithStack(err)
}
if mb.c.Dialect.Name() == "sqlite3" {
if err := mb.c.RawQuery("PRAGMA foreign_keys=ON").Exec(); err != nil {
return err
return sqlcon.HandleError(err)
}
}

5
oryx/popx/sql_template_funcs.go
View File

@ -4,8 +4,9 @@
package popx
import (
"fmt"
"regexp"
"github.com/pkg/errors"
)
var SQLTemplateFuncs = map[string]interface{}{
@ -16,7 +17,7 @@ var identifierPattern = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_]*$")
func Identifier(i string) (string, error) {
if !identifierPattern.MatchString(i) {
return "", fmt.Errorf("invalid SQL identifier '%s'", i)
return "", errors.Errorf("invalid SQL identifier '%s'", i)
}
return i, nil
}

23
oryx/popx/transaction.go
View File

@ -9,6 +9,7 @@ import (
"github.com/cockroachdb/cockroach-go/v2/crdb"
"github.com/jmoiron/sqlx"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/ory/pop/v6"
@ -30,7 +31,7 @@ func Transaction(ctx context.Context, connection *pop.Connection, callback func(
c := ctx.Value(transactionKey)
if c != nil {
if conn, ok := c.(*pop.Connection); ok {
return callback(ctx, conn.WithContext(ctx))
return errors.WithStack(callback(ctx, conn.WithContext(ctx)))
}
}
@ -38,24 +39,24 @@ func Transaction(ctx context.Context, connection *pop.Connection, callback func(
return connection.WithContext(ctx).Dialect.Lock(func() error {
transaction, err := connection.NewTransaction()
if err != nil {
return err
return errors.WithStack(err)
}
attempt := 0
return crdb.ExecuteInTx(ctx, sqlxTxAdapter{transaction.TX.Tx}, func() error {
return errors.WithStack(crdb.ExecuteInTx(ctx, sqlxTxAdapter{transaction.TX.Tx}, func() error {
attempt++
if attempt > 1 {
caller := caller()
transactionRetries.WithLabelValues(caller).Inc()
}
return callback(WithTransaction(ctx, transaction), transaction)
})
return errors.WithStack(callback(WithTransaction(ctx, transaction), transaction))
}))
})
}
return connection.WithContext(ctx).Transaction(func(tx *pop.Connection) error {
return callback(WithTransaction(ctx, tx), tx)
})
return errors.WithStack(connection.WithContext(ctx).Transaction(func(tx *pop.Connection) error {
return errors.WithStack(callback(WithTransaction(ctx, tx), tx))
}))
}
func GetConnection(ctx context.Context, connection *pop.Connection) *pop.Connection {
@ -76,15 +77,15 @@ var _ crdb.Tx = sqlxTxAdapter{}
func (s sqlxTxAdapter) Exec(ctx context.Context, query string, args ...interface{}) error {
_, err := s.Tx.ExecContext(ctx, query, args...)
return err
return errors.WithStack(err)
}
func (s sqlxTxAdapter) Commit(ctx context.Context) error {
return s.Tx.Commit()
return errors.WithStack(s.Tx.Commit())
}
func (s sqlxTxAdapter) Rollback(ctx context.Context) error {
return s.Tx.Rollback()
return errors.WithStack(s.Tx.Rollback())
}
var (

8
oryx/sqlcon/error.go
View File

@ -74,11 +74,11 @@ func HandleError(err error) error {
if errors.Is(err, sql.ErrNoRows) {
return errors.WithStack(ErrNoRows)
} else if errors.As(err, &st) {
return handlePostgres(err, st.SQLState())
return errors.WithStack(handlePostgres(err, st.SQLState()))
} else if e := new(pq.Error); errors.As(err, &e) {
return handlePostgres(err, string(e.Code))
return errors.WithStack(handlePostgres(err, string(e.Code)))
} else if e := new(pgconn.PgError); errors.As(err, &e) {
return handlePostgres(err, e.Code)
return errors.WithStack(handlePostgres(err, e.Code))
} else if e := new(mysql.MySQLError); errors.As(err, &e) {
switch e.Number {
case 1062:
@ -89,7 +89,7 @@ func HandleError(err error) error {
}
if err := handleSqlite(err); err != nil {
return err
return errors.WithStack(err)
}
return errors.WithStack(err)

1
persistence/sql/migratest/migration_test.go
View File

@ -108,7 +108,6 @@ func TestMigrations(t *testing.T) {
tm, err := popx.NewMigrationBox(
sql.Migrations,
c, l,
popx.WithPerMigrationTimeout(time.Minute),
popx.WithTestdata(t, os.DirFS("./testdata")))
require.NoError(t, err)
require.NoError(t, tm.Up(ctx))