From 97019b11bd9ae0f9e622571ce813c190152a5ebd Mon Sep 17 00:00:00 2001 From: Samuel Marks <807580+SamuelMarks@users.noreply.github.com> Date: Sun, 18 Aug 2019 12:21:42 +1000 Subject: [PATCH] a*.go: Handling unhandled errors --- accounts/keystore/account_cache.go | 10 +++++++-- accounts/keystore/account_cache_test.go | 26 +++++++++++++++++++----- consensus/ethash/algorithm.go | 8 ++++++-- consensus/ethash/algorithm_test.go | 12 +++++++++-- core/rawdb/accessors_chain_test.go | 4 +++- core/rawdb/accessors_indexes.go | 4 ++-- core/rawdb/accessors_indexes_test.go | 4 +++- eth/api.go | 18 ++++++++++++++--- eth/api_backend.go | 4 +++- eth/api_test.go | 8 ++++++-- eth/api_tracer.go | 13 +++++++++--- eth/downloader/api.go | 4 +++- eth/filters/api.go | 12 ++++++++--- internal/build/archive.go | 14 ++++++++++--- internal/build/azure.go | 6 +++++- internal/debug/api.go | 27 +++++++++++++++++++------ les/api_backend.go | 4 +++- les/api_test.go | 10 +++++++-- light/txpool.go | 4 +++- mobile/android_test.go | 16 ++++++++++++--- node/api.go | 4 +++- signer/core/api.go | 4 +++- signer/core/api_test.go | 16 +++++++++++---- signer/storage/aes_gcm_storage_test.go | 4 +++- whisper/whisperv6/api.go | 8 ++++++-- 25 files changed, 190 insertions(+), 54 deletions(-) diff --git a/accounts/keystore/account_cache.go b/accounts/keystore/account_cache.go index 8f660e282f..f326c61663 100644 --- a/accounts/keystore/account_cache.go +++ b/accounts/keystore/account_cache.go @@ -212,7 +212,9 @@ func (ac *accountCache) maybeReload() { ac.watcher.start() ac.throttle.Reset(minReloadInterval) ac.mu.Unlock() - ac.scanAccounts() + if err := ac.scanAccounts(); err != nil { + // ignore error + } } func (ac *accountCache) close() { @@ -253,7 +255,11 @@ func (ac *accountCache) scanAccounts() error { log.Trace("Failed to open keystore file", "path", path, "err", err) return nil } - defer fd.Close() + defer func() { + if err := fd.Close(); err != nil { + panic(err) + } + }() buf.Reset(fd) // Parse the address. key.Address = "" diff --git a/accounts/keystore/account_cache_test.go b/accounts/keystore/account_cache_test.go index fe9233c046..4d0f602884 100644 --- a/accounts/keystore/account_cache_test.go +++ b/accounts/keystore/account_cache_test.go @@ -55,7 +55,11 @@ func TestWatchNewFile(t *testing.T) { t.Parallel() dir, ks := tmpKeyStore(t, false) - defer os.RemoveAll(dir) + defer func() { + if err := os.RemoveAll(dir); err != nil { + panic(err) + } + }() // Ensure the watcher is started before adding any files. ks.Accounts() @@ -106,8 +110,14 @@ func TestWatchNoDir(t *testing.T) { time.Sleep(100 * time.Millisecond) // Create the directory and copy a key file into it. - os.MkdirAll(dir, 0700) - defer os.RemoveAll(dir) + if err := os.MkdirAll(dir, 0700); err != nil { + panic(err) + } + defer func() { + if err := os.RemoveAll(dir); err != nil { + panic(err) + } + }() file := filepath.Join(dir, "aaa") if err := cp.CopyFile(file, cachetestAccounts[0].URL.Path); err != nil { t.Fatal(err) @@ -332,8 +342,14 @@ func TestUpdatedKeyfileContents(t *testing.T) { time.Sleep(100 * time.Millisecond) // Create the directory and copy a key file into it. - os.MkdirAll(dir, 0700) - defer os.RemoveAll(dir) + if err := os.MkdirAll(dir, 0700); err != nil { + panic(err) + } + defer func() { + if err := os.RemoveAll(dir); err != nil { + panic(err) + } + }() file := filepath.Join(dir, "aaa") // Place one of our testfiles in there diff --git a/consensus/ethash/algorithm.go b/consensus/ethash/algorithm.go index d6c871092e..689140f130 100644 --- a/consensus/ethash/algorithm.go +++ b/consensus/ethash/algorithm.go @@ -111,8 +111,12 @@ func makeHasher(h hash.Hash) hasher { outputLen := rh.Size() return func(dest []byte, data []byte) { rh.Reset() - rh.Write(data) - rh.Read(dest[:outputLen]) + if _, err := rh.Write(data); err != nil { + // ignore error + } + if _, err := rh.Read(dest[:outputLen]); err != nil { + // ignore error + } } } diff --git a/consensus/ethash/algorithm_test.go b/consensus/ethash/algorithm_test.go index cf8552f3ab..089bb70cf0 100644 --- a/consensus/ethash/algorithm_test.go +++ b/consensus/ethash/algorithm_test.go @@ -702,7 +702,11 @@ func TestConcurrentDiskCacheGeneration(t *testing.T) { if err != nil { t.Fatalf("Failed to create temporary cache dir: %v", err) } - defer os.RemoveAll(cachedir) + defer func() { + if err := os.RemoveAll(cachedir); err != nil { + panic(err) + } + }() // Define a heavy enough block, one from mainnet should do block := types.NewBlockWithHeader(&types.Header{ @@ -730,7 +734,11 @@ func TestConcurrentDiskCacheGeneration(t *testing.T) { go func(idx int) { defer pend.Done() ethash := New(Config{cachedir, 0, 1, "", 0, 0, ModeNormal}, nil, false) - defer ethash.Close() + defer func() { + if err := ethash.Close(); err != nil { + panic(err) + } + }() if err := ethash.VerifySeal(nil, block.Header()); err != nil { t.Errorf("proc %d: block verification failed: %v", idx, err) } diff --git a/core/rawdb/accessors_chain_test.go b/core/rawdb/accessors_chain_test.go index 8c8affffd9..d45967db0c 100644 --- a/core/rawdb/accessors_chain_test.go +++ b/core/rawdb/accessors_chain_test.go @@ -71,7 +71,9 @@ func TestBodyStorage(t *testing.T) { body := &types.Body{Uncles: []*types.Header{{Extra: []byte("test header")}}} hasher := sha3.NewLegacyKeccak256() - rlp.Encode(hasher, body) + if err := rlp.Encode(hasher, body); err != nil { + t.Fatalf("rlp.Encode failed with %v on %v", hasher, body) + } hash := common.BytesToHash(hasher.Sum(nil)) if entry := ReadBody(db, hash, 0); entry != nil { diff --git a/core/rawdb/accessors_indexes.go b/core/rawdb/accessors_indexes.go index 38f8fe10ea..9d6ee3b965 100644 --- a/core/rawdb/accessors_indexes.go +++ b/core/rawdb/accessors_indexes.go @@ -64,8 +64,8 @@ func WriteTxLookupEntries(db ethdb.KeyValueWriter, block *types.Block) { } // DeleteTxLookupEntry removes all transaction data associated with a hash. -func DeleteTxLookupEntry(db ethdb.KeyValueWriter, hash common.Hash) { - db.Delete(txLookupKey(hash)) +func DeleteTxLookupEntry(db ethdb.KeyValueWriter, hash common.Hash) error { + return db.Delete(txLookupKey(hash)) } // ReadTransaction retrieves a specific transaction from the database, along with diff --git a/core/rawdb/accessors_indexes_test.go b/core/rawdb/accessors_indexes_test.go index c09bff0101..52ce772295 100644 --- a/core/rawdb/accessors_indexes_test.go +++ b/core/rawdb/accessors_indexes_test.go @@ -56,7 +56,9 @@ func TestLookupStorage(t *testing.T) { Index: uint64(index), } data, _ := rlp.EncodeToBytes(entry) - db.Put(txLookupKey(tx.Hash()), data) + if err := db.Put(txLookupKey(tx.Hash()), data); err != nil { + // ignore error + } } }, }, diff --git a/eth/api.go b/eth/api.go index 98c2f5874f..603c709ffc 100644 --- a/eth/api.go +++ b/eth/api.go @@ -173,12 +173,20 @@ func (api *PrivateAdminAPI) ExportChain(file string) (bool, error) { if err != nil { return false, err } - defer out.Close() + defer func() { + if err := out.Close(); err != nil { + panic(err) + } + }() var writer io.Writer = out if strings.HasSuffix(file, ".gz") { writer = gzip.NewWriter(writer) - defer writer.(*gzip.Writer).Close() + defer func() { + if err := writer.(*gzip.Writer).Close(); err != nil { + panic(err) + } + }() } // Export the blockchain @@ -205,7 +213,11 @@ func (api *PrivateAdminAPI) ImportChain(file string) (bool, error) { if err != nil { return false, err } - defer in.Close() + defer func() { + if err := in.Close(); err != nil { + panic(err) + } + }() var reader io.Reader = in if strings.HasSuffix(file, ".gz") { diff --git a/eth/api_backend.go b/eth/api_backend.go index 69904a70f2..251cc0045b 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -56,7 +56,9 @@ func (b *EthAPIBackend) CurrentBlock() *types.Block { func (b *EthAPIBackend) SetHead(number uint64) { b.eth.protocolManager.downloader.Cancel() - b.eth.blockchain.SetHead(number) + if err := b.eth.blockchain.SetHead(number); err != nil { + panic(err) + } } func (b *EthAPIBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Header, error) { diff --git a/eth/api_test.go b/eth/api_test.go index 1e7c489c32..9006a20c46 100644 --- a/eth/api_test.go +++ b/eth/api_test.go @@ -81,7 +81,9 @@ func TestAccountRange(t *testing.T) { } } - state.Commit(true) + if _, err := state.Commit(true); err != nil { + panic(err) + } root := state.IntermediateRoot(true) trie, err := statedb.OpenTrie(root) @@ -165,7 +167,9 @@ func TestEmptyAccountRange(t *testing.T) { state, _ = state.New(common.Hash{}, statedb) ) - state.Commit(true) + if _, err := state.Commit(true); err != nil { + panic(err) + } root := state.IntermediateRoot(true) trie, err := statedb.OpenTrie(root) diff --git a/eth/api_tracer.go b/eth/api_tracer.go index ce211cbd99..d24f465f08 100644 --- a/eth/api_tracer.go +++ b/eth/api_tracer.go @@ -340,7 +340,9 @@ func (api *PrivateDebugAPI) traceChain(ctx context.Context, start, end *types.Bl // Stream completed traces to the user, aborting on the first error for result, ok := done[next]; ok; result, ok = done[next] { if len(result.Traces) > 0 || next == end.NumberU64() { - notifier.Notify(sub.ID, result) + if err := notifier.Notify(sub.ID, result); err != nil { + panic(err) + } } delete(done, next) next++ @@ -598,10 +600,15 @@ func (api *PrivateDebugAPI) standardTraceBlockToFile(ctx context.Context, block vmenv := vm.NewEVM(vmctx, statedb, api.eth.blockchain.Config(), vmConf) _, _, _, err = core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.Gas())) if writer != nil { - writer.Flush() + if err := writer.Flush(); err != nil { + panic(err) + } } if dump != nil { - dump.Close() + if er := dump.Close(); er != nil { + log.Error("Previous error", err) + panic(er) + } log.Info("Wrote standard trace", "file", dump.Name()) } if err != nil { diff --git a/eth/downloader/api.go b/eth/downloader/api.go index 57ff3d71af..4af197399d 100644 --- a/eth/downloader/api.go +++ b/eth/downloader/api.go @@ -105,7 +105,9 @@ func (api *PublicDownloaderAPI) Syncing(ctx context.Context) (*rpc.Subscription, for { select { case status := <-statuses: - notifier.Notify(rpcSub.ID, status) + if err := notifier.Notify(rpcSub.ID, status); err != nil { + panic(err) + } case <-rpcSub.Err(): sub.Unsubscribe() return diff --git a/eth/filters/api.go b/eth/filters/api.go index 5ed80a8875..f6b4bad527 100644 --- a/eth/filters/api.go +++ b/eth/filters/api.go @@ -153,7 +153,9 @@ func (api *PublicFilterAPI) NewPendingTransactions(ctx context.Context) (*rpc.Su // To keep the original behaviour, send a single tx hash in one notification. // TODO(rjl493456442) Send a batch of tx hashes in one notification for _, h := range hashes { - notifier.Notify(rpcSub.ID, h) + if err := notifier.Notify(rpcSub.ID, h); err != nil { + panic(err) + } } case <-rpcSub.Err(): pendingTxSub.Unsubscribe() @@ -219,7 +221,9 @@ func (api *PublicFilterAPI) NewHeads(ctx context.Context) (*rpc.Subscription, er for { select { case h := <-headers: - notifier.Notify(rpcSub.ID, h) + if err := notifier.Notify(rpcSub.ID, h); err != nil { + panic(err) + } case <-rpcSub.Err(): headersSub.Unsubscribe() return @@ -256,7 +260,9 @@ func (api *PublicFilterAPI) Logs(ctx context.Context, crit FilterCriteria) (*rpc select { case logs := <-matchedLogs: for _, log := range logs { - notifier.Notify(rpcSub.ID, &log) + if err := notifier.Notify(rpcSub.ID, &log); err != nil { + panic(err) + } } case <-rpcSub.Err(): // client send an unsubscribe request logsSub.Unsubscribe() diff --git a/internal/build/archive.go b/internal/build/archive.go index ac680ba63d..eb5591435b 100644 --- a/internal/build/archive.go +++ b/internal/build/archive.go @@ -58,7 +58,11 @@ func AddFile(a Archive, file string) error { if err != nil { return err } - defer fd.Close() + defer func () { + if err := fd.Close(); err != nil { + panic(err) + } + }() fi, err := fd.Stat() if err != nil { return err @@ -81,10 +85,14 @@ func WriteArchive(name string, files []string) (err error) { } defer func() { - archfd.Close() + if er := archfd.Close(); er != nil { + panic(er) + } // Remove the half-written archive on failure. if err != nil { - os.Remove(name) + if e := os.Remove(name); e != nil { + // Ignore error + } } }() archive, basename := NewArchive(archfd) diff --git a/internal/build/azure.go b/internal/build/azure.go index 7862842650..4a1c2d4e8a 100644 --- a/internal/build/azure.go +++ b/internal/build/azure.go @@ -63,7 +63,11 @@ func AzureBlobstoreUpload(path string, name string, config AzureBlobstoreConfig) if err != nil { return err } - defer in.Close() + defer func () { + if err := in.Close(); err != nil { + panic(err) + } + }() _, err = blockblob.Upload(context.Background(), in, azblob.BlobHTTPHeaders{}, azblob.Metadata{}, azblob.BlobAccessConditions{}) return err diff --git a/internal/debug/api.go b/internal/debug/api.go index 86a4218f6a..3839aa3f0f 100644 --- a/internal/debug/api.go +++ b/internal/debug/api.go @@ -90,7 +90,9 @@ func (h *HandlerT) CpuProfile(file string, nsec uint) error { return err } time.Sleep(time.Duration(nsec) * time.Second) - h.StopCPUProfile() + if err := h.StopCPUProfile(); err != nil { + panic(err) + } return nil } @@ -106,7 +108,10 @@ func (h *HandlerT) StartCPUProfile(file string) error { return err } if err := pprof.StartCPUProfile(f); err != nil { - f.Close() + if er := f.Close(); er != nil { + log.Error("Overridden error", err) + return er + } return err } h.cpuW = f @@ -124,7 +129,9 @@ func (h *HandlerT) StopCPUProfile() error { return errors.New("CPU profiling not in progress") } log.Info("Done writing CPU profile", "dump", h.cpuFile) - h.cpuW.Close() + if err := h.cpuW.Close(); err != nil { + panic(err) + } h.cpuW = nil h.cpuFile = "" return nil @@ -137,7 +144,9 @@ func (h *HandlerT) GoTrace(file string, nsec uint) error { return err } time.Sleep(time.Duration(nsec) * time.Second) - h.StopGoTrace() + if err := h.StopGoTrace(); err != nil { + return err + } return nil } @@ -192,7 +201,9 @@ func (*HandlerT) WriteMemProfile(file string) error { // Stacks returns a printed representation of the stacks of all goroutines. func (*HandlerT) Stacks() string { buf := new(bytes.Buffer) - pprof.Lookup("goroutine").WriteTo(buf, 2) + if err := pprof.Lookup("goroutine").WriteTo(buf, 2); err != nil { + panic(err) + } return buf.String() } @@ -214,7 +225,11 @@ func writeProfile(name, file string) error { if err != nil { return err } - defer f.Close() + defer func () { + if err := f.Close(); err != nil { + panic(err) + } + }() return p.WriteTo(f, 0) } diff --git a/les/api_backend.go b/les/api_backend.go index 07601c2423..5f39aa502a 100644 --- a/les/api_backend.go +++ b/les/api_backend.go @@ -55,7 +55,9 @@ func (b *LesApiBackend) CurrentBlock() *types.Block { func (b *LesApiBackend) SetHead(number uint64) { b.eth.protocolManager.downloader.Cancel() - b.eth.blockchain.SetHead(number) + if err := b.eth.blockchain.SetHead(number); err != nil { + panic(err) + } } func (b *LesApiBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Header, error) { diff --git a/les/api_test.go b/les/api_test.go index 6e622313ca..a91f9b7156 100644 --- a/les/api_test.go +++ b/les/api_test.go @@ -116,7 +116,9 @@ func testCapacityAPI(t *testing.T, clientCount int) { if i != freeIdx { setCapacity(ctx, t, serverRpcClient, client.ID(), testCap/uint64(len(clients))) } - net.Connect(client.ID(), server.ID()) + if err := net.Connect(client.ID(), server.ID()); err != nil { + panic(err) + } for { select { @@ -434,7 +436,11 @@ func NewAdapter(adapterType string, services adapters.Services) (adapter adapter if err0 != nil { return nil, teardown, err0 } - teardown = func() { os.RemoveAll(baseDir) } + teardown = func() { + if err := os.RemoveAll(baseDir); err != nil { + panic(err) + } + } adapter = adapters.NewExecAdapter(baseDir) /*case "docker": adapter, err = adapters.NewDockerAdapter() diff --git a/light/txpool.go b/light/txpool.go index 11a0e76ae0..79680bcb7e 100644 --- a/light/txpool.go +++ b/light/txpool.go @@ -204,7 +204,9 @@ func (pool *TxPool) rollbackTxs(hash common.Hash, txc txStateChanges) { if list, ok := pool.mined[hash]; ok { for _, tx := range list { txHash := tx.Hash() - rawdb.DeleteTxLookupEntry(batch, txHash) + if err := rawdb.DeleteTxLookupEntry(batch, txHash); err != nil { + // ignore err + } pool.pending[txHash] = tx txc.setState(txHash, false) } diff --git a/mobile/android_test.go b/mobile/android_test.go index 3d3bd66d08..c0dbe16d05 100644 --- a/mobile/android_test.go +++ b/mobile/android_test.go @@ -169,7 +169,9 @@ func TestAndroid(t *testing.T) { if _, err := os.Stat(autopath); err != nil { t.Skip("ANDROID_HOME environment var not set, skipping") } - os.Setenv("ANDROID_HOME", autopath) + if err := os.Setenv("ANDROID_HOME", autopath); err != nil { + panic(err) + } } if _, err := exec.Command("which", "gomobile").CombinedOutput(); err != nil { t.Log("gomobile missing, installing it...") @@ -188,7 +190,11 @@ func TestAndroid(t *testing.T) { if err != nil { t.Fatalf("failed to create temporary workspace: %v", err) } - defer os.RemoveAll(workspace) + defer func() { + if err := os.RemoveAll(workspace); err != nil { + panic(err) + } + }() pwd, err := os.Getwd() if err != nil { @@ -197,7 +203,11 @@ func TestAndroid(t *testing.T) { if err := os.Chdir(workspace); err != nil { t.Fatalf("failed to switch to temporary workspace: %v", err) } - defer os.Chdir(pwd) + defer func() { + if err := os.Chdir(pwd); err != nil { + panic(err) + } + }() // Create the skeleton of the Android project for _, dir := range []string{"src/main", "src/androidTest/java/org/ethereum/gethtest", "libs"} { diff --git a/node/api.go b/node/api.go index 66cd1dde33..4af4e7b4fb 100644 --- a/node/api.go +++ b/node/api.go @@ -128,7 +128,9 @@ func (api *PrivateAdminAPI) PeerEvents(ctx context.Context) (*rpc.Subscription, for { select { case event := <-events: - notifier.Notify(rpcSub.ID, event) + if err := notifier.Notify(rpcSub.ID, event); err != nil { + panic(err) + } case <-sub.Err(): return case <-rpcSub.Err(): diff --git a/signer/core/api.go b/signer/core/api.go index 244767acaf..462bdc20b4 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -362,7 +362,9 @@ func (api *SignerAPI) startUSBListener() { } case accounts.WalletDropped: log.Info("Old wallet dropped", "url", event.Wallet.URL()) - event.Wallet.Close() + if err := event.Wallet.Close(); err != nil { + panic(err) + } } } }() diff --git a/signer/core/api_test.go b/signer/core/api_test.go index 30948f99bf..5e9c4842a0 100644 --- a/signer/core/api_test.go +++ b/signer/core/api_test.go @@ -99,12 +99,16 @@ func (ui *headlessUi) ApproveNewAccount(request *core.NewAccountRequest) (core.N func (ui *headlessUi) ShowError(message string) { //stdout is used by communication - fmt.Fprintln(os.Stderr, message) + if err := fmt.Fprintln(os.Stderr, message); err != nil { + panic(err) + } } func (ui *headlessUi) ShowInfo(message string) { //stdout is used by communication - fmt.Fprintln(os.Stderr, message) + if err := fmt.Fprintln(os.Stderr, message); err != nil { + panic(err) + } } func tmpDirName(t *testing.T) string { @@ -286,7 +290,9 @@ func TestSignTx(t *testing.T) { t.Fatal(err) } parsedTx := &types.Transaction{} - rlp.Decode(bytes.NewReader(res.Raw), parsedTx) + if err := rlp.Decode(bytes.NewReader(res.Raw), parsedTx); err != nil { + panic(err) + } //The tx should NOT be modified by the UI if parsedTx.Value().Cmp(tx.Value.ToInt()) != 0 { @@ -312,7 +318,9 @@ func TestSignTx(t *testing.T) { t.Fatal(err) } parsedTx2 := &types.Transaction{} - rlp.Decode(bytes.NewReader(res.Raw), parsedTx2) + if err := rlp.Decode(bytes.NewReader(res.Raw), parsedTx2); err != nil { + panic(err) + } //The tx should be modified by the UI if parsedTx2.Value().Cmp(tx.Value.ToInt()) != 0 { diff --git a/signer/storage/aes_gcm_storage_test.go b/signer/storage/aes_gcm_storage_test.go index 664ef12994..7123077a40 100644 --- a/signer/storage/aes_gcm_storage_test.go +++ b/signer/storage/aes_gcm_storage_test.go @@ -70,7 +70,9 @@ func TestFileStorage(t *testing.T) { filename: fmt.Sprintf("%v/vault.json", d), key: []byte("AES256Key-32Characters1234567890"), } - stored.writeEncryptedStorage(a) + if err := stored.writeEncryptedStorage(a); err != nil { + // ignore error + } read := &AESEncryptedStorage{ filename: fmt.Sprintf("%v/vault.json", d), key: []byte("AES256Key-32Characters1234567890"), diff --git a/whisper/whisperv6/api.go b/whisper/whisperv6/api.go index d6d4c8d3de..8018a6e12b 100644 --- a/whisper/whisperv6/api.go +++ b/whisper/whisperv6/api.go @@ -418,10 +418,14 @@ func (api *PublicWhisperAPI) Messages(ctx context.Context, crit Criteria) (*rpc. } } case <-rpcSub.Err(): - api.w.Unsubscribe(id) + if err := api.w.Unsubscribe(id); err != nil { + panic(err) + } return case <-notifier.Closed(): - api.w.Unsubscribe(id) + if err := api.w.Unsubscribe(id); err != nil { + panic(err) + } return } }