better errors: address comments

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
vitessio · Jun 28, 2018 · 4f72c7d · 4f72c7d
1 parent 411fba0
commit 4f72c7d
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 7 deletions.
diff --git a/go/vt/discovery/tablet_stats_cache.go b/go/vt/discovery/tablet_stats_cache.go
@@ -27,6 +27,7 @@ import (
 	"vitess.io/vitess/go/vt/srvtopo"
 	"vitess.io/vitess/go/vt/topo"
 	"vitess.io/vitess/go/vt/topo/topoproto"
+	"vitess.io/vitess/go/vt/topotools"
 )
 
 // TabletStatsCache is a HealthCheckStatsListener that keeps both the
@@ -484,22 +485,22 @@ func (tc *TabletStatsCache) Unsubscribe(i int) error {
 func (tc *TabletStatsCache) GetAggregateStats(target *querypb.Target) (*querypb.AggregateStats, error) {
 	e := tc.getEntry(target.Keyspace, target.Shard, target.TabletType)
 	if e == nil {
-		return nil, topo.NewError(topo.NoNode, target.Keyspace+"/"+target.Shard+"@"+target.TabletType.String())
+		return nil, topo.NewError(topo.NoNode, topotools.TargetIdent(target))
 	}
 
 	e.mu.RLock()
 	defer e.mu.RUnlock()
 	if target.TabletType == topodatapb.TabletType_MASTER {
 		if len(e.aggregates) == 0 {
-			return nil, topo.NewError(topo.NoNode, target.Keyspace+"/"+target.Shard+"@"+target.TabletType.String())
+			return nil, topo.NewError(topo.NoNode, topotools.TargetIdent(target))
 		}
 		for _, agg := range e.aggregates {
 			return agg, nil
 		}
 	}
 	agg, ok := e.aggregates[target.Cell]
 	if !ok {
-		return nil, topo.NewError(topo.NoNode, target.Keyspace+"/"+target.Shard+"@"+target.TabletType.String())
+		return nil, topo.NewError(topo.NoNode, topotools.TargetIdent(target))
 	}
 	return agg, nil
 }
@@ -508,15 +509,23 @@ func (tc *TabletStatsCache) GetAggregateStats(target *querypb.Target) (*querypb.
 func (tc *TabletStatsCache) GetMasterCell(keyspace, shard string) (cell string, err error) {
 	e := tc.getEntry(keyspace, shard, topodatapb.TabletType_MASTER)
 	if e == nil {
-		return "", topo.NewError(topo.NoNode, keyspace+"/"+shard+"@"+topodatapb.TabletType_MASTER.String())
+		return "", topo.NewError(topo.NoNode, topotools.TargetIdent(&querypb.Target{
+			Keyspace:   keyspace,
+			Shard:      shard,
+			TabletType: topodatapb.TabletType_MASTER,
+		}))
 	}
 
 	e.mu.RLock()
 	defer e.mu.RUnlock()
 	for cell := range e.aggregates {
 		return cell, nil
 	}
-	return "", topo.NewError(topo.NoNode, keyspace+"/"+shard+"@"+topodatapb.TabletType_MASTER.String())
+	return "", topo.NewError(topo.NoNode, topotools.TargetIdent(&querypb.Target{
+		Keyspace:   keyspace,
+		Shard:      shard,
+		TabletType: topodatapb.TabletType_MASTER,
+	}))
 }
 
 // Compile-time interface check.

diff --git a/go/vt/topo/errors.go b/go/vt/topo/errors.go
@@ -60,7 +60,7 @@ func NewError(code ErrorCode, node string) error {
 	case NoUpdateNeeded:
 		message = fmt.Sprintf("no update needed: %s", node)
 	default:
-		message = fmt.Sprintf("unknonw code: %s", node)
+		message = fmt.Sprintf("unknown code: %s", node)
 	}
 	return Error{
 		code:    code,

diff --git a/go/vt/vtgate/vtgate_test.go b/go/vt/vtgate/vtgate_test.go
@@ -342,7 +342,7 @@ func TestVTGateExecuteWithKeyspaceShard(t *testing.T) {
 		"select id from none",
 		nil,
 	)
-	want = "vtgate: : target: TestUnsharded.noshard.master, no valid tablet: node doesn't exist: TestUnsharded/noshard@MASTER"
+	want = "vtgate: : target: TestUnsharded.noshard.master, no valid tablet: node doesn't exist: TestUnsharded/noshard (MASTER)"
 	if err == nil || err.Error() != want {
 		t.Errorf("Execute: %v, want %s", err, want)
 	}