From 331857c9e6df11eb46785a89e1142a301c0b3c45 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Wed, 6 Sep 2017 13:11:47 -0400 Subject: [PATCH] linting: apply errcheck part2 --- blockchain/pool.go | 5 ++++- config/toml_test.go | 5 ++++- consensus/common_test.go | 1 - consensus/reactor.go | 4 +++- consensus/reactor_test.go | 4 +++- consensus/replay.go | 19 ++++++++++++++--- consensus/replay_file.go | 18 ++++++++++++---- consensus/replay_test.go | 14 ++++++++++--- consensus/state.go | 26 +++++++++++++++-------- consensus/state_test.go | 28 ++++++++++++++++++------- mempool/mempool.go | 16 ++++++++++----- mempool/mempool_test.go | 6 ++++-- p2p/upnp/probe.go | 7 ++++--- p2p/upnp/upnp.go | 43 +++++++++++++++++++++++++++++++-------- 14 files changed, 147 insertions(+), 49 deletions(-) diff --git a/blockchain/pool.go b/blockchain/pool.go index 47e59711..4016f146 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -311,7 +311,10 @@ func (pool *BlockPool) makeNextRequester() { pool.requesters[nextHeight] = request pool.numPending++ - request.Start() + _, err := request.Start() + if err != nil { + panic(err) + } } func (pool *BlockPool) sendRequest(height int, peerID string) { diff --git a/config/toml_test.go b/config/toml_test.go index d8f372ae..c435ccb3 100644 --- a/config/toml_test.go +++ b/config/toml_test.go @@ -24,7 +24,10 @@ func TestEnsureRoot(t *testing.T) { // setup temp dir for test tmpDir, err := ioutil.TempDir("", "config-test") require.Nil(err) - defer os.RemoveAll(tmpDir) + defer func() { + err := os.RemoveAll(tmpDir) + require.Nil(err) + }() // create root dir EnsureRoot(tmpDir) diff --git a/consensus/common_test.go b/consensus/common_test.go index 50793e65..8528b0a9 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -268,7 +268,6 @@ func newConsensusStateWithConfigAndBlockStore(thisConfig *cfg.Config, state *sm. eventBus.SetLogger(log.TestingLogger().With("module", "events")) eventBus.Start() cs.SetEventBus(eventBus) - return cs } diff --git a/consensus/reactor.go b/consensus/reactor.go index cc8faf4c..44206f50 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -55,7 +55,9 @@ func NewConsensusReactor(consensusState *ConsensusState, fastSync bool) *Consens // OnStart implements BaseService. func (conR *ConsensusReactor) OnStart() error { conR.Logger.Info("ConsensusReactor ", "fastSync", conR.FastSync()) - conR.BaseReactor.OnStart() + if err := conR.BaseReactor.OnStart(); err != nil { + return err + } err := conR.startBroadcastRoutine() if err != nil { diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index 45e94a12..2d27cdd8 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -112,7 +112,9 @@ func TestReactorProposalHeartbeats(t *testing.T) { }, css) // send a tx - css[3].mempool.CheckTx([]byte{1, 2, 3}, nil) + if err := css[3].mempool.CheckTx([]byte{1, 2, 3}, nil); err != nil { + t.Fatal(err) + } // wait till everyone makes the first new block timeoutWaitGroup(t, N, func(wg *sync.WaitGroup, j int) { diff --git a/consensus/replay.go b/consensus/replay.go index 49aa5e7f..4557a7d0 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -100,7 +100,9 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { // and Handshake could reuse ConsensusState if it weren't for this check (since we can crash after writing ENDHEIGHT). gr, found, err := cs.wal.SearchForEndHeight(uint64(csHeight)) if gr != nil { - gr.Close() + if err := gr.Close(); err != nil { + return err + } } if found { return fmt.Errorf("WAL should not contain #ENDHEIGHT %d.", csHeight) @@ -112,6 +114,12 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { cs.Logger.Error("Replay: wal.group.Search returned EOF", "#ENDHEIGHT", csHeight-1) } else if err != nil { return err + } else { + defer func() { + if err := gr.Close(); err != nil { + return + } + }() } if !found { return errors.New(cmn.Fmt("Cannot replay height %d. WAL does not contain #ENDHEIGHT for %d.", csHeight, csHeight-1)) @@ -230,7 +238,9 @@ func (h *Handshaker) ReplayBlocks(appHash []byte, appBlockHeight int, proxyApp p // If appBlockHeight == 0 it means that we are at genesis and hence should send InitChain if appBlockHeight == 0 { validators := types.TM2PB.Validators(h.state.Validators) - proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}) + if err := proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}); err != nil { + return nil, err + } } // First handle edge cases and constraints on the storeBlockHeight @@ -363,7 +373,10 @@ func newMockProxyApp(appHash []byte, abciResponses *sm.ABCIResponses) proxy.AppC abciResponses: abciResponses, }) cli, _ := clientCreator.NewABCIClient() - cli.Start() + _, err := cli.Start() + if err != nil { + panic(err) + } return proxy.NewAppConnConsensus(cli) } diff --git a/consensus/replay_file.go b/consensus/replay_file.go index 6b52b5b0..fcab4d03 100644 --- a/consensus/replay_file.go +++ b/consensus/replay_file.go @@ -65,7 +65,11 @@ func (cs *ConsensusState) ReplayFile(file string, console bool) error { } pb := newPlayback(file, fp, cs, cs.state.Copy()) - defer pb.fp.Close() + defer func() { + if err := pb.fp.Close(); err != nil { + return + } + }() var nextN int // apply N msgs in a row var msg *TimedWALMessage @@ -127,7 +131,9 @@ func (pb *playback) replayReset(count int, newStepCh chan interface{}) error { newCS.SetEventBus(pb.cs.eventBus) newCS.startForReplay() - pb.fp.Close() + if err := pb.fp.Close(); err != nil { + return err + } fp, err := os.OpenFile(pb.fileName, os.O_RDONLY, 0666) if err != nil { return err @@ -220,7 +226,9 @@ func (pb *playback) replayConsoleLoop() int { defer pb.cs.eventBus.Unsubscribe(ctx, subscriber, types.EventQueryNewRoundStep) if len(tokens) == 1 { - pb.replayReset(1, newStepCh) + if err := pb.replayReset(1, newStepCh); err != nil { + panic(err) + } } else { i, err := strconv.Atoi(tokens[1]) if err != nil { @@ -228,7 +236,9 @@ func (pb *playback) replayConsoleLoop() int { } else if i > pb.count { fmt.Printf("argument to back must not be larger than the current count (%d)\n", pb.count) } else { - pb.replayReset(i, newStepCh) + if err := pb.replayReset(i, newStepCh); err != nil { + panic(err) + } } } diff --git a/consensus/replay_test.go b/consensus/replay_test.go index a5d3f088..992201cd 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -411,7 +411,9 @@ func buildAppStateFromChain(proxyApp proxy.AppConns, } validators := types.TM2PB.Validators(state.Validators) - proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}) + if err := proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}); err != nil { + panic(err) + } defer proxyApp.Stop() switch mode { @@ -445,7 +447,9 @@ func buildTMStateFromChain(config *cfg.Config, state *sm.State, chain []*types.B defer proxyApp.Stop() validators := types.TM2PB.Validators(state.Validators) - proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}) + if err := proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}); err != nil { + panic(err) + } var latestAppHash []byte @@ -486,7 +490,11 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) { if !found { return nil, nil, errors.New(cmn.Fmt("WAL does not contain height %d.", 1)) } - defer gr.Close() + defer func() { + if err := gr.Close(); err != nil { + return + } + }() // log.Notice("Build a blockchain by reading from the WAL") diff --git a/consensus/state.go b/consensus/state.go index c65976d7..b4bfa878 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -225,11 +225,14 @@ func (cs *ConsensusState) OnStart() error { } // we need the timeoutRoutine for replay so - // we don't block on the tick chan. + // we don't block on the tick chan. // NOTE: we will get a build up of garbage go routines - // firing on the tockChan until the receiveRoutine is started - // to deal with them (by that point, at most one will be valid) - cs.timeoutTicker.Start() + // firing on the tockChan until the receiveRoutine is started + // to deal with them (by that point, at most one will be valid) + _, err := cs.timeoutTicker.Start() + if err != nil { + return err + } // we may have lost some votes if the process crashed // reload from consensus log to catchup @@ -254,7 +257,10 @@ func (cs *ConsensusState) OnStart() error { // timeoutRoutine: receive requests for timeouts on tickChan and fire timeouts on tockChan // receiveRoutine: serializes processing of proposoals, block parts, votes; coordinates state transitions func (cs *ConsensusState) startRoutines(maxSteps int) { - cs.timeoutTicker.Start() + _, err := cs.timeoutTicker.Start() + if err != nil { + panic(err) + } go cs.receiveRoutine(maxSteps) } @@ -338,12 +344,16 @@ func (cs *ConsensusState) AddProposalBlockPart(height, round int, part *types.Pa // SetProposalAndBlock inputs the proposal and all block parts. func (cs *ConsensusState) SetProposalAndBlock(proposal *types.Proposal, block *types.Block, parts *types.PartSet, peerKey string) error { - cs.SetProposal(proposal, peerKey) + if err := cs.SetProposal(proposal, peerKey); err != nil { + return err + } for i := 0; i < parts.Total(); i++ { part := parts.GetPart(i) - cs.AddProposalBlockPart(proposal.Height, proposal.Round, part, peerKey) + if err := cs.AddProposalBlockPart(proposal.Height, proposal.Round, part, peerKey); err != nil { + return err + } } - return nil // TODO errors + return nil } //------------------------------------------------------------ diff --git a/consensus/state_test.go b/consensus/state_test.go index 49ec1185..ecccafed 100644 --- a/consensus/state_test.go +++ b/consensus/state_test.go @@ -209,7 +209,9 @@ func TestBadProposal(t *testing.T) { } // set the proposal block - cs1.SetProposalAndBlock(proposal, propBlock, propBlockParts, "some peer") + if err := cs1.SetProposalAndBlock(proposal, propBlock, propBlockParts, "some peer"); err != nil { + t.Fatal(err) + } // start the machine startTestRound(cs1, height, round) @@ -478,7 +480,9 @@ func TestLockNoPOL(t *testing.T) { // now we're on a new round and not the proposer // so set the proposal block - cs1.SetProposalAndBlock(prop, propBlock, propBlock.MakePartSet(partSize), "") + if err := cs1.SetProposalAndBlock(prop, propBlock, propBlock.MakePartSet(partSize), ""); err != nil { + t.Fatal(err) + } <-proposalCh <-voteCh // prevote @@ -555,7 +559,9 @@ func TestLockPOLRelock(t *testing.T) { <-timeoutWaitCh //XXX: this isnt guaranteed to get there before the timeoutPropose ... - cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer") + if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer"); err != nil { + t.Fatal(err) + } <-newRoundCh t.Log("### ONTO ROUND 1") @@ -667,7 +673,9 @@ func TestLockPOLUnlock(t *testing.T) { lockedBlockHash := rs.LockedBlock.Hash() //XXX: this isnt guaranteed to get there before the timeoutPropose ... - cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer") + if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer"); err != nil { + t.Fatal(err) + } <-newRoundCh t.Log("#### ONTO ROUND 1") @@ -754,7 +762,9 @@ func TestLockPOLSafety1(t *testing.T) { incrementRound(vs2, vs3, vs4) //XXX: this isnt guaranteed to get there before the timeoutPropose ... - cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer") + if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer"); err != nil { + t.Fatal(err) + } <-newRoundCh t.Log("### ONTO ROUND 1") @@ -866,7 +876,9 @@ func TestLockPOLSafety2(t *testing.T) { startTestRound(cs1, height, 1) <-newRoundCh - cs1.SetProposalAndBlock(prop1, propBlock1, propBlockParts1, "some peer") + if err := cs1.SetProposalAndBlock(prop1, propBlock1, propBlockParts1, "some peer"); err != nil { + t.Fatal(err) + } <-proposalCh <-voteCh // prevote @@ -891,7 +903,9 @@ func TestLockPOLSafety2(t *testing.T) { if err := vs3.SignProposal(config.ChainID, newProp); err != nil { t.Fatal(err) } - cs1.SetProposalAndBlock(newProp, propBlock0, propBlockParts0, "some peer") + if err := cs1.SetProposalAndBlock(newProp, propBlock0, propBlockParts0, "some peer"); err != nil { + t.Fatal(err) + } // Add the pol votes addVotes(cs1, prevotes...) diff --git a/mempool/mempool.go b/mempool/mempool.go index caaa034e..d1475f33 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -189,8 +189,14 @@ func (mem *Mempool) CheckTx(tx types.Tx, cb func(*abci.Response)) (err error) { // WAL if mem.wal != nil { // TODO: Notify administrators when WAL fails - mem.wal.Write([]byte(tx)) - mem.wal.Write([]byte("\n")) + _, err := mem.wal.Write([]byte(tx)) + if err != nil { + return err + } + _, err = mem.wal.Write([]byte("\n")) + if err != nil { + return err + } } // END WAL @@ -332,9 +338,9 @@ func (mem *Mempool) collectTxs(maxTxs int) types.Txs { // NOTE: this should be called *after* block is committed by consensus. // NOTE: unsafe; Lock/Unlock must be managed by caller func (mem *Mempool) Update(height int, txs types.Txs) { - // TODO: check err ? - mem.proxyAppConn.FlushSync() // To flush async resCb calls e.g. from CheckTx - + if err := mem.proxyAppConn.FlushSync(); err != nil { // To flush async resCb calls e.g. from CheckTx + panic(err) + } // First, create a lookup map of txns in new txs. txsMap := make(map[string]struct{}) for _, tx := range txs { diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index 46401e88..a19ca32e 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -49,9 +49,11 @@ func checkTxs(t *testing.T, mempool *Mempool, count int) types.Txs { for i := 0; i < count; i++ { txBytes := make([]byte, 20) txs[i] = txBytes - rand.Read(txBytes) - err := mempool.CheckTx(txBytes, nil) + _, err := rand.Read(txBytes) if err != nil { + t.Error(err) + } + if err := mempool.CheckTx(txBytes, nil); err != nil { t.Fatal("Error after CheckTx: %v", err) } } diff --git a/p2p/upnp/probe.go b/p2p/upnp/probe.go index 74d4d4c5..b3056d4c 100644 --- a/p2p/upnp/probe.go +++ b/p2p/upnp/probe.go @@ -97,11 +97,12 @@ func Probe(logger log.Logger) (caps UPNPCapabilities, err error) { // Deferred cleanup defer func() { - err = nat.DeletePortMapping("tcp", intPort, extPort) - if err != nil { + if err := nat.DeletePortMapping("tcp", intPort, extPort); err != nil { logger.Error(cmn.Fmt("Port mapping delete error: %v", err)) } - listener.Close() + if err := listener.Close(); err != nil { + panic(err) + } }() supportsHairpin := testHairpin(listener, fmt.Sprintf("%v:%v", ext, extPort), logger) diff --git a/p2p/upnp/upnp.go b/p2p/upnp/upnp.go index 7d44d1e3..a90b1004 100644 --- a/p2p/upnp/upnp.go +++ b/p2p/upnp/upnp.go @@ -40,11 +40,14 @@ func Discover() (nat NAT, err error) { return } socket := conn.(*net.UDPConn) - defer socket.Close() + defer func() { + if err := socket.Close(); err != nil { + return + } + }() - err = socket.SetDeadline(time.Now().Add(3 * time.Second)) - if err != nil { - return + if err := socket.SetDeadline(time.Now().Add(3 * time.Second)); err != nil { + return nil, err } st := "InternetGatewayDevice:1" @@ -198,7 +201,11 @@ func getServiceURL(rootURL string) (url, urnDomain string, err error) { if err != nil { return } - defer r.Body.Close() + defer func() { + if err := r.Body.Close(); err != nil { + return + } + }() if r.StatusCode >= 400 { err = errors.New(string(r.StatusCode)) return @@ -296,15 +303,25 @@ func (n *upnpNAT) getExternalIPAddress() (info statusInfo, err error) { var response *http.Response response, err = soapRequest(n.serviceURL, "GetExternalIPAddress", message, n.urnDomain) if response != nil { - defer response.Body.Close() + defer func() { + if err := response.Body.Close(); err != nil { + return + } + }() } if err != nil { return } var envelope Envelope data, err := ioutil.ReadAll(response.Body) + if err != nil { + return + } reader := bytes.NewReader(data) - xml.NewDecoder(reader).Decode(&envelope) + err = xml.NewDecoder(reader).Decode(&envelope) + if err != nil { + return + } info = statusInfo{envelope.Soap.ExternalIP.IPAddress} @@ -339,7 +356,11 @@ func (n *upnpNAT) AddPortMapping(protocol string, externalPort, internalPort int var response *http.Response response, err = soapRequest(n.serviceURL, "AddPortMapping", message, n.urnDomain) if response != nil { - defer response.Body.Close() + defer func() { + if err := response.Body.Close(); err != nil { + return + } + }() } if err != nil { return @@ -365,7 +386,11 @@ func (n *upnpNAT) DeletePortMapping(protocol string, externalPort, internalPort var response *http.Response response, err = soapRequest(n.serviceURL, "DeletePortMapping", message, n.urnDomain) if response != nil { - defer response.Body.Close() + defer func() { + if err := response.Body.Close(); err != nil { + return + } + }() } if err != nil { return