Fix more errcheck lints

Signed-off-by: Andrew Mason <amason@slack-corp.com>

.golangci.yml

@@ -37,7 +37,7 @@ issues:
     ### BEGIN: errcheck exclusion rules. Each rule should be considered
     #   a TODO for removal after adding error checks to that package/file/etc,
     #   except where otherwise noted.
-    - path: '^go/cmd/'
+    - path: '^go/cmd/(vtcombo|vtgateclienttest|vtorc)/'
       linters:
       - errcheck
     - path: '^go/mysql/'
@@ -58,15 +58,6 @@ issues:
     - path: '^go/vt/automation/'
       linters:
       - errcheck
-    - path: '^go/vt/binlog/binlogplayertest/'
-      linters:
-      - errcheck
-    - path: '^go/vt/discovery/.*_test.go'
-      linters:
-      - errcheck
-    - path: '^go/vt/key/key_test.go'
-      linters:
-      - errcheck
     - path: '^go/vt/mysqlctl/'
       linters:
       - errcheck
@@ -87,28 +78,16 @@ issues:
     - path: '^go/vt/servenv/'
       linters:
       - errcheck
-    - path: '^go/vt/sqlparser/.*_test.go'
-      linters:
-      - errcheck
     # This code is autogenerated and should be permanently excluded.
     - path: '^go/vt/sqlparser/goyacc'
       linters:
       - errcheck
-    - path: '^go/vt/srvtopo/.*_test.go'
-      linters:
-      - errcheck
     - path: '^go/vt/throttler/.*_test.go'
       linters:
       - errcheck
-    - path: '^go/vt/tlstest/'
-      linters:
-      - errcheck
     - path: '^go/vt/topo/.*/*._test.go'
       linters:
       - errcheck
-    - path: '^go/vt/vitessdriver/.*_test.go'
-      linters:
-      - errcheck
     - path: '^go/vt/vtcombo/'
       linters:
       - errcheck
@@ -121,22 +100,16 @@ issues:
     - path: '^go/vt/vtctl/grpcvtctlserver/'
       linters:
       - errcheck
-    - path: '^go/vt/vtctl/grpcvtctldserver/testutil/'
-      linters:
-      - errcheck
     - path: '^go/vt/vtctld/(schema|.*_test).go'
       linters:
       - errcheck
-    - path: '^go/vt/vterrors/'
-      linters:
-      - errcheck
     - path: '^go/vt/vtexplain/'
       linters:
       - errcheck
     - path: '^go/vt/vtgate/.*_test.go'
       linters:
       - errcheck
-    - path: '^go/vt/vttablet/(customrule|filelogger|grpctmserver|onlineddl|sandboxconn|tabletconntest|tabletserver)/'
+    - path: '^go/vt/vttablet/(customrule|filelogger|grpctmserver|onlineddl|sandboxconn|tabletserver)/'
       linters:
       - errcheck
     - path: '^go/vt/vttablet/tabletmanager/vreplication'
@@ -148,9 +121,6 @@ issues:
     - path: '^go/vt/vttest'
       linters:
       - errcheck
-    - path: '^go/vt/withddl/.*_test.go'
-      linters:
-      - errcheck
     - path: '^go/vt/worker'
       linters:
       - errcheck

go/cmd/mysqlctld/mysqlctld.go

@@ -124,7 +124,9 @@ func main() {
 	servenv.OnTermSync(func() {
 		log.Infof("mysqlctl received SIGTERM, shutting down mysqld first")
 		ctx := context.Background()
-		mysqld.Shutdown(ctx, cnf, true)
+		if err := mysqld.Shutdown(ctx, cnf, true); err != nil {
+			log.Errorf("failed to shutdown mysqld: %v", err)
+		}
 	})
 
 	// Start RPC server and wait for SIGTERM.

go/cmd/vtbackup/vtbackup.go

@@ -235,7 +235,9 @@ func takeBackup(ctx context.Context, topoServer *topo.Server, backupStorage back
 		// skip shutdown just because we timed out waiting for other things.
 		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 		defer cancel()
-		mysqld.Shutdown(ctx, mycnf, false)
+		if err := mysqld.Shutdown(ctx, mycnf, false); err != nil {
+			log.Errorf("failed to shutdown mysqld: %v", err)
+		}
 	}()
 
 	extraEnv := map[string]string{

go/cmd/vtclient/vtclient_test.go

@@ -77,7 +77,7 @@ func TestVtclient(t *testing.T) {
 	if err := cluster.Setup(); err != nil {
 		t.Fatalf("InitSchemas failed: %v", err)
 	}
-	defer cluster.TearDown()
+	defer cluster.TearDown() // nolint:errcheck
 
 	vtgateAddr := fmt.Sprintf("localhost:%v", cluster.Env.PortForProtocol("vtcombo", "grpc"))
 	queries := []struct {

go/cmd/vtctl/vtctl.go

@@ -85,7 +85,7 @@ func main() {
 	startMsg := fmt.Sprintf("USER=%v SUDO_USER=%v %v", os.Getenv("USER"), os.Getenv("SUDO_USER"), strings.Join(os.Args, " "))
 
 	if syslogger, err := syslog.New(syslog.LOG_INFO, "vtctl "); err == nil {
-		syslogger.Info(startMsg)
+		syslogger.Info(startMsg) // nolint:errcheck
 	} else {
 		log.Warningf("cannot connect to syslog: %v", err)
 	}

go/streamlog/streamlog_flaky_test.go

@@ -235,7 +235,9 @@ func TestFile(t *testing.T) {
 
 	// Send the rotate signal which should reopen the original file path
 	// for new logs to go to
-	_ = syscall.Kill(syscall.Getpid(), syscall.SIGUSR2)
+	if err := syscall.Kill(syscall.Getpid(), syscall.SIGUSR2); err != nil {
+		t.Logf("failed to send streamlog rotate signal: %v", err)
+	}
 	time.Sleep(10 * time.Millisecond)
 
 	logger.Send(&logMessage{"test 4"})

go/vt/binlog/binlogplayertest/player.go

@@ -112,7 +112,9 @@ func (fake *FakeBinlogStreamer) StreamKeyRange(ctx context.Context, position str
 		!proto.Equal(charset, testKeyRangeRequest.Charset) {
 		fake.t.Errorf("wrong StreamKeyRange parameter, got %+v want %+v", req, testKeyRangeRequest)
 	}
-	callback(testBinlogTransaction)
+	if err := callback(testBinlogTransaction); err != nil {
+		fake.t.Logf("StreamKeyRange callback failed: %v", err)
+	}
 	return nil
 }
 
@@ -178,7 +180,9 @@ func (fake *FakeBinlogStreamer) StreamTables(ctx context.Context, position strin
 		!proto.Equal(charset, testTablesRequest.Charset) {
 		fake.t.Errorf("wrong StreamTables parameter, got %+v want %+v", req, testTablesRequest)
 	}
-	callback(testBinlogTransaction)
+	if err := callback(testBinlogTransaction); err != nil {
+		fake.t.Logf("StreamTables callback failed: %v", err)
+	}
 	return nil
 }
 

go/vt/discovery/healthcheck_test.go

@@ -1062,7 +1062,7 @@ func TestCellAliases(t *testing.T) {
 		Cells: []string{"cell1", "cell2"},
 	}
 	assert.Nil(t, ts.CreateCellsAlias(context.Background(), "region1", cellsAlias), "failed to create cell alias")
-	defer ts.DeleteCellsAlias(context.Background(), "region1")
+	defer deleteCellsAlias(t, ts, "region1")
 
 	// add a tablet as replica in diff cell, same region
 	tablet := createTestTablet(1, "cell2", "host2")
@@ -1311,3 +1311,9 @@ func createTestTablet(uid uint32, cell, host string) *topodatapb.Tablet {
 }
 
 var mustMatch = utils.MustMatchFn(".Conn" /* ignored fields*/)
+
+func deleteCellsAlias(t *testing.T, ts *topo.Server, alias string) {
+	if err := ts.DeleteCellsAlias(context.Background(), alias); err != nil {
+		t.Logf("DeleteCellsAlias(%s) failed: %v", alias, err)
+	}
+}

go/vt/discovery/legacy_tablet_stats_cache_test.go

@@ -41,7 +41,7 @@ func TestLegacyTabletStatsCache(t *testing.T) {
 		log.Errorf("creating cellsAlias \"region1\" failed: %v", err)
 	}
 
-	defer ts.DeleteCellsAlias(context.Background(), "region1")
+	defer deleteCellsAlias(t, ts, "region1")
 
 	cellsAlias = &topodatapb.CellsAlias{
 		Cells: []string{"cell2"},
@@ -51,7 +51,7 @@ func TestLegacyTabletStatsCache(t *testing.T) {
 		log.Errorf("creating cellsAlias \"region2\" failed: %v", err)
 	}
 
-	defer ts.DeleteCellsAlias(context.Background(), "region2")
+	defer deleteCellsAlias(t, ts, "region2")
 
 	// We want to unit test LegacyTabletStatsCache without a full-blown
 	// LegacyHealthCheck object, so we can't call NewLegacyTabletStatsCache.

go/vt/key/key_test.go

@@ -697,7 +697,9 @@ func BenchmarkKeyRangesOverlap(b *testing.B) {
 	}
 
 	for i := 0; i < b.N; i++ {
-		KeyRangesOverlap(kr1, kr2)
+		if _, err := KeyRangesOverlap(kr1, kr2); err != nil {
+			b.Fatal(err)
+		}
 	}
 }
 

go/vt/sqlparser/parse_test.go

@@ -2055,7 +2055,7 @@ func TestValid(t *testing.T) {
 			// There's no way automated way to verify that a node calls
 			// all its children. But we can examine code coverage and
 			// ensure that all walkSubtree functions were called.
-			Walk(func(node SQLNode) (bool, error) {
+			_ = Walk(func(node SQLNode) (bool, error) {
 				return true, nil
 			}, tree)
 		})

go/vt/srvtopo/resilient_server_test.go

@@ -66,7 +66,8 @@ func TestGetSrvKeyspace(t *testing.T) {
 		ShardingColumnName: "id",
 		ShardingColumnType: topodatapb.KeyspaceIdType_UINT64,
 	}
-	ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
+	err = ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
+	require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want)
 
 	// wait until we get the right value
 	var got *topodatapb.SrvKeyspace
@@ -126,7 +127,8 @@ func TestGetSrvKeyspace(t *testing.T) {
 		ShardingColumnType: topodatapb.KeyspaceIdType_UINT64,
 	}
 
-	ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
+	err = ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
+	require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want)
 	expiry = time.Now().Add(5 * time.Second)
 	updateTime := time.Now()
 	for {
@@ -250,7 +252,8 @@ func TestGetSrvKeyspace(t *testing.T) {
 		ShardingColumnName: "id3",
 		ShardingColumnType: topodatapb.KeyspaceIdType_UINT64,
 	}
-	ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
+	err = ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
+	require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want)
 	expiry = time.Now().Add(5 * time.Second)
 	for {
 		got, err = rs.GetSrvKeyspace(context.Background(), "test_cell", "test_ks")
@@ -382,7 +385,8 @@ func TestGetSrvKeyspaceCreated(t *testing.T) {
 		ShardingColumnName: "id",
 		ShardingColumnType: topodatapb.KeyspaceIdType_UINT64,
 	}
-	ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
+	err := ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
+	require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want)
 
 	// Wait until we get the right value.
 	expiry := time.Now().Add(5 * time.Second)
@@ -506,8 +510,11 @@ func TestGetSrvKeyspaceNames(t *testing.T) {
 		ShardingColumnName: "id",
 		ShardingColumnType: topodatapb.KeyspaceIdType_UINT64,
 	}
-	ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
+	err := ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want)
-	ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks2", want)
+	require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want)
+
+	err = ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks2", want)
+	require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks2, %s) failed", want)
 
 	ctx := context.Background()
 	names, err := rs.GetSrvKeyspaceNames(ctx, "test_cell", false)

go/vt/tlstest/tlstest_test.go

@@ -104,7 +104,7 @@ func testClientServer(t *testing.T, combineCerts bool) {
 		defer wg.Done()
 		clientConn, clientErr := tls.DialWithDialer(dialer, "tcp", addr, clientConfig)
 		if clientErr == nil {
-			clientConn.Write([]byte{42})
+			_, _ = clientConn.Write([]byte{42})
 			clientConn.Close()
 		}
 	}()

go/vt/vttablet/tabletconntest/fakequeryservice.go

@@ -644,7 +644,9 @@ func (f *FakeQueryService) MessageStream(ctx context.Context, target *querypb.Ta
 	if name != MessageName {
 		f.t.Errorf("name: %s, want %s", name, MessageName)
 	}
-	callback(MessageStreamResult)
+	if err := callback(MessageStreamResult); err != nil {
+		f.t.Logf("MessageStream callback failed: %v", err)
+	}
 	return nil
 }
 
@@ -700,7 +702,9 @@ func (f *FakeQueryService) StreamHealth(ctx context.Context, callback func(*quer
 	if shr == nil {
 		shr = TestStreamHealthStreamHealthResponse
 	}
-	callback(shr)
+	if err := callback(shr); err != nil {
+		f.t.Logf("StreamHealth callback failed: %v", err)
+	}
 	return nil
 }
 

go/vt/withddl/withddl_test.go

@@ -42,7 +42,7 @@ func TestExec(t *testing.T) {
 	defer conn.Close()
 	_, err = conn.ExecuteFetch("create database t", 10000, true)
 	require.NoError(t, err)
-	defer conn.ExecuteFetch("drop database t", 10000, true)
+	defer conn.ExecuteFetch("drop database t", 10000, true) // nolint:errcheck
 
 	testcases := []struct {
 		name    string
@@ -207,7 +207,7 @@ func TestExecIgnore(t *testing.T) {
 	defer conn.Close()
 	_, err = conn.ExecuteFetch("create database t", 10000, true)
 	require.NoError(t, err)
-	defer conn.ExecuteFetch("drop database t", 10000, true)
+	defer conn.ExecuteFetch("drop database t", 10000, true) // nolint:errcheck
 
 	withdb := connParams
 	withdb.DbName = "t"
@@ -225,7 +225,7 @@ func TestExecIgnore(t *testing.T) {
 	assert.Error(t, err)
 
 	_, _ = execconn.ExecuteFetch("create table a(id int, primary key(id))", 10000, false)
-	defer execconn.ExecuteFetch("drop table a", 10000, false)
+	defer execconn.ExecuteFetch("drop table a", 10000, false) // nolint:errcheck
 	_, _ = execconn.ExecuteFetch("insert into a values(1)", 10000, false)
 	qr, err = wd.ExecIgnore(ctx, "select * from a", execconn.ExecuteFetch)
 	require.NoError(t, err)