sql: address instances of improper error wrapping

Release note: None
cockroachdb · Dec 11, 2021 · 3e3b98e · 3e3b98e
1 parent 0386504
commit 3e3b98e
Show file tree

Hide file tree

Showing 16 changed files with 41 additions and 40 deletions.
diff --git a/pkg/sql/catalog/bootstrap/metadata.go b/pkg/sql/catalog/bootstrap/metadata.go
@@ -289,7 +289,7 @@ func createZoneConfigKV(
 ) roachpb.KeyValue {
 	value := roachpb.Value{}
 	if err := value.SetProto(zoneConfig); err != nil {
-		panic(errors.AssertionFailedf("could not marshal ZoneConfig for ID: %d: %s", keyID, err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "could not marshal ZoneConfig for ID: %d", keyID))
 	}
 	return roachpb.KeyValue{
 		Key:   codec.ZoneKey(uint32(keyID)),

diff --git a/pkg/sql/colexec/colexectestutils/utils.go b/pkg/sql/colexec/colexectestutils/utils.go
@@ -760,7 +760,7 @@ func setColVal(vec coldata.Vec, idx int, val interface{}, evalCtx *tree.EvalCont
 			decimalVal, _, err := apd.NewFromString(fmt.Sprintf("%f", floatVal))
 			if err != nil {
 				colexecerror.InternalError(
-					errors.AssertionFailedf("unable to set decimal %f: %v", floatVal, err))
+					errors.NewAssertionErrorWithWrappedErrf(err, "unable to set decimal %f", floatVal))
 			}
 			// .Set is used here instead of assignment to ensure the pointer address
 			// of the underlying storage for apd.Decimal remains the same. This can
@@ -777,7 +777,7 @@ func setColVal(vec coldata.Vec, idx int, val interface{}, evalCtx *tree.EvalCont
 			j, err = json.ParseJSON(s)
 			if err != nil {
 				colexecerror.InternalError(
-					errors.AssertionFailedf("unable to set json %s: %v", s, err))
+					errors.NewAssertionErrorWithWrappedErrf(err, "unable to set json %s", s))
 			}
 		}
 		vec.JSON().Set(idx, j)
@@ -991,7 +991,7 @@ func (s *opTestInput) Next() coldata.Batch {
 						d := apd.Decimal{}
 						_, err := d.SetFloat64(rng.Float64())
 						if err != nil {
-							colexecerror.InternalError(errors.AssertionFailedf("%v", err))
+							colexecerror.InternalError(errors.NewAssertionErrorWithWrappedErrf(err, "error setting float"))
 						}
 						col.Index(outputIdx).Set(reflect.ValueOf(d))
 					case types.BytesFamily:
@@ -1003,7 +1003,7 @@ func (s *opTestInput) Next() coldata.Batch {
 					case types.JsonFamily:
 						j, err := json.Random(20, rng)
 						if err != nil {
-							colexecerror.InternalError(errors.AssertionFailedf("%v", err))
+							colexecerror.InternalError(errors.NewAssertionErrorWithWrappedErrf(err, "error creating json"))
 						}
 						setColVal(vec, outputIdx, j, s.evalCtx)
 					case types.TimestampTZFamily:

diff --git a/pkg/sql/colexec/columnarizer.go b/pkg/sql/colexec/columnarizer.go
@@ -134,7 +134,7 @@ func newColumnarizer(
 				// propagate it in DrainMeta.
 				if err := c.Close(); buildutil.CrdbTestBuild && err != nil {
 					// Close never returns an error.
-					colexecerror.InternalError(errors.AssertionFailedf("unexpected error %v from Columnarizer.Close", err))
+					colexecerror.InternalError(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected error from Columnarizer.Close"))
 				}
 				return nil
 			}},

diff --git a/pkg/sql/colexec/hashjoiner_test.go b/pkg/sql/colexec/hashjoiner_test.go
@@ -45,7 +45,7 @@ func init() {
 	for i, f := range floats {
 		_, err := decs[i].SetFloat64(f)
 		if err != nil {
-			colexecerror.InternalError(errors.AssertionFailedf("%v", err))
+			colexecerror.InternalError(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected error"))
 		}
 	}
 }

diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go
@@ -1112,7 +1112,7 @@ func getFinalSourceQuery(
 		tree.FmtPlaceholderFormat(func(ctx *tree.FmtCtx, placeholder *tree.Placeholder) {
 			d, err := placeholder.Eval(evalCtx)
 			if err != nil {
-				panic(errors.AssertionFailedf("failed to serialize placeholder: %s", err))
+				panic(errors.NewAssertionErrorWithWrappedErrf(err, "failed to serialize placeholder"))
 			}
 			d.Format(ctx)
 		}),

diff --git a/pkg/sql/delete_preserving_index_test.go b/pkg/sql/delete_preserving_index_test.go
@@ -219,12 +219,12 @@ func compareRevisionHistories(
 ) error {
 	decodedExpected, err := decodeVersionedValues(expectedHistory, false)
 	if err != nil {
-		return errors.Newf("error while decoding revision history %s", err)
+		return errors.Wrap(err, "error while decoding revision history")
 	}
 
 	decodedDeletePreserving, err := decodeVersionedValues(deletePreservingHistory, true)
 	if err != nil {
-		return errors.Newf("error while decoding revision history for delete-preserving encoding %s", err)
+		return errors.Wrap(err, "error while decoding revision history for delete-preserving encoding")
 	}
 
 	return compareVersionedValueWrappers(decodedExpected, expectedPrefixLength, decodedDeletePreserving, deletePreservingPrefixLength)

diff --git a/pkg/sql/execinfrapb/data.go b/pkg/sql/execinfrapb/data.go
@@ -82,7 +82,7 @@ func ExprFmtCtxBase(evalCtx *tree.EvalContext) *tree.FmtCtx {
 			func(fmtCtx *tree.FmtCtx, p *tree.Placeholder) {
 				d, err := p.Eval(evalCtx)
 				if err != nil {
-					panic(errors.AssertionFailedf("failed to serialize placeholder: %s", err))
+					panic(errors.NewAssertionErrorWithWrappedErrf(err, "failed to serialize placeholder"))
 				}
 				d.Format(fmtCtx)
 			},

diff --git a/pkg/sql/opt/norm/project_funcs.go b/pkg/sql/opt/norm/project_funcs.go
@@ -425,15 +425,15 @@ func (c *CustomFuncs) UnnestJSONFromValues(
 		tupleVals := make(memo.ScalarListExpr, len(newCols))
 		iter, err := firstJSON.ObjectIter()
 		if err != nil {
-			panic(errors.AssertionFailedf("failed to retrieve ObjectIter: %v", err))
+			panic(errors.NewAssertionErrorWithWrappedErrf(err, "failed to retrieve ObjectIter"))
 		}
 		// Iterate through the keys of the first row and use them to get the values
 		// from the current row. Add these values to the new Values rows.
 		idx := 0
 		for iter.Next() {
 			jsonVal, err := rowExpr.(*memo.ConstExpr).Value.(*tree.DJSON).FetchValKey(iter.Key())
 			if err != nil {
-				panic(errors.AssertionFailedf("FetchValKey failed: %v", err))
+				panic(errors.NewAssertionErrorWithWrappedErrf(err, "FetchValKey failed"))
 			}
 			dJSON := tree.NewDJSON(jsonVal)
 			tupleVals[idx] = c.f.ConstructConstVal(dJSON, types.Jsonb)
@@ -470,7 +470,7 @@ func (c *CustomFuncs) FoldJSONFieldAccess(
 	firstJSON := oldValues.Rows[0].(*memo.TupleExpr).Elems[0].(*memo.ConstExpr).Value.(*tree.DJSON)
 	iter, err := firstJSON.ObjectIter()
 	if err != nil {
-		panic(errors.AssertionFailedf("failed to retrieve ObjectIter: %v", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "failed to retrieve ObjectIter"))
 	}
 	idx := 0
 	for iter.Next() {
@@ -518,7 +518,7 @@ func (c *CustomFuncs) MakeColsForUnnestJSON(
 	// Create a new column for each JSON key found in dJSON.
 	iter, err := dJSON.ObjectIter()
 	if err != nil {
-		panic(errors.AssertionFailedf("failed to retrieve ObjectIter: %v", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "failed to retrieve ObjectIter"))
 	}
 	newColIDs := make(opt.ColList, 0, dJSON.Len())
 	for iter.Next() {

diff --git a/pkg/sql/opt/norm/project_set_funcs.go b/pkg/sql/opt/norm/project_set_funcs.go
@@ -122,24 +122,24 @@ func (c *CustomFuncs) ConstructValuesFromZips(zip memo.ZipExpr) memo.RelExpr {
 			}
 			generator, err := function.Overload.Generator(c.f.evalCtx, tree.Datums{t.Value})
 			if err != nil {
-				panic(errors.AssertionFailedf("generator retrieval failed: %v", err))
+				panic(errors.NewAssertionErrorWithWrappedErrf(err, "generator retrieval failed"))
 			}
 			if err = generator.Start(c.f.evalCtx.Context, c.f.evalCtx.Txn); err != nil {
-				panic(errors.AssertionFailedf("generator.Start failed: %v", err))
+				panic(errors.NewAssertionErrorWithWrappedErrf(err, "generator.Start failed"))
 			}
 
 			for j := 0; ; j++ {
 				hasNext, err := generator.Next(c.f.evalCtx.Context)
 				if err != nil {
-					panic(errors.AssertionFailedf("generator.Next failed: %v", err))
+					panic(errors.NewAssertionErrorWithWrappedErrf(err, "generator.Next failed"))
 				}
 				if !hasNext {
 					break
 				}
 
 				vals, err := generator.Values()
 				if err != nil {
-					panic(errors.AssertionFailedf("failed to retrieve values: %v", err))
+					panic(errors.NewAssertionErrorWithWrappedErrf(err, "failed to retrieve values"))
 				}
 				if len(vals) != 1 {
 					panic(errors.AssertionFailedf(

diff --git a/pkg/sql/pgwire/command_result.go b/pkg/sql/pgwire/command_result.go
@@ -124,7 +124,7 @@ func (r *commandResult) Close(ctx context.Context, t sql.TransactionStatusIndica
 
 	for _, notice := range r.buffer.notices {
 		if err := r.conn.bufferNotice(ctx, notice); err != nil {
-			panic(errors.AssertionFailedf("unexpected err when sending notice: %s", err))
+			panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err when sending notice"))
 		}
 	}
 
@@ -134,7 +134,7 @@ func (r *commandResult) Close(ctx context.Context, t sql.TransactionStatusIndica
 			paramStatusUpdate.val,
 		); err != nil {
 			panic(
-				errors.AssertionFailedf("unexpected err when sending parameter status update: %s", err),
+				errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err when sending parameter status update"),
 			)
 		}
 	}
@@ -194,8 +194,7 @@ func (r *commandResult) SetError(err error) {
 func (r *commandResult) addInternal(bufferData func()) error {
 	r.assertNotReleased()
 	if r.err != nil {
-		panic(errors.AssertionFailedf("can't call AddRow after having set error: %s",
-			r.err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(r.err, "can't call AddRow after having set error"))
 	}
 	r.conn.writerState.fi.registerCmd(r.pos)
 	if err := r.conn.GetErr(); err != nil {

diff --git a/pkg/sql/pgwire/conn.go b/pkg/sql/pgwire/conn.go
@@ -1289,7 +1289,7 @@ func (c *conn) bufferRow(
 		}
 	}
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
@@ -1345,28 +1345,28 @@ func (c *conn) bufferReadyForQuery(txnStatus byte) {
 	c.msgBuilder.initMsg(pgwirebase.ServerMsgReady)
 	c.msgBuilder.writeByte(txnStatus)
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
 func (c *conn) bufferParseComplete() {
 	c.msgBuilder.initMsg(pgwirebase.ServerMsgParseComplete)
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
 func (c *conn) bufferBindComplete() {
 	c.msgBuilder.initMsg(pgwirebase.ServerMsgBindComplete)
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
 func (c *conn) bufferCloseComplete() {
 	c.msgBuilder.initMsg(pgwirebase.ServerMsgCloseComplete)
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
@@ -1375,28 +1375,28 @@ func (c *conn) bufferCommandComplete(tag []byte) {
 	c.msgBuilder.write(tag)
 	c.msgBuilder.nullTerminate()
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
 func (c *conn) bufferPortalSuspended() {
 	c.msgBuilder.initMsg(pgwirebase.ServerMsgPortalSuspended)
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
 func (c *conn) bufferErr(ctx context.Context, err error) {
 	if err := writeErr(ctx, c.sv,
 		err, &c.msgBuilder, &c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
 func (c *conn) bufferEmptyQueryResponse() {
 	c.msgBuilder.initMsg(pgwirebase.ServerMsgEmptyQuery)
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
@@ -1467,14 +1467,14 @@ func (c *conn) bufferParamDesc(types []oid.Oid) {
 		c.msgBuilder.putInt32(int32(t))
 	}
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 
 func (c *conn) bufferNoDataMsg() {
 	c.msgBuilder.initMsg(pgwirebase.ServerMsgNoData)
 	if err := c.msgBuilder.finishMsg(&c.writerState.buf); err != nil {
-		panic(errors.AssertionFailedf("unexpected err from buffer: %s", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "unexpected err from buffer"))
 	}
 }
 

diff --git a/pkg/sql/repair.go b/pkg/sql/repair.go
@@ -117,7 +117,7 @@ func (p *planner) UnsafeUpsertDescriptor(
 		existingVersion = mut.GetVersion()
 		marshaled, err := protoutil.Marshal(mut.DescriptorProto())
 		if err != nil {
-			return errors.AssertionFailedf("failed to marshal existing descriptor %v: %v", mut, err)
+			return errors.NewAssertionErrorWithWrappedErrf(err, "failed to marshal existing descriptor %v", mut)
 		}
 		existingStr = hex.EncodeToString(marshaled)
 		previousOwner = mut.GetPrivileges().Owner().Normalized()

diff --git a/pkg/sql/sqlstats/ssmemstorage/ss_mem_storage.go b/pkg/sql/sqlstats/ssmemstorage/ss_mem_storage.go
@@ -688,7 +688,8 @@ func (s *Container) MergeApplicationStatementStats(
 		// Calling Iterate.*Stats() function with a visitor function that does not
 		// return error should not cause any error.
 		panic(
-			errors.AssertionFailedf("unexpected error returned when iterating through application stats: %s", err))
+			errors.NewAssertionErrorWithWrappedErrf(err, "unexpected error returned when iterating through application stats"),
+		)
 	}
 
 	return discardedStats
@@ -720,7 +721,8 @@ func (s *Container) MergeApplicationTransactionStats(
 		// Calling Iterate.*Stats() function with a visitor function that does not
 		// return error should not cause any error.
 		panic(
-			errors.AssertionFailedf("unexpected error returned when iterating through application stats: %s", err))
+			errors.NewAssertionErrorWithWrappedErrf(err, "unexpected error returned when iterating through application stats"),
+		)
 	}
 
 	return discardedStats

diff --git a/pkg/sql/table.go b/pkg/sql/table.go
@@ -284,7 +284,7 @@ func (p *planner) writeTableDescToBatch(
 	}
 
 	if err := catalog.ValidateSelf(tableDesc); err != nil {
-		return errors.AssertionFailedf("table descriptor is not valid: %s\n%v", err, tableDesc)
+		return errors.NewAssertionErrorWithWrappedErrf(err, "table descriptor is not valid\n%v\n", tableDesc)
 	}
 
 	return p.Descriptors().WriteDescToBatch(

diff --git a/pkg/sql/txn_restart_test.go b/pkg/sql/txn_restart_test.go
@@ -364,7 +364,7 @@ func (ta *TxnAborter) statementFilter(
 	ta.mu.Unlock()
 	if shouldAbort {
 		if err := ta.abortTxn(ri.key); err != nil {
-			panic(errors.AssertionFailedf("TxnAborter failed to abort: %s", err))
+			panic(errors.NewAssertionErrorWithWrappedErrf(err, "TxnAborter failed to abort"))
 		}
 	}
 }

diff --git a/pkg/sql/types/types.go b/pkg/sql/types/types.go
@@ -1890,7 +1890,7 @@ func (t *T) Size() (n int) {
 	temp := *t
 	err := temp.downgradeType()
 	if err != nil {
-		panic(errors.AssertionFailedf("error during Size call: %v", err))
+		panic(errors.NewAssertionErrorWithWrappedErrf(err, "error during Size call"))
 	}
 	return temp.InternalType.Size()
 }