Skip to content

Commit 8ec386b

Browse files
committed
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 8ec386b

File tree

17 files changed

+478
-0
lines changed

17 files changed

+478
-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: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package common
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
"google.golang.org/grpc/codes"
10+
)
11+
12+
func TestSchemaNotInitializedError(t *testing.T) {
13+
underlyingErr := fmt.Errorf("relation \"caveat\" does not exist (SQLSTATE 42P01)")
14+
err := NewSchemaNotInitializedError(underlyingErr)
15+
16+
t.Run("error message contains migration instructions", func(t *testing.T) {
17+
require.Contains(t, err.Error(), "spicedb datastore migrate")
18+
require.Contains(t, err.Error(), "database schema has not been initialized")
19+
})
20+
21+
t.Run("unwrap returns underlying error", func(t *testing.T) {
22+
var schemaErr SchemaNotInitializedError
23+
require.True(t, errors.As(err, &schemaErr))
24+
require.ErrorIs(t, schemaErr.Unwrap(), underlyingErr)
25+
})
26+
27+
t.Run("grpc status is FailedPrecondition", func(t *testing.T) {
28+
var schemaErr SchemaNotInitializedError
29+
require.True(t, errors.As(err, &schemaErr))
30+
status := schemaErr.GRPCStatus()
31+
require.Equal(t, codes.FailedPrecondition, status.Code())
32+
})
33+
34+
t.Run("can be detected with errors.As", func(t *testing.T) {
35+
var schemaErr SchemaNotInitializedError
36+
require.True(t, errors.As(err, &schemaErr))
37+
})
38+
39+
t.Run("wrapped error preserves chain", func(t *testing.T) {
40+
wrappedErr := fmt.Errorf("outer: %w", err)
41+
var schemaErr SchemaNotInitializedError
42+
require.True(t, errors.As(wrappedErr, &schemaErr))
43+
})
44+
}

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: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
package common
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/go-sql-driver/mysql"
9+
"github.com/stretchr/testify/require"
10+
11+
dscommon "github.com/authzed/spicedb/internal/datastore/common"
12+
)
13+
14+
func TestIsMissingTableError(t *testing.T) {
15+
t.Run("returns true for missing table error", func(t *testing.T) {
16+
mysqlErr := &mysql.MySQLError{
17+
Number: mysqlMissingTableErrorNumber,
18+
Message: "Table 'spicedb.caveat' doesn't exist",
19+
}
20+
require.True(t, IsMissingTableError(mysqlErr))
21+
})
22+
23+
t.Run("returns false for other mysql errors", func(t *testing.T) {
24+
mysqlErr := &mysql.MySQLError{
25+
Number: 1062, // Duplicate entry error
26+
Message: "Duplicate entry '1' for key 'PRIMARY'",
27+
}
28+
require.False(t, IsMissingTableError(mysqlErr))
29+
})
30+
31+
t.Run("returns false for non-mysql errors", func(t *testing.T) {
32+
err := fmt.Errorf("some other error")
33+
require.False(t, IsMissingTableError(err))
34+
})
35+
36+
t.Run("returns false for nil error", func(t *testing.T) {
37+
require.False(t, IsMissingTableError(nil))
38+
})
39+
40+
t.Run("returns true for wrapped missing table error", func(t *testing.T) {
41+
mysqlErr := &mysql.MySQLError{
42+
Number: mysqlMissingTableErrorNumber,
43+
Message: "Table 'spicedb.caveat' doesn't exist",
44+
}
45+
wrappedErr := fmt.Errorf("query failed: %w", mysqlErr)
46+
require.True(t, IsMissingTableError(wrappedErr))
47+
})
48+
}
49+
50+
func TestWrapMissingTableError(t *testing.T) {
51+
t.Run("wraps missing table error", func(t *testing.T) {
52+
mysqlErr := &mysql.MySQLError{
53+
Number: mysqlMissingTableErrorNumber,
54+
Message: "Table 'spicedb.caveat' doesn't exist",
55+
}
56+
wrapped := WrapMissingTableError(mysqlErr)
57+
require.NotNil(t, wrapped)
58+
59+
var schemaErr dscommon.SchemaNotInitializedError
60+
require.True(t, errors.As(wrapped, &schemaErr))
61+
require.Contains(t, wrapped.Error(), "spicedb datastore migrate")
62+
})
63+
64+
t.Run("returns nil for non-missing-table errors", func(t *testing.T) {
65+
mysqlErr := &mysql.MySQLError{
66+
Number: 1062, // Duplicate entry error
67+
Message: "Duplicate entry '1' for key 'PRIMARY'",
68+
}
69+
require.Nil(t, WrapMissingTableError(mysqlErr))
70+
})
71+
72+
t.Run("returns nil for non-mysql errors", func(t *testing.T) {
73+
err := fmt.Errorf("some other error")
74+
require.Nil(t, WrapMissingTableError(err))
75+
})
76+
77+
t.Run("returns nil for nil error", func(t *testing.T) {
78+
require.Nil(t, WrapMissingTableError(nil))
79+
})
80+
81+
t.Run("preserves original error in chain", func(t *testing.T) {
82+
mysqlErr := &mysql.MySQLError{
83+
Number: mysqlMissingTableErrorNumber,
84+
Message: "Table 'spicedb.caveat' doesn't exist",
85+
}
86+
wrapped := WrapMissingTableError(mysqlErr)
87+
require.NotNil(t, wrapped)
88+
89+
// The original mysql error should be accessible via unwrapping
90+
var foundMySQLErr *mysql.MySQLError
91+
require.True(t, errors.As(wrapped, &foundMySQLErr))
92+
require.Equal(t, uint16(mysqlMissingTableErrorNumber), foundMySQLErr.Number)
93+
})
94+
95+
t.Run("does not double-wrap already wrapped errors", func(t *testing.T) {
96+
mysqlErr := &mysql.MySQLError{
97+
Number: mysqlMissingTableErrorNumber,
98+
Message: "Table 'spicedb.caveat' doesn't exist",
99+
}
100+
// First wrap
101+
wrapped := WrapMissingTableError(mysqlErr)
102+
require.NotNil(t, wrapped)
103+
104+
// Second wrap should return the already-wrapped error (preserving it through call chain)
105+
doubleWrapped := WrapMissingTableError(wrapped)
106+
require.NotNil(t, doubleWrapped)
107+
require.Equal(t, wrapped, doubleWrapped)
108+
109+
// Should still be detectable as SchemaNotInitializedError
110+
var schemaErr dscommon.SchemaNotInitializedError
111+
require.True(t, errors.As(doubleWrapped, &schemaErr))
112+
})
113+
}

0 commit comments

Comments
 (0)