Ensure NewForHLC returns an error if the incoming decimal is invalid

This shouldn't be possible, but there are some indications it might be

Author: josephschorr

internal/datastore/crdb/crdb.go

@@ -478,7 +478,7 @@ func readCRDBNow(ctx context.Context, reader pgxcommon.DBFuncQuerier) (datastore
 		return datastore.NoRevision, fmt.Errorf("unable to read timestamp: %w", err)
 	}
 
-	return revisions.NewForHLC(hlcNow), nil
+	return revisions.NewForHLC(hlcNow)
 }
 
 func readClusterTTLNanos(ctx context.Context, conn pgxcommon.DBFuncQuerier) (int64, error) {

internal/datastore/crdb/stats.go

@@ -105,5 +105,5 @@ func updateCounter(ctx context.Context, tx pgx.Tx, change int64) (datastore.Revi
 		return datastore.NoRevision, fmt.Errorf("unable to executed upsert counter query: %w", err)
 	}
 
-	return revisions.NewForHLC(timestamp), nil
+	return revisions.NewForHLC(timestamp)
 }

internal/datastore/revisions/hlcrevision.go

@@ -73,9 +73,13 @@ func HLCRevisionFromString(revisionStr string) (HLCRevision, error) {
 }
 
 // NewForHLC creates a new revision for the given hybrid logical clock.
-func NewForHLC(decimal decimal.Decimal) HLCRevision {
-	rev, _ := HLCRevisionFromString(decimal.String())
-	return rev
+func NewForHLC(decimal decimal.Decimal) (HLCRevision, error) {
+	rev, err := HLCRevisionFromString(decimal.String())
+	if err != nil {
+		return zeroHLC, fmt.Errorf("invalid HLC decimal: %v (%s) => %w", decimal, decimal.String(), err)
+	}
+
+	return rev, nil
 }
 
 // NewHLCForTime creates a new revision for the given time.

internal/datastore/revisions/hlcrevision_test.go

@@ -24,7 +24,8 @@ func TestNewForHLC(t *testing.T) {
 			d, err := decimal.NewFromString(tc)
 			require.NoError(t, err)
 
-			rev := NewForHLC(d)
+			rev, err := NewForHLC(d)
+			require.NoError(t, err)
 			require.Equal(t, tc, rev.String())
 		})
 	}

pkg/zedtoken/zedtoken_test.go

@@ -166,10 +166,16 @@ var hlcDecodeTests = []struct {
 	expectError      bool
 }{
 	{
-		format:           "V1 ZedToken",
-		token:            "CAIaFQoTMTYyMTUzODE4OTAyODkyODAwMA==",
-		expectedRevision: revisions.NewForHLC(decimal.NewFromInt(1621538189028928000)),
-		expectError:      false,
+		format: "V1 ZedToken",
+		token:  "CAIaFQoTMTYyMTUzODE4OTAyODkyODAwMA==",
+		expectedRevision: func() datastore.Revision {
+			r, err := revisions.NewForHLC(decimal.NewFromInt(1621538189028928000))
+			if err != nil {
+				panic(err)
+			}
+			return r
+		}(),
+		expectError: false,
 	},
 	{
 		format: "V1 ZedToken",
@@ -179,7 +185,13 @@ var hlcDecodeTests = []struct {
 			if err != nil {
 				panic(err)
 			}
-			return revisions.NewForHLC(v)
+
+			r, err := revisions.NewForHLC(v)
+			if err != nil {
+				panic(err)
+			}
+
+			return r
 		})(),
 		expectError: false,
 	},