util/pretty: mitigate stack overflows of Pretty

pkg/BUILD.bazel

@@ -2419,6 +2419,7 @@ GO_TARGETS = [
     "//pkg/util/sdnotify:sdnotify_test",
     "//pkg/util/search:search",
     "//pkg/util/search:search_test",
+    "//pkg/util/sentryutil:sentryutil",
     "//pkg/util/shuffle:shuffle",
     "//pkg/util/shuffle:shuffle_test",
     "//pkg/util/slidingwindow:slidingwindow",

pkg/ccl/kvccl/kvfollowerreadsccl/BUILD.bazel

@@ -74,7 +74,6 @@ go_test(
         "//pkg/testutils/sqlutils",
         "//pkg/testutils/testcluster",
         "//pkg/util",
-        "//pkg/util/errorutil",
         "//pkg/util/hlc",
         "//pkg/util/leaktest",
         "//pkg/util/log",

pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go

@@ -40,7 +40,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
 	"github.com/cockroachdb/cockroach/pkg/util"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -497,15 +496,15 @@ func (s mockNodeStore) GetNodeDescriptor(id roachpb.NodeID) (*roachpb.NodeDescri
 			return desc, nil
 		}
 	}
-	return nil, errorutil.NewNodeNotFoundError(id)
+	return nil, kvpb.NewNodeDescNotFoundError(id)
 }
 
 func (s mockNodeStore) GetNodeDescriptorCount() int {
 	return len(s)
 }
 
 func (s mockNodeStore) GetStoreDescriptor(id roachpb.StoreID) (*roachpb.StoreDescriptor, error) {
-	return nil, errorutil.NewStoreNotFoundError(id)
+	return nil, kvpb.NewStoreDescNotFoundError(id)
 }
 
 // TestOracle tests the Oracle exposed by this package.

pkg/cli/sqlfmt.go

@@ -76,7 +76,11 @@ func runSQLFmt(cmd *cobra.Command, args []string) error {
 	}
 
 	for i := range sl {
-		fmt.Print(cfg.Pretty(sl[i].AST))
+		p, err := cfg.Pretty(sl[i].AST)
+		if err != nil {
+			return err
+		}
+		fmt.Print(p)
 		if len(sl) > 1 {
 			fmt.Print(";")
 		}

pkg/cmd/reduce/reduce/reducesql/reducesql.go

@@ -427,6 +427,7 @@ func collectASTs(stmts statements.Statements) []tree.NodeFormatter {
 
 func joinASTs(stmts []tree.NodeFormatter) string {
 	var sb strings.Builder
+	var fmtCtx *tree.FmtCtx
 	for i, stmt := range stmts {
 		if i > 0 {
 			sb.WriteString("\n\n")
@@ -438,7 +439,16 @@ func joinASTs(stmts []tree.NodeFormatter) string {
 			UseTabs:   false,
 			Simplify:  true,
 		}
-		sb.WriteString(cfg.Pretty(stmt))
+		p, err := cfg.Pretty(stmt)
+		if err != nil {
+			// Use simple printing if pretty-printing fails.
+			if fmtCtx == nil {
+				fmtCtx = tree.NewFmtCtx(tree.FmtParsable)
+			}
+			stmt.Format(fmtCtx)
+			p = fmtCtx.CloseAndGetString()
+		}
+		sb.WriteString(p)
 		sb.WriteString(";")
 	}
 	return sb.String()

pkg/gossip/gossip.go

@@ -877,7 +877,7 @@ func (g *Gossip) getNodeDescriptor(
 		return nodeDescriptor, nil
 	}
 
-	return nil, errorutil.NewNodeNotFoundError(nodeID)
+	return nil, kvpb.NewNodeDescNotFoundError(nodeID)
 }
 
 // getNodeIDAddress looks up the address of the node by ID. The method accepts a

pkg/internal/sqlsmith/setup_test.go

@@ -115,7 +115,10 @@ func TestGenerateParse(t *testing.T) {
 		if err != nil {
 			t.Fatalf("%v: %v", stmt, err)
 		}
-		stmt = sqlsmith.TestingPrettyCfg.Pretty(parsed.AST)
+		stmt, err = sqlsmith.TestingPrettyCfg.Pretty(parsed.AST)
+		if err != nil {
+			t.Fatal(err)
+		}
 		fmt.Print("STMT: ", i, "\n", stmt, ";\n\n")
 		if *flagExec {
 			db.Exec(t, `SET statement_timeout = '9s'`)

pkg/internal/sqlsmith/sqlsmith.go

@@ -190,7 +190,12 @@ func (s *Smither) Generate() string {
 			continue
 		}
 		i = 0
-		return prettyCfg.Pretty(stmt)
+		p, err := prettyCfg.Pretty(stmt)
+		if err != nil {
+			// Use simple printing if pretty-printing fails.
+			p = tree.AsStringWithFlags(stmt, tree.FmtParsable)
+		}
+		return p
 	}
 }
 

pkg/kv/kvclient/kvcoord/BUILD.bazel

@@ -206,7 +206,6 @@ go_test(
         "//pkg/util/caller",
         "//pkg/util/ctxgroup",
         "//pkg/util/encoding",
-        "//pkg/util/errorutil",
         "//pkg/util/grpcutil",
         "//pkg/util/hlc",
         "//pkg/util/leaktest",

pkg/kv/kvclient/kvcoord/replica_slice_test.go

@@ -17,9 +17,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/util"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/shuffle"
@@ -39,7 +39,7 @@ func (ns *mockNodeStore) GetNodeDescriptor(nodeID roachpb.NodeID) (*roachpb.Node
 			return &nd, nil
 		}
 	}
-	return nil, errorutil.NewNodeNotFoundError(nodeID)
+	return nil, kvpb.NewNodeDescNotFoundError(nodeID)
 }
 
 // GetNodeDescriptorCount is part of the NodeDescStore interface.
@@ -51,7 +51,7 @@ func (ns *mockNodeStore) GetNodeDescriptorCount() int {
 func (ns *mockNodeStore) GetStoreDescriptor(
 	storeID roachpb.StoreID,
 ) (*roachpb.StoreDescriptor, error) {
-	return nil, errorutil.NewStoreNotFoundError(storeID)
+	return nil, kvpb.NewStoreDescNotFoundError(storeID)
 }
 
 func TestNewReplicaSlice(t *testing.T) {

pkg/kv/kvclient/kvtenant/BUILD.bazel

@@ -33,7 +33,6 @@ go_library(
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
         "//pkg/ts/tspb",
-        "//pkg/util/errorutil",
         "//pkg/util/grpcutil",
         "//pkg/util/hlc",
         "//pkg/util/log",

pkg/kv/kvclient/kvtenant/connector.go

@@ -39,7 +39,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/ts/tspb"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/grpcutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -517,7 +516,7 @@ func (c *connector) GetNodeDescriptor(nodeID roachpb.NodeID) (*roachpb.NodeDescr
 	defer c.mu.RUnlock()
 	desc, ok := c.mu.nodeDescs[nodeID]
 	if !ok {
-		return nil, errorutil.NewNodeNotFoundError(nodeID)
+		return nil, kvpb.NewNodeDescNotFoundError(nodeID)
 	}
 	return desc, nil
 }
@@ -535,7 +534,7 @@ func (c *connector) GetStoreDescriptor(storeID roachpb.StoreID) (*roachpb.StoreD
 	defer c.mu.RUnlock()
 	desc, ok := c.mu.storeDescs[storeID]
 	if !ok {
-		return nil, errorutil.NewStoreNotFoundError(storeID)
+		return nil, kvpb.NewStoreDescNotFoundError(storeID)
 	}
 	return desc, nil
 }

pkg/kv/kvclient/kvtenant/connector_test.go

@@ -321,7 +321,7 @@ func TestConnectorGossipSubscription(t *testing.T) {
 	require.NoError(t, err)
 	desc, err = c.GetNodeDescriptor(3)
 	require.Nil(t, desc)
-	require.Regexp(t, "unable to look up descriptor for n3", err)
+	require.Regexp(t, "node descriptor with node ID 3 was not found", err)
 
 	// Test GetStoreDescriptor.
 	storeID1 := roachpb.StoreID(1)
@@ -340,7 +340,7 @@ func TestConnectorGossipSubscription(t *testing.T) {
 	require.Equal(t, store2, storeDesc)
 	storeDesc, err = c.GetStoreDescriptor(3)
 	require.Nil(t, storeDesc)
-	require.Regexp(t, "unable to look up descriptor for store ID 3", err)
+	require.Regexp(t, "store descriptor with store ID 3 was not found", err)
 
 	// Return updated GossipSubscription response.
 	node1Up := &roachpb.NodeDescriptor{NodeID: 1, Address: util.MakeUnresolvedAddr("tcp", "1.2.3.4")}
@@ -567,7 +567,7 @@ func TestConnectorRetriesUnreachable(t *testing.T) {
 	require.NoError(t, err)
 	desc, err = c.GetNodeDescriptor(3)
 	require.Nil(t, desc)
-	require.Regexp(t, "unable to look up descriptor for n3", err)
+	require.Regexp(t, "node descriptor with node ID 3 was not found", err)
 }
 
 // TestConnectorRetriesError tests that connector iterates over each of

pkg/kv/kvpb/BUILD.bazel

@@ -33,6 +33,7 @@ go_library(
         "//pkg/util/admission/admissionpb",
         "//pkg/util/buildutil",
         "//pkg/util/caller",
+        "//pkg/util/errorutil",
         "//pkg/util/hlc",
         "//pkg/util/humanizeutil",
         "//pkg/util/log",

pkg/kv/kvpb/errors.go

@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/caller"
+	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -1627,10 +1628,49 @@ func (e *MissingRecordError) SafeFormatError(p errors.Printer) (next error) {
 	return nil
 }
 
+// DescNotFoundError is reported when a descriptor is missing.
+type DescNotFoundError struct {
+	id      int32
+	isStore bool
+}
+
+// NewStoreDescNotFoundError initializes a new DescNotFoundError for a missing
+// store descriptor.
+func NewStoreDescNotFoundError(storeID roachpb.StoreID) *DescNotFoundError {
+	return &DescNotFoundError{
+		id:      int32(storeID),
+		isStore: true,
+	}
+}
+
+// NewNodeDescNotFoundError initializes a new DescNotFoundError for a missing
+// node descriptor.
+func NewNodeDescNotFoundError(nodeID roachpb.NodeID) *DescNotFoundError {
+	return &DescNotFoundError{
+		id:      int32(nodeID),
+		isStore: false,
+	}
+}
+
+func (e *DescNotFoundError) Error() string {
+	return redact.Sprint(e).StripMarkers()
+}
+
+func (e *DescNotFoundError) SafeFormatError(p errors.Printer) (next error) {
+	s := redact.SafeString("node")
+	if e.isStore {
+		s = "store"
+	}
+	p.Printf("%s descriptor with %s ID %d was not found", s, s, e.id)
+	return nil
+}
+
 func init() {
 	errors.RegisterLeafDecoder(errors.GetTypeKey((*MissingRecordError)(nil)), func(_ context.Context, _ string, _ []string, _ proto.Message) error {
 		return &MissingRecordError{}
 	})
+	errorutilpath := reflect.TypeOf(errorutil.TempSentinel{}).PkgPath()
+	errors.RegisterTypeMigration(errorutilpath, "*errorutil.descriptorNotFound", &DescNotFoundError{})
 }
 
 var _ errors.SafeFormatter = &MissingRecordError{}

pkg/kv/kvserver/BUILD.bazel

@@ -480,7 +480,6 @@ go_test(
         "//pkg/util/circuit",
         "//pkg/util/ctxgroup",
         "//pkg/util/encoding",
-        "//pkg/util/errorutil",
         "//pkg/util/future",
         "//pkg/util/grunning",
         "//pkg/util/hlc",

pkg/kv/kvserver/store_test.go

@@ -60,7 +60,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/gossiputil"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -155,7 +154,7 @@ func (m mockNodeStore) GetNodeDescriptorCount() int {
 }
 
 func (m mockNodeStore) GetStoreDescriptor(id roachpb.StoreID) (*roachpb.StoreDescriptor, error) {
-	return nil, errorutil.NewStoreNotFoundError(id)
+	return nil, kvpb.NewStoreDescNotFoundError(id)
 }
 
 type dummyFirstRangeProvider struct {

pkg/sql/BUILD.bazel

@@ -558,12 +558,14 @@ go_library(
         "//pkg/util/metric",
         "//pkg/util/mon",
         "//pkg/util/optional",
+        "//pkg/util/pretty",
         "//pkg/util/protoutil",
         "//pkg/util/quotapool",
         "//pkg/util/randutil",
         "//pkg/util/rangedesc",
         "//pkg/util/retry",
         "//pkg/util/ring",
+        "//pkg/util/sentryutil",
         "//pkg/util/startup",
         "//pkg/util/stop",
         "//pkg/util/syncutil",

pkg/sql/conn_executor.go

@@ -80,6 +80,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/log/severity"
 	"github.com/cockroachdb/cockroach/pkg/util/metric"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
+	"github.com/cockroachdb/cockroach/pkg/util/sentryutil"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -3792,7 +3793,7 @@ func (ex *connExecutor) txnStateTransitionsApplyWrapper(
 				errors.Safe(advInfo.txnEvent.eventType.String()),
 				res.Err())
 			log.Errorf(ex.Ctx(), "%v", err)
-			errorutil.SendReport(ex.Ctx(), &ex.server.cfg.Settings.SV, err)
+			sentryutil.SendReport(ex.Ctx(), &ex.server.cfg.Settings.SV, err)
 			return advanceInfo{}, err
 		}
 

pkg/sql/crdb_internal.go

@@ -4530,7 +4530,7 @@ CREATE TABLE crdb_internal.ranges_no_leases (
 				replicaLocalityDatum := tree.DNull
 				nodeDesc, err := p.ExecCfg().NodeDescs.GetNodeDescriptor(replica.NodeID)
 				if err != nil {
-					if !errorutil.IsDescriptorNotFoundError(err) {
+					if !errors.Is(err, &kvpb.DescNotFoundError{}) {
 						return nil, err
 					}
 				} else {

pkg/sql/crdb_internal_ranges_deprecated.go

@@ -15,6 +15,7 @@ import (
 	"sort"
 
 	"github.com/cockroachdb/cockroach/pkg/keys"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
@@ -24,8 +25,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
+	"github.com/cockroachdb/errors"
 )
 
 // crdbInternalRangesViewDEPRECATED is the pre-v23.1
@@ -200,7 +201,7 @@ CREATE TABLE crdb_internal.ranges_no_leases (
 				replicaLocalityDatum := tree.DNull
 				nodeDesc, err := p.ExecCfg().NodeDescs.GetNodeDescriptor(replica.NodeID)
 				if err != nil {
-					if !errorutil.IsDescriptorNotFoundError(err) {
+					if !errors.Is(err, &kvpb.DescNotFoundError{}) {
 						return nil, err
 					}
 				} else {