From c781a10a719aa1323646b1b069e0f3f3f9f8a674 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 16 Sep 2024 10:15:39 -0600 Subject: [PATCH] Wrap errors where applicable --- internal/datastore/common/changes.go | 7 ++++--- internal/datastore/common/sql.go | 5 +---- internal/datastore/crdb/crdb.go | 4 ++-- internal/datastore/crdb/readwrite.go | 2 +- internal/datastore/crdb/stats.go | 2 +- internal/datastore/mysql/gc.go | 2 +- internal/datastore/mysql/readwrite.go | 2 +- internal/datastore/mysql/revisions.go | 2 +- internal/datastore/mysql/stats.go | 2 +- internal/datastore/postgres/common/bulk.go | 2 +- internal/datastore/postgres/readwrite.go | 2 +- internal/datastore/postgres/revisions.go | 6 +++--- internal/datastore/revisions/hlcrevision.go | 6 ++++-- internal/datastore/spanner/readwrite.go | 2 +- internal/datastore/spanner/stats.go | 4 ++-- internal/graph/cursors.go | 2 +- 16 files changed, 26 insertions(+), 26 deletions(-) diff --git a/internal/datastore/common/changes.go b/internal/datastore/common/changes.go index 342155ed9d..78d7636580 100644 --- a/internal/datastore/common/changes.go +++ b/internal/datastore/common/changes.go @@ -120,9 +120,10 @@ func (ch *Changes[R, K]) adjustByteSize(item sized, delta int) error { return spiceerrors.MustBugf("byte size underflow") } - // We checked for underflow above, so the current byte size - // should fit in a uint64 - currentByteSize, _ := safecast.ToUint64(ch.currentByteSize) + currentByteSize, err := safecast.ToUint64(ch.currentByteSize) + if err != nil { + return spiceerrors.MustBugf("could not cast currentByteSize to uint64: %v", err) + } if currentByteSize > ch.maxByteSize { return datastore.NewMaximumChangesSizeExceededError(ch.maxByteSize) diff --git a/internal/datastore/common/sql.go b/internal/datastore/common/sql.go index 1bccadbb7c..f762999c05 100644 --- a/internal/datastore/common/sql.go +++ b/internal/datastore/common/sql.go @@ -7,7 +7,6 @@ import ( sq "github.com/Masterminds/squirrel" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" - "github.com/ccoveille/go-safecast" "github.com/jzelinskie/stringz" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -573,9 +572,7 @@ func (tqs QueryExecutor) ExecuteQuery( return nil, err } - // A length shouldn't be non-negative, so we can cast without - // checking here. - lenQueryTuples, _ := safecast.ToUint64(len(queryTuples)) + lenQueryTuples := uint64(len(queryTuples)) if lenQueryTuples > limit { queryTuples = queryTuples[:limit] } diff --git a/internal/datastore/crdb/crdb.go b/internal/datastore/crdb/crdb.go index 6a6855f9d4..901f8bd754 100644 --- a/internal/datastore/crdb/crdb.go +++ b/internal/datastore/crdb/crdb.go @@ -424,11 +424,11 @@ func (cds *crdbDatastore) ReadyState(ctx context.Context) (datastore.ReadyState, } writeTotal, err := safecast.ToUint32(cds.writePool.Stat().TotalConns()) if err != nil { - return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast writeTotal to uint32") + return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast writeTotal to uint32: %v", err) } readTotal, err := safecast.ToUint32(cds.readPool.Stat().TotalConns()) if err != nil { - return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast readTotal to uint32") + return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast readTotal to uint32: %v", err) } if writeTotal < writeMin || readTotal < readMin { return datastore.ReadyState{ diff --git a/internal/datastore/crdb/readwrite.go b/internal/datastore/crdb/readwrite.go index 410e4e9ae1..626f5e8153 100644 --- a/internal/datastore/crdb/readwrite.go +++ b/internal/datastore/crdb/readwrite.go @@ -433,7 +433,7 @@ func (rwt *crdbReadWriteTXN) DeleteRelationships(ctx context.Context, filter *v1 rwt.relCountChange -= modified.RowsAffected() rowsAffected, err := safecast.ToUint64(modified.RowsAffected()) if err != nil { - return false, spiceerrors.MustBugf("could not cast RowsAffected to uint64") + return false, spiceerrors.MustBugf("could not cast RowsAffected to uint64: %v", err) } if delLimit > 0 && rowsAffected == delLimit { return true, nil diff --git a/internal/datastore/crdb/stats.go b/internal/datastore/crdb/stats.go index dd28426ae4..2b66297d91 100644 --- a/internal/datastore/crdb/stats.go +++ b/internal/datastore/crdb/stats.go @@ -122,7 +122,7 @@ func (cds *crdbDatastore) Statistics(ctx context.Context) (datastore.Stats, erro uintRowCount, err := safecast.ToUint64(rowCount) if err != nil { - return spiceerrors.MustBugf("row count was negative") + return spiceerrors.MustBugf("row count was negative: %v", err) } estimatedRelCount = uintRowCount return nil diff --git a/internal/datastore/mysql/gc.go b/internal/datastore/mysql/gc.go index e890e41490..bde07fe40a 100644 --- a/internal/datastore/mysql/gc.go +++ b/internal/datastore/mysql/gc.go @@ -68,7 +68,7 @@ func (mds *Datastore) TxIDBefore(ctx context.Context, before time.Time) (datasto uintValue, err := safecast.ToUint64(value.Int64) if err != nil { - return datastore.NoRevision, spiceerrors.MustBugf("value could not be cast to uint64") + return datastore.NoRevision, spiceerrors.MustBugf("value could not be cast to uint64: %v", err) } return revisions.NewForTransactionID(uintValue), nil diff --git a/internal/datastore/mysql/readwrite.go b/internal/datastore/mysql/readwrite.go index f351764e2a..7a283d8d5a 100644 --- a/internal/datastore/mysql/readwrite.go +++ b/internal/datastore/mysql/readwrite.go @@ -377,7 +377,7 @@ func (rwt *mysqlReadWriteTXN) DeleteRelationships(ctx context.Context, filter *v uintRowsAffected, err := safecast.ToUint64(rowsAffected) if err != nil { - return false, spiceerrors.MustBugf("rowsAffected was negative") + return false, spiceerrors.MustBugf("rowsAffected was negative: %v", err) } if delLimit > 0 && uintRowsAffected == delLimit { diff --git a/internal/datastore/mysql/revisions.go b/internal/datastore/mysql/revisions.go index aad60cc432..1128622322 100644 --- a/internal/datastore/mysql/revisions.go +++ b/internal/datastore/mysql/revisions.go @@ -180,7 +180,7 @@ func (mds *Datastore) createNewTransaction(ctx context.Context, tx *sql.Tx) (new uintLastInsertID, err := safecast.ToUint64(lastInsertID) if err != nil { - return 0, spiceerrors.MustBugf("lastInsertID was negative") + return 0, spiceerrors.MustBugf("lastInsertID was negative: %v", err) } return uintLastInsertID, nil diff --git a/internal/datastore/mysql/stats.go b/internal/datastore/mysql/stats.go index d4efcc4320..340e943c55 100644 --- a/internal/datastore/mysql/stats.go +++ b/internal/datastore/mysql/stats.go @@ -78,7 +78,7 @@ func (mds *Datastore) Statistics(ctx context.Context) (datastore.Stats, error) { uintCount, err := safecast.ToUint64(count.Int64) if err != nil { - return datastore.Stats{}, spiceerrors.MustBugf("could not cast count to uint64") + return datastore.Stats{}, spiceerrors.MustBugf("could not cast count to uint64: %v", err) } return datastore.Stats{ diff --git a/internal/datastore/postgres/common/bulk.go b/internal/datastore/postgres/common/bulk.go index 613d8cefbd..1d12e7bd9c 100644 --- a/internal/datastore/postgres/common/bulk.go +++ b/internal/datastore/postgres/common/bulk.go @@ -78,7 +78,7 @@ func BulkLoad( copied, err := tx.CopyFrom(ctx, pgx.Identifier{tupleTableName}, colNames, adapter) uintCopied, castErr := safecast.ToUint64(copied) if castErr != nil { - return 0, spiceerrors.MustBugf("number copied was negative") + return 0, spiceerrors.MustBugf("number copied was negative: %v", castErr) } return uintCopied, err } diff --git a/internal/datastore/postgres/readwrite.go b/internal/datastore/postgres/readwrite.go index d2f89dbaa0..6e182236b2 100644 --- a/internal/datastore/postgres/readwrite.go +++ b/internal/datastore/postgres/readwrite.go @@ -388,7 +388,7 @@ func (rwt *pgReadWriteTXN) deleteRelationshipsWithLimit(ctx context.Context, fil // validate the limit intLimit, err := safecast.ToInt64(limit) if err != nil { - return false, fmt.Errorf("limit argument could not safely be cast to int64") + return false, fmt.Errorf("limit argument could not safely be cast to int64: %w", err) } // Construct a select query for the relationships to be removed. diff --git a/internal/datastore/postgres/revisions.go b/internal/datastore/postgres/revisions.go index d25291778b..e35a268d1a 100644 --- a/internal/datastore/postgres/revisions.go +++ b/internal/datastore/postgres/revisions.go @@ -328,20 +328,20 @@ func (pr postgresRevision) OptionalNanosTimestamp() (uint64, bool) { func (pr postgresRevision) MarshalBinary() ([]byte, error) { xminInt, err := safecast.ToInt64(pr.snapshot.xmin) if err != nil { - return nil, spiceerrors.MustBugf("could not safely cast snapshot xip to int64") + return nil, spiceerrors.MustBugf("could not safely cast snapshot xip to int64: %v", err) } relativeXips := make([]int64, len(pr.snapshot.xipList)) for i, xip := range pr.snapshot.xipList { intXip, err := safecast.ToInt64(xip) if err != nil { - return nil, spiceerrors.MustBugf("could not safely cast snapshot xip to int64") + return nil, spiceerrors.MustBugf("could not safely cast snapshot xip to int64: %v", err) } relativeXips[i] = intXip - xminInt } relativeXmax, err := safecast.ToInt64(pr.snapshot.xmax) if err != nil { - return nil, spiceerrors.MustBugf("could not safely cast snapshot xmax to int64") + return nil, spiceerrors.MustBugf("could not safely cast snapshot xmax to int64: %v", err) } protoRevision := implv1.PostgresRevision{ Xmin: pr.snapshot.xmin, diff --git a/internal/datastore/revisions/hlcrevision.go b/internal/datastore/revisions/hlcrevision.go index 5e0a70c8af..20e0ae645e 100644 --- a/internal/datastore/revisions/hlcrevision.go +++ b/internal/datastore/revisions/hlcrevision.go @@ -61,8 +61,10 @@ func parseHLCRevisionString(revisionStr string) (datastore.Revision, error) { return datastore.NoRevision, fmt.Errorf("invalid revision string: %q", revisionStr) } - // Because we parsed with a bit size of 32 above, we know this range check should pass. - uintLogicalClock, _ := safecast.ToUint32(logicalclock) + uintLogicalClock, err := safecast.ToUint32(logicalclock) + if err != nil { + return datastore.NoRevision, spiceerrors.MustBugf("could not cast logicalclock to uint32: %v", err) + } return HLCRevision{timestamp, uintLogicalClock + logicalClockOffset}, nil } diff --git a/internal/datastore/spanner/readwrite.go b/internal/datastore/spanner/readwrite.go index 72852ab997..a83fe194f2 100644 --- a/internal/datastore/spanner/readwrite.go +++ b/internal/datastore/spanner/readwrite.go @@ -182,7 +182,7 @@ func deleteWithFilter(ctx context.Context, rwt *spanner.ReadWriteTransaction, fi uintNumDeleted, err := safecast.ToUint64(numDeleted) if err != nil { - return false, spiceerrors.MustBugf("numDeleted was negative") + return false, spiceerrors.MustBugf("numDeleted was negative: %v", err) } if delLimit > 0 && uintNumDeleted == delLimit { diff --git a/internal/datastore/spanner/stats.go b/internal/datastore/spanner/stats.go index 2dd422bc7a..6f0e420ce7 100644 --- a/internal/datastore/spanner/stats.go +++ b/internal/datastore/spanner/stats.go @@ -106,7 +106,7 @@ func (sd *spannerDatastore) Statistics(ctx context.Context) (datastore.Stats, er estimatedBytesPerRelationship, err = safecast.ToUint64(totalByteCount / totalRelationships) if err != nil { - return datastore.Stats{}, spiceerrors.MustBugf("could not cast estimated bytes to uint64") + return datastore.Stats{}, spiceerrors.MustBugf("could not cast estimated bytes to uint64: %v", err) } if estimatedBytesPerRelationship > 0 { sd.cachedEstimatedBytesPerRelationshipLock.Lock() @@ -147,7 +147,7 @@ func (sd *spannerDatastore) Statistics(ctx context.Context) (datastore.Stats, er uintByteEstimate, err := safecast.ToUint64(byteEstimate.Int64) if err != nil { - return datastore.Stats{}, spiceerrors.MustBugf("unable to cast byteEstimate to uint64") + return datastore.Stats{}, spiceerrors.MustBugf("unable to cast byteEstimate to uint64: %v", err) } return datastore.Stats{ diff --git a/internal/graph/cursors.go b/internal/graph/cursors.go index 021f137674..d73fd5bb8d 100644 --- a/internal/graph/cursors.go +++ b/internal/graph/cursors.go @@ -505,7 +505,7 @@ func (ls *parallelLimitedIndexedStream[Q]) completedTaskIndex(index int) error { // Remove the already emitted data from the overall limits. publishedCount, err := safecast.ToUint32(ls.countingStream.PublishedCount()) if err != nil { - return spiceerrors.MustBugf("cannot cast published count to uint32") + return spiceerrors.MustBugf("cannot cast published count to uint32: %v", err) } if err := ls.ci.limits.markAlreadyPublished(publishedCount); err != nil { return err