From e4fef2574e045cddfd8b9c8c8b8b381f4ad600ac Mon Sep 17 00:00:00 2001 From: Andrew Mason Date: Sat, 29 May 2021 20:01:55 -0400 Subject: [PATCH] Fix more errcheck lints Signed-off-by: Andrew Mason --- .golangci.yml | 34 ++----------------- go/cmd/mysqlctld/mysqlctld.go | 4 ++- go/cmd/vtbackup/vtbackup.go | 4 ++- go/cmd/vtclient/vtclient_test.go | 2 +- go/cmd/vtctl/vtctl.go | 2 +- go/streamlog/streamlog_flaky_test.go | 4 ++- go/vt/binlog/binlogplayertest/player.go | 8 +++-- go/vt/discovery/healthcheck_test.go | 8 ++++- .../legacy_tablet_stats_cache_test.go | 4 +-- go/vt/key/key_test.go | 4 ++- go/vt/sqlparser/parse_test.go | 2 +- go/vt/srvtopo/resilient_server_test.go | 19 +++++++---- go/vt/tlstest/tlstest_test.go | 2 +- .../tabletconntest/fakequeryservice.go | 8 +++-- go/vt/withddl/withddl_test.go | 6 ++-- 15 files changed, 55 insertions(+), 56 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d5030da6bf..66b17739d7 100644 --- a/.golangci.yml +++ b/.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 diff --git a/go/cmd/mysqlctld/mysqlctld.go b/go/cmd/mysqlctld/mysqlctld.go index 2b9a7f84f3..aabca15b79 100644 --- a/go/cmd/mysqlctld/mysqlctld.go +++ b/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. diff --git a/go/cmd/vtbackup/vtbackup.go b/go/cmd/vtbackup/vtbackup.go index 6e1e0f44b0..42e6c1b7e4 100644 --- a/go/cmd/vtbackup/vtbackup.go +++ b/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{ diff --git a/go/cmd/vtclient/vtclient_test.go b/go/cmd/vtclient/vtclient_test.go index 4c68892548..4dbe9010ef 100644 --- a/go/cmd/vtclient/vtclient_test.go +++ b/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 { diff --git a/go/cmd/vtctl/vtctl.go b/go/cmd/vtctl/vtctl.go index 4444d23a37..844e8f8ec0 100644 --- a/go/cmd/vtctl/vtctl.go +++ b/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) } diff --git a/go/streamlog/streamlog_flaky_test.go b/go/streamlog/streamlog_flaky_test.go index 8c67b57abb..8b6e40b11b 100644 --- a/go/streamlog/streamlog_flaky_test.go +++ b/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"}) diff --git a/go/vt/binlog/binlogplayertest/player.go b/go/vt/binlog/binlogplayertest/player.go index bfbd234f93..e3468f9291 100644 --- a/go/vt/binlog/binlogplayertest/player.go +++ b/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 } diff --git a/go/vt/discovery/healthcheck_test.go b/go/vt/discovery/healthcheck_test.go index d1052eb6b3..d2f6d25eaf 100644 --- a/go/vt/discovery/healthcheck_test.go +++ b/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) + } +} diff --git a/go/vt/discovery/legacy_tablet_stats_cache_test.go b/go/vt/discovery/legacy_tablet_stats_cache_test.go index c88b6c0bd8..eabbb38ffa 100644 --- a/go/vt/discovery/legacy_tablet_stats_cache_test.go +++ b/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. diff --git a/go/vt/key/key_test.go b/go/vt/key/key_test.go index d8d98e92f2..49babeff3f 100644 --- a/go/vt/key/key_test.go +++ b/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) + } } } diff --git a/go/vt/sqlparser/parse_test.go b/go/vt/sqlparser/parse_test.go index 474487cc56..facffd39a9 100644 --- a/go/vt/sqlparser/parse_test.go +++ b/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) }) diff --git a/go/vt/srvtopo/resilient_server_test.go b/go/vt/srvtopo/resilient_server_test.go index caffc75a52..796dcffc32 100644 --- a/go/vt/srvtopo/resilient_server_test.go +++ b/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) - ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks2", want) + err := ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", 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) diff --git a/go/vt/tlstest/tlstest_test.go b/go/vt/tlstest/tlstest_test.go index 02e6d83915..de34b077af 100644 --- a/go/vt/tlstest/tlstest_test.go +++ b/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() } }() diff --git a/go/vt/vttablet/tabletconntest/fakequeryservice.go b/go/vt/vttablet/tabletconntest/fakequeryservice.go index bb63a6fcde..847d7b3e0a 100644 --- a/go/vt/vttablet/tabletconntest/fakequeryservice.go +++ b/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 } diff --git a/go/vt/withddl/withddl_test.go b/go/vt/withddl/withddl_test.go index dd44a325d7..5d62472176 100644 --- a/go/vt/withddl/withddl_test.go +++ b/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)