Skip to content

Commit 126c8ff

Browse files
committed
fix: improve error messages for missing database tables
When datastore operations fail due to missing database tables (typically because migrations haven't been run), users now see a clear error message: 'please ensure you have run spicedb datastore migrate' This improves the user experience by providing actionable guidance instead of raw database errors like 'relation X does not exist'. The improvement covers all reader operations across CRDB, PostgreSQL, and MySQL datastores including namespace, caveat, counter, and statistics queries.
1 parent 833dea2 commit 126c8ff

File tree

17 files changed

+475
-0
lines changed

17 files changed

+475
-0
lines changed

internal/datastore/common/errors.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,32 @@ type RevisionUnavailableError struct {
176176
func NewRevisionUnavailableError(err error) error {
177177
return RevisionUnavailableError{err}
178178
}
179+
180+
// SchemaNotInitializedError is returned when a datastore operation fails because the
181+
// required database tables do not exist. This typically means that migrations have not been run.
182+
type SchemaNotInitializedError struct {
183+
error
184+
}
185+
186+
func (err SchemaNotInitializedError) GRPCStatus() *status.Status {
187+
return spiceerrors.WithCodeAndDetails(
188+
err,
189+
codes.FailedPrecondition,
190+
spiceerrors.ForReason(
191+
v1.ErrorReason_ERROR_REASON_UNSPECIFIED,
192+
map[string]string{},
193+
),
194+
)
195+
}
196+
197+
func (err SchemaNotInitializedError) Unwrap() error {
198+
return err.error
199+
}
200+
201+
// NewSchemaNotInitializedError creates a new SchemaNotInitializedError with a helpful message
202+
// instructing the user to run migrations.
203+
func NewSchemaNotInitializedError(underlying error) error {
204+
return SchemaNotInitializedError{
205+
fmt.Errorf("datastore error: the database schema has not been initialized; please run \"spicedb datastore migrate\": %w", underlying),
206+
}
207+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package common
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
"google.golang.org/grpc/codes"
9+
)
10+
11+
func TestSchemaNotInitializedError(t *testing.T) {
12+
underlyingErr := fmt.Errorf("relation \"caveat\" does not exist (SQLSTATE 42P01)")
13+
err := NewSchemaNotInitializedError(underlyingErr)
14+
15+
t.Run("error message contains migration instructions", func(t *testing.T) {
16+
require.Contains(t, err.Error(), "spicedb datastore migrate")
17+
require.Contains(t, err.Error(), "database schema has not been initialized")
18+
})
19+
20+
t.Run("unwrap returns underlying error", func(t *testing.T) {
21+
var schemaErr SchemaNotInitializedError
22+
require.ErrorAs(t, err, &schemaErr)
23+
require.ErrorIs(t, schemaErr.Unwrap(), underlyingErr)
24+
})
25+
26+
t.Run("grpc status is FailedPrecondition", func(t *testing.T) {
27+
var schemaErr SchemaNotInitializedError
28+
require.ErrorAs(t, err, &schemaErr)
29+
status := schemaErr.GRPCStatus()
30+
require.Equal(t, codes.FailedPrecondition, status.Code())
31+
})
32+
33+
t.Run("can be detected with errors.As", func(t *testing.T) {
34+
var schemaErr SchemaNotInitializedError
35+
require.ErrorAs(t, err, &schemaErr)
36+
})
37+
38+
t.Run("wrapped error preserves chain", func(t *testing.T) {
39+
wrappedErr := fmt.Errorf("outer: %w", err)
40+
var schemaErr SchemaNotInitializedError
41+
require.ErrorAs(t, wrappedErr, &schemaErr)
42+
})
43+
}

internal/datastore/crdb/caveat.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/jackc/pgx/v5"
1111

1212
"github.com/authzed/spicedb/internal/datastore/crdb/schema"
13+
pgxcommon "github.com/authzed/spicedb/internal/datastore/postgres/common"
1314
"github.com/authzed/spicedb/internal/datastore/revisions"
1415
"github.com/authzed/spicedb/pkg/datastore"
1516
core "github.com/authzed/spicedb/pkg/proto/core/v1"
@@ -53,6 +54,9 @@ func (cr *crdbReader) ReadCaveatByName(ctx context.Context, name string) (*core.
5354
if errors.Is(err, pgx.ErrNoRows) {
5455
err = datastore.NewCaveatNameNotFoundErr(name)
5556
}
57+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
58+
return nil, datastore.NoRevision, wrappedErr
59+
}
5660
return nil, datastore.NoRevision, fmt.Errorf(errReadCaveat, name, err)
5761
}
5862

@@ -109,6 +113,9 @@ func (cr *crdbReader) lookupCaveats(ctx context.Context, caveatNames []string) (
109113
return nil
110114
}, sql, args...)
111115
if err != nil {
116+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
117+
return nil, wrappedErr
118+
}
112119
return nil, fmt.Errorf(errListCaveats, err)
113120
}
114121

internal/datastore/crdb/reader.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,9 @@ func (cr *crdbReader) CountRelationships(ctx context.Context, name string) (int,
128128
return row.Scan(&count)
129129
}, sql, args...)
130130
if err != nil {
131+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
132+
return 0, wrappedErr
133+
}
131134
return 0, err
132135
}
133136

@@ -192,6 +195,9 @@ func (cr *crdbReader) lookupCounters(ctx context.Context, optionalFilterName str
192195
return nil
193196
}, sql, args...)
194197
if err != nil {
198+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
199+
return nil, wrappedErr
200+
}
195201
return nil, err
196202
}
197203

@@ -207,6 +213,9 @@ func (cr *crdbReader) ReadNamespaceByName(
207213
if errors.As(err, &datastore.NamespaceNotFoundError{}) {
208214
return nil, datastore.NoRevision, err
209215
}
216+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
217+
return nil, datastore.NoRevision, wrappedErr
218+
}
210219
return nil, datastore.NoRevision, fmt.Errorf(errUnableToReadConfig, err)
211220
}
212221

@@ -220,6 +229,9 @@ func (cr *crdbReader) ListAllNamespaces(ctx context.Context) ([]datastore.Revisi
220229

221230
nsDefs, sql, err := loadAllNamespaces(ctx, cr.query, addFromToQuery)
222231
if err != nil {
232+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
233+
return nil, wrappedErr
234+
}
223235
return nil, fmt.Errorf(errUnableToListNamespaces, err)
224236
}
225237
cr.assertHasExpectedAsOfSystemTime(sql)
@@ -232,6 +244,9 @@ func (cr *crdbReader) LookupNamespacesWithNames(ctx context.Context, nsNames []s
232244
}
233245
nsDefs, err := cr.lookupNamespaces(ctx, cr.query, nsNames)
234246
if err != nil {
247+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
248+
return nil, wrappedErr
249+
}
235250
return nil, fmt.Errorf(errUnableToListNamespaces, err)
236251
}
237252
return nsDefs, nil

internal/datastore/crdb/stats.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ func (cds *crdbDatastore) UniqueID(ctx context.Context) (string, error) {
3333
if err := cds.readPool.QueryRowFunc(ctx, func(ctx context.Context, row pgx.Row) error {
3434
return row.Scan(&uniqueID)
3535
}, sql, args...); err != nil {
36+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
37+
return "", wrappedErr
38+
}
3639
return "", fmt.Errorf("unable to query unique ID: %w", err)
3740
}
3841

@@ -59,6 +62,9 @@ func (cds *crdbDatastore) Statistics(ctx context.Context) (datastore.Stats, erro
5962
return sb.From(tableName)
6063
})
6164
if err != nil {
65+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
66+
return wrappedErr
67+
}
6268
return fmt.Errorf("unable to read namespaces: %w", err)
6369
}
6470
return nil
@@ -69,6 +75,9 @@ func (cds *crdbDatastore) Statistics(ctx context.Context) (datastore.Stats, erro
6975
if cds.analyzeBeforeStatistics {
7076
if err := cds.readPool.BeginTxFunc(ctx, pgx.TxOptions{AccessMode: pgx.ReadOnly}, func(tx pgx.Tx) error {
7177
if _, err := tx.Exec(ctx, "ANALYZE "+cds.schema.RelationshipTableName); err != nil {
78+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
79+
return wrappedErr
80+
}
7281
return fmt.Errorf("unable to analyze tuple table: %w", err)
7382
}
7483

@@ -143,6 +152,9 @@ func (cds *crdbDatastore) Statistics(ctx context.Context) (datastore.Stats, erro
143152
log.Warn().Bool("has-rows", hasRows).Msg("unable to find row count in statistics query result")
144153
return nil
145154
}, "SHOW STATISTICS FOR TABLE "+cds.schema.RelationshipTableName); err != nil {
155+
if wrappedErr := pgxcommon.WrapMissingTableError(err); wrappedErr != nil {
156+
return datastore.Stats{}, wrappedErr
157+
}
146158
return datastore.Stats{}, fmt.Errorf("unable to query unique estimated row count: %w", err)
147159
}
148160

internal/datastore/mysql/caveat.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
sq "github.com/Masterminds/squirrel"
1010

1111
"github.com/authzed/spicedb/internal/datastore/common"
12+
mysqlcommon "github.com/authzed/spicedb/internal/datastore/mysql/common"
1213
"github.com/authzed/spicedb/internal/datastore/revisions"
1314
"github.com/authzed/spicedb/pkg/datastore"
1415
core "github.com/authzed/spicedb/pkg/proto/core/v1"
@@ -41,6 +42,9 @@ func (mr *mysqlReader) ReadCaveatByName(ctx context.Context, name string) (*core
4142
if errors.Is(err, sql.ErrNoRows) {
4243
return nil, datastore.NoRevision, datastore.NewCaveatNameNotFoundErr(name)
4344
}
45+
if wrappedErr := mysqlcommon.WrapMissingTableError(err); wrappedErr != nil {
46+
return nil, datastore.NoRevision, wrappedErr
47+
}
4448
return nil, datastore.NoRevision, fmt.Errorf(errReadCaveat, err)
4549
}
4650
def := core.CaveatDefinition{}
@@ -82,6 +86,9 @@ func (mr *mysqlReader) lookupCaveats(ctx context.Context, caveatNames []string)
8286

8387
rows, err := tx.QueryContext(ctx, listSQL, listArgs...)
8488
if err != nil {
89+
if wrappedErr := mysqlcommon.WrapMissingTableError(err); wrappedErr != nil {
90+
return nil, wrappedErr
91+
}
8592
return nil, fmt.Errorf(errListCaveats, err)
8693
}
8794
defer common.LogOnError(ctx, rows.Close)
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package common
2+
3+
import (
4+
"errors"
5+
6+
"github.com/go-sql-driver/mysql"
7+
8+
dscommon "github.com/authzed/spicedb/internal/datastore/common"
9+
)
10+
11+
const (
12+
// mysqlMissingTableErrorNumber is the MySQL error number for "table doesn't exist".
13+
// This corresponds to MySQL error 1146 (ER_NO_SUCH_TABLE) with SQLSTATE 42S02.
14+
mysqlMissingTableErrorNumber = 1146
15+
)
16+
17+
// IsMissingTableError returns true if the error is a MySQL error indicating a missing table.
18+
// This typically happens when migrations have not been run.
19+
func IsMissingTableError(err error) bool {
20+
var mysqlErr *mysql.MySQLError
21+
return errors.As(err, &mysqlErr) && mysqlErr.Number == mysqlMissingTableErrorNumber
22+
}
23+
24+
// WrapMissingTableError checks if the error is a missing table error and wraps it with
25+
// a helpful message instructing the user to run migrations. If it's not a missing table error,
26+
// it returns nil. If it's already a SchemaNotInitializedError, it returns the original error
27+
// to preserve the wrapped error through the call chain.
28+
func WrapMissingTableError(err error) error {
29+
// Don't double-wrap if already a SchemaNotInitializedError - return original to preserve it
30+
var schemaErr dscommon.SchemaNotInitializedError
31+
if errors.As(err, &schemaErr) {
32+
return err
33+
}
34+
if IsMissingTableError(err) {
35+
return dscommon.NewSchemaNotInitializedError(err)
36+
}
37+
return nil
38+
}
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package common
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/go-sql-driver/mysql"
8+
"github.com/stretchr/testify/require"
9+
10+
dscommon "github.com/authzed/spicedb/internal/datastore/common"
11+
)
12+
13+
func TestIsMissingTableError(t *testing.T) {
14+
t.Run("returns true for missing table error", func(t *testing.T) {
15+
mysqlErr := &mysql.MySQLError{
16+
Number: mysqlMissingTableErrorNumber,
17+
Message: "Table 'spicedb.caveat' doesn't exist",
18+
}
19+
require.True(t, IsMissingTableError(mysqlErr))
20+
})
21+
22+
t.Run("returns false for other mysql errors", func(t *testing.T) {
23+
mysqlErr := &mysql.MySQLError{
24+
Number: 1062, // Duplicate entry error
25+
Message: "Duplicate entry '1' for key 'PRIMARY'",
26+
}
27+
require.False(t, IsMissingTableError(mysqlErr))
28+
})
29+
30+
t.Run("returns false for non-mysql errors", func(t *testing.T) {
31+
err := fmt.Errorf("some other error")
32+
require.False(t, IsMissingTableError(err))
33+
})
34+
35+
t.Run("returns false for nil error", func(t *testing.T) {
36+
require.False(t, IsMissingTableError(nil))
37+
})
38+
39+
t.Run("returns true for wrapped missing table error", func(t *testing.T) {
40+
mysqlErr := &mysql.MySQLError{
41+
Number: mysqlMissingTableErrorNumber,
42+
Message: "Table 'spicedb.caveat' doesn't exist",
43+
}
44+
wrappedErr := fmt.Errorf("query failed: %w", mysqlErr)
45+
require.True(t, IsMissingTableError(wrappedErr))
46+
})
47+
}
48+
49+
func TestWrapMissingTableError(t *testing.T) {
50+
t.Run("wraps missing table error", func(t *testing.T) {
51+
mysqlErr := &mysql.MySQLError{
52+
Number: mysqlMissingTableErrorNumber,
53+
Message: "Table 'spicedb.caveat' doesn't exist",
54+
}
55+
wrapped := WrapMissingTableError(mysqlErr)
56+
require.Error(t, wrapped)
57+
58+
var schemaErr dscommon.SchemaNotInitializedError
59+
require.ErrorAs(t, wrapped, &schemaErr)
60+
require.Contains(t, wrapped.Error(), "spicedb datastore migrate")
61+
})
62+
63+
t.Run("returns nil for non-missing-table errors", func(t *testing.T) {
64+
mysqlErr := &mysql.MySQLError{
65+
Number: 1062, // Duplicate entry error
66+
Message: "Duplicate entry '1' for key 'PRIMARY'",
67+
}
68+
require.NoError(t, WrapMissingTableError(mysqlErr))
69+
})
70+
71+
t.Run("returns nil for non-mysql errors", func(t *testing.T) {
72+
err := fmt.Errorf("some other error")
73+
require.NoError(t, WrapMissingTableError(err))
74+
})
75+
76+
t.Run("returns nil for nil error", func(t *testing.T) {
77+
require.NoError(t, WrapMissingTableError(nil))
78+
})
79+
80+
t.Run("preserves original error in chain", func(t *testing.T) {
81+
mysqlErr := &mysql.MySQLError{
82+
Number: mysqlMissingTableErrorNumber,
83+
Message: "Table 'spicedb.caveat' doesn't exist",
84+
}
85+
wrapped := WrapMissingTableError(mysqlErr)
86+
require.Error(t, wrapped)
87+
88+
// The original mysql error should be accessible via unwrapping
89+
var foundMySQLErr *mysql.MySQLError
90+
require.ErrorAs(t, wrapped, &foundMySQLErr)
91+
require.Equal(t, uint16(mysqlMissingTableErrorNumber), foundMySQLErr.Number)
92+
})
93+
94+
t.Run("does not double-wrap already wrapped errors", func(t *testing.T) {
95+
mysqlErr := &mysql.MySQLError{
96+
Number: mysqlMissingTableErrorNumber,
97+
Message: "Table 'spicedb.caveat' doesn't exist",
98+
}
99+
// First wrap
100+
wrapped := WrapMissingTableError(mysqlErr)
101+
require.Error(t, wrapped)
102+
103+
// Second wrap should return the already-wrapped error (preserving it through call chain)
104+
doubleWrapped := WrapMissingTableError(wrapped)
105+
require.Error(t, doubleWrapped)
106+
require.Equal(t, wrapped, doubleWrapped)
107+
108+
// Should still be detectable as SchemaNotInitializedError
109+
var schemaErr dscommon.SchemaNotInitializedError
110+
require.ErrorAs(t, doubleWrapped, &schemaErr)
111+
})
112+
}

0 commit comments

Comments
 (0)