From 156bb42758b5fba801fa523ec1e03dfb894ae4a2 Mon Sep 17 00:00:00 2001 From: Nikita Kryuchkov Date: Sun, 14 Jul 2019 21:07:13 +0300 Subject: [PATCH] Remove all linter excluding rules, fix all linter issues that appeared after removal. Make some code cleanup. --- .gitignore | 1 - .golangci.yml | 6 +- README.md | 4 +- ci_scripts/run-internal-tests.sh | 4 +- cmd/apps/helloworld/helloworld.go | 6 +- cmd/apps/skychat/README.md | 4 + cmd/apps/skychat/chat.go | 11 +- .../therealproxy-client.go | 6 +- cmd/apps/therealproxy/README.md | 4 + cmd/apps/therealproxy/therealproxy.go | 6 +- .../therealssh-client/therealssh-client.go | 12 +- cmd/apps/therealssh/README.md | 8 +- cmd/apps/therealssh/therealssh.go | 16 +- cmd/messaging-server/commands/root.go | 7 +- cmd/setup-node/commands/root.go | 4 +- cmd/skywire-cli/commands/node/gen-config.go | 6 +- cmd/skywire-cli/commands/root.go | 6 +- cmd/skywire-node/commands/root.go | 15 +- cmd/therealssh-cli/commands/root.go | 16 +- docs/Tests.Detection-of-unstable-tests.md | 8 +- integration/InteractiveEnvironments.md | 6 +- internal/httpauth/auth.go | 8 +- internal/httpauth/client.go | 23 +- internal/httpauth/client_test.go | 26 ++- internal/testhelpers/testhelpers.go | 19 ++ internal/therealproxy/client.go | 14 +- internal/therealproxy/server.go | 3 +- internal/therealproxy/server_test.go | 12 +- internal/therealssh/auth.go | 8 +- internal/therealssh/auth_test.go | 8 +- internal/therealssh/channel.go | 30 ++- internal/therealssh/channel_test.go | 7 +- internal/therealssh/client.go | 13 +- internal/therealssh/server.go | 13 +- internal/therealssh/server_test.go | 1 - internal/therealssh/session.go | 15 +- internal/therealssh/shell.go | 2 +- internal/therealssh/shell_darwin.go | 4 +- pkg/app/app.go | 30 ++- pkg/app/app_test.go | 104 ++++++--- pkg/app/packet_test.go | 2 +- pkg/app/protocol.go | 12 +- pkg/httputil/httputil.go | 10 +- pkg/manager/config.go | 2 +- pkg/manager/node.go | 5 +- pkg/manager/user.go | 2 +- pkg/manager/user_manager.go | 2 +- pkg/node/config.go | 2 +- pkg/node/config_test.go | 18 +- pkg/node/node.go | 28 ++- pkg/node/node_test.go | 25 ++- pkg/node/rpc_client.go | 13 +- pkg/node/rpc_test.go | 64 +++--- pkg/route-finder/client/client.go | 12 +- pkg/router/app_manager_test.go | 29 ++- pkg/router/managed_routing_table.go | 2 +- pkg/router/port_manager.go | 4 +- pkg/router/route_manager.go | 4 +- pkg/router/router.go | 39 +++- pkg/router/router_test.go | 198 ++++++++++++++---- pkg/routing/boltdb_routing_table.go | 24 ++- pkg/routing/boltdb_routing_table_test.go | 5 +- pkg/routing/packet.go | 2 +- pkg/routing/routing_table_test.go | 3 +- pkg/routing/rule.go | 4 +- pkg/setup/node.go | 18 +- pkg/setup/node_test.go | 61 ++++-- pkg/setup/protocol_test.go | 7 +- pkg/transport-discovery/client/client.go | 43 +++- pkg/transport-discovery/client/client_test.go | 31 +-- pkg/transport/discovery.go | 4 +- pkg/transport/handshake_test.go | 12 +- pkg/transport/log.go | 12 +- pkg/transport/log_test.go | 4 +- pkg/transport/manager.go | 38 +++- pkg/transport/manager_test.go | 12 +- pkg/transport/mock.go | 1 - pkg/transport/tcp_transport.go | 8 +- pkg/transport/tcp_transport_test.go | 4 +- pkg/transport/transport.go | 3 + pkg/util/pathutil/homedir.go | 6 +- vendor/github.com/skycoin/dmsg/client.go | 13 +- vendor/github.com/skycoin/dmsg/server.go | 4 - vendor/github.com/skycoin/dmsg/transport.go | 7 - 84 files changed, 914 insertions(+), 381 deletions(-) create mode 100644 internal/testhelpers/testhelpers.go diff --git a/.gitignore b/.gitignore index 423a3ec4e..6a601693b 100644 --- a/.gitignore +++ b/.gitignore @@ -10,7 +10,6 @@ .idea/ /skywire.json -/*-config.json /apps/ /skywire/ /local* diff --git a/.golangci.yml b/.golangci.yml index 8f87242ef..2dad27a04 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -169,10 +169,6 @@ issues: # it can be disabled by `exclude-use-default: false`. To list all # excluded by default patterns execute `golangci-lint run --help` exclude: - - "G304: Potential file inclusion via variable" - - "G204: Subprocess launched with variable" - - "G104: Errors unhandled" - - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked # Independently from option `exclude` we use default exclude patterns, # it can be disabled by this option. To list all @@ -192,4 +188,4 @@ issues: # large codebase. It's not practical to fix all existing issues at the moment # of integration: much better don't allow issues in new code. # Default is false. - new: false \ No newline at end of file + new: false diff --git a/README.md b/README.md index 708180179..53ef8e95e 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ - [2. Get an IP of node](#2-get-an-ip-of-node) - [3. Open in browser containerized `skychat` application](#3-open-in-browser-containerized-skychat-application) - [4. Create new dockerized `skywire-nodes`](#4-create-new-dockerized-skywire-nodes) - - [5. Env-vars for develoment-/testing- purposes](#5-env-vars-for-develoment-testing--purposes) + - [5. Env-vars for development-/testing- purposes](#5-env-vars-for-development-testing--purposes) - [6. "Hello-Mike-Hello-Joe" test](#6-hello-mike-hello-joe-test) ## Notes on this release @@ -424,7 +424,7 @@ Instead of skywire-runner you can use: - `golang`, `buildpack-deps:stretch-scm` "as is" - and `debian`, `ubuntu` - after `apt-get install ca-certificates` in them. Look in `skywire-runner.Dockerfile` for example -#### 5. Env-vars for develoment-/testing- purposes +#### 5. Env-vars for development-/testing- purposes ```bash export SW_NODE_A=127.0.0.1 diff --git a/ci_scripts/run-internal-tests.sh b/ci_scripts/run-internal-tests.sh index 95c99d7af..b15a3d365 100644 --- a/ci_scripts/run-internal-tests.sh +++ b/ci_scripts/run-internal-tests.sh @@ -19,8 +19,8 @@ go clean -testcache &>/dev/null || go test -race -tags no_ci -cover -timeout=5m go clean -testcache &>/dev/null || go test -race -tags no_ci -cover -timeout=5m github.com/skycoin/skywire/internal/therealproxy -run TestProxy >> ./logs/internal/TestProxy.log -go clean -testcache &>/dev/null || go test -race -tags no_ci -cover -timeout=5m github.com/skycoin/skywire/internal/therealssh -run TestListAuthoriser >> ./logs/internal/TestListAuthoriser.log -go clean -testcache &>/dev/null || go test -race -tags no_ci -cover -timeout=5m github.com/skycoin/skywire/internal/therealssh -run TestFileAuthoriser >> ./logs/internal/TestFileAuthoriser.log +go clean -testcache &>/dev/null || go test -race -tags no_ci -cover -timeout=5m github.com/skycoin/skywire/internal/therealssh -run TestListAuthorizer >> ./logs/internal/TestListAuthorizer.log +go clean -testcache &>/dev/null || go test -race -tags no_ci -cover -timeout=5m github.com/skycoin/skywire/internal/therealssh -run TestFileAuthorizer >> ./logs/internal/TestFileAuthorizer.log go clean -testcache &>/dev/null || go test -race -tags no_ci -cover -timeout=5m github.com/skycoin/skywire/internal/therealssh -run TestChannelServe >> ./logs/internal/TestChannelServe.log go clean -testcache &>/dev/null || go test -race -tags no_ci -cover -timeout=5m github.com/skycoin/skywire/internal/therealssh -run TestChannelSendWrite >> ./logs/internal/TestChannelSendWrite.log go clean -testcache &>/dev/null || go test -race -tags no_ci -cover -timeout=5m github.com/skycoin/skywire/internal/therealssh -run TestChannelRead >> ./logs/internal/TestChannelRead.log diff --git a/cmd/apps/helloworld/helloworld.go b/cmd/apps/helloworld/helloworld.go index 25205930c..ab0e13e4a 100644 --- a/cmd/apps/helloworld/helloworld.go +++ b/cmd/apps/helloworld/helloworld.go @@ -17,7 +17,11 @@ func main() { if err != nil { log.Fatal("Setup failure: ", err) } - defer helloworldApp.Close() + defer func() { + if err := helloworldApp.Close(); err != nil { + log.Println("Failed to close app: ", err) + } + }() if len(os.Args) == 1 { log.Println("listening for incoming connections") diff --git a/cmd/apps/skychat/README.md b/cmd/apps/skychat/README.md index 66aed3b62..11985e98c 100644 --- a/cmd/apps/skychat/README.md +++ b/cmd/apps/skychat/README.md @@ -13,6 +13,7 @@ Create 2 node config files: `skywire1.json` ```json +{ "apps": [ { "app": "skychat", @@ -21,11 +22,13 @@ Create 2 node config files: "port": 1 } ] +} ``` `skywire2.json` ```json +{ "apps": [ { "app": "skychat", @@ -35,6 +38,7 @@ Create 2 node config files: "args": ["-addr", ":8001"] } ] +} ``` Compile binaries and start 2 nodes: diff --git a/cmd/apps/skychat/chat.go b/cmd/apps/skychat/chat.go index ba80c6f18..4841f9dee 100644 --- a/cmd/apps/skychat/chat.go +++ b/cmd/apps/skychat/chat.go @@ -38,7 +38,11 @@ func main() { if err != nil { log.Fatal("Setup failure: ", err) } - defer func() { _ = a.Close() }() + defer func() { + if err := a.Close(); err != nil { + log.Println("Failed to close app: ", err) + } + }() chatApp = a @@ -83,7 +87,10 @@ func handleConn(conn net.Conn) { return } - clientMsg, _ := json.Marshal(map[string]string{"sender": raddr.PubKey.Hex(), "message": string(buf[:n])}) // nolint + clientMsg, err := json.Marshal(map[string]string{"sender": raddr.PubKey.Hex(), "message": string(buf[:n])}) + if err != nil { + log.Printf("Failed to marshal json: %v", err) + } select { case clientCh <- string(clientMsg): log.Printf("received and sent to ui: %s\n", clientMsg) diff --git a/cmd/apps/therealproxy-client/therealproxy-client.go b/cmd/apps/therealproxy-client/therealproxy-client.go index 68f8a1186..fd089cde7 100644 --- a/cmd/apps/therealproxy-client/therealproxy-client.go +++ b/cmd/apps/therealproxy-client/therealproxy-client.go @@ -30,7 +30,11 @@ func main() { if err != nil { log.Fatal("Setup failure: ", err) } - defer socksApp.Close() + defer func() { + if err := socksApp.Close(); err != nil { + log.Println("Failed to close app: ", err) + } + }() if *serverPK == "" { log.Fatal("Invalid server PubKey") diff --git a/cmd/apps/therealproxy/README.md b/cmd/apps/therealproxy/README.md index 5ad2e0bed..bd7f3a883 100644 --- a/cmd/apps/therealproxy/README.md +++ b/cmd/apps/therealproxy/README.md @@ -15,6 +15,7 @@ Create 2 node config files: - `skywire1.json` ```json +{ "apps": [ { "app": "socksproxy", @@ -24,11 +25,13 @@ Create 2 node config files: "args": ["-passcode", "123456"] } ] +} ``` - `skywire2.json` ```json +{ "apps": [ { "app": "socksproxy-client", @@ -38,6 +41,7 @@ Create 2 node config files: "args": ["-srv", "024ec47420176680816e0406250e7156465e4531f5b26057c9f6297bb0303558c7"] } ] +} ``` Compile binaries and start 2 nodes: diff --git a/cmd/apps/therealproxy/therealproxy.go b/cmd/apps/therealproxy/therealproxy.go index 34bfdbb41..a1396e9c3 100644 --- a/cmd/apps/therealproxy/therealproxy.go +++ b/cmd/apps/therealproxy/therealproxy.go @@ -20,7 +20,11 @@ func main() { if err != nil { log.Fatal("Setup failure: ", err) } - defer socksApp.Close() + defer func() { + if err := socksApp.Close(); err != nil { + log.Println("Failed to close app: ", err) + } + }() srv, err := therealproxy.NewServer(*passcode) if err != nil { diff --git a/cmd/apps/therealssh-client/therealssh-client.go b/cmd/apps/therealssh-client/therealssh-client.go index b0ab61005..80663e241 100644 --- a/cmd/apps/therealssh-client/therealssh-client.go +++ b/cmd/apps/therealssh-client/therealssh-client.go @@ -22,7 +22,11 @@ func main() { if err != nil { log.Fatal("Setup failure: ", err) } - defer sshApp.Close() + defer func() { + if err := sshApp.Close(); err != nil { + log.Println("Failed to close app: ", err) + } + }() ssh.Debug = *debug @@ -30,7 +34,11 @@ func main() { if err != nil { log.Fatal("Client setup failure: ", err) } - defer client.Close() + defer func() { + if err := client.Close(); err != nil { + log.Println("Failed to close client: ", err) + } + }() if err := http.Serve(rpc, nil); err != nil { log.Fatal("Failed to start RPC interface: ", err) diff --git a/cmd/apps/therealssh/README.md b/cmd/apps/therealssh/README.md index fa0c149d4..414bdfc08 100644 --- a/cmd/apps/therealssh/README.md +++ b/cmd/apps/therealssh/README.md @@ -20,6 +20,7 @@ Create 2 node config files: `skywire1.json` ```json +{ "apps": [ { "app": "SSH", @@ -28,11 +29,13 @@ Create 2 node config files: "port": 2 } ] +} ``` `skywire2.json` ```json +{ "apps": [ { "app": "SSH-client", @@ -41,6 +44,7 @@ Create 2 node config files: "port": 22 } ] +} ``` Compile binaries and start 2 nodes: @@ -56,8 +60,8 @@ $ ./skywire-node skywire2.json Add public key of the second node to the auth file: ```bash -$ mkdir `/.therealssh -$ echo "0348c941c5015a05c455ff238af2e57fb8f914c399aab604e9abb5b32b91a4c1fe" > `/.SSH/authorized_keys +$ mkdir /.therealssh +$ echo "0348c941c5015a05c455ff238af2e57fb8f914c399aab604e9abb5b32b91a4c1fe" > /.SSH/authorized_keys ``` Connect to the first node using CLI: diff --git a/cmd/apps/therealssh/therealssh.go b/cmd/apps/therealssh/therealssh.go index 69e813baa..2bb5d2237 100644 --- a/cmd/apps/therealssh/therealssh.go +++ b/cmd/apps/therealssh/therealssh.go @@ -7,7 +7,7 @@ import ( "flag" "log" - homedir "github.com/mitchellh/go-homedir" + "github.com/mitchellh/go-homedir" ssh "github.com/skycoin/skywire/internal/therealssh" "github.com/skycoin/skywire/pkg/app" @@ -24,7 +24,11 @@ func main() { if err != nil { log.Fatal("Setup failure: ", err) } - defer sshApp.Close() + defer func() { + if err := sshApp.Close(); err != nil { + log.Println("Failed to close app: ", err) + } + }() path, err := homedir.Expand(*authFile) if err != nil { @@ -35,11 +39,15 @@ func main() { auth, err := ssh.NewFileAuthorizer(path) if err != nil { - log.Fatal("Failed to setup Authoriser: ", err) + log.Fatal("Failed to setup Authorizer: ", err) } server := ssh.NewServer(auth) - defer server.Close() + defer func() { + if err := server.Close(); err != nil { + log.Println("Failed to close server: ", err) + } + }() for { conn, err := sshApp.Accept() diff --git a/cmd/messaging-server/commands/root.go b/cmd/messaging-server/commands/root.go index 6878a2f3a..d4d6674c7 100644 --- a/cmd/messaging-server/commands/root.go +++ b/cmd/messaging-server/commands/root.go @@ -9,9 +9,10 @@ import ( "net" "net/http" "os" + "path/filepath" "github.com/prometheus/client_golang/prometheus/promhttp" - logrus_syslog "github.com/sirupsen/logrus/hooks/syslog" + logrussyslog "github.com/sirupsen/logrus/hooks/syslog" "github.com/skycoin/dmsg" "github.com/skycoin/dmsg/cipher" "github.com/skycoin/dmsg/disc" @@ -56,7 +57,7 @@ var rootCmd = &cobra.Command{ logging.SetLevel(logLevel) if syslogAddr != "" { - hook, err := logrus_syslog.NewSyslogHook("udp", syslogAddr, syslog.LOG_INFO, tag) + hook, err := logrussyslog.NewSyslogHook("udp", syslogAddr, syslog.LOG_INFO, tag) if err != nil { logger.Fatalf("Unable to connect to syslog daemon on %v", syslogAddr) } @@ -97,7 +98,7 @@ func parseConfig(configFile string) *Config { var rdr io.Reader var err error if !cfgFromStdin { - rdr, err = os.Open(configFile) + rdr, err = os.Open(filepath.Clean(configFile)) if err != nil { log.Fatalf("Failed to open config: %s", err) } diff --git a/cmd/setup-node/commands/root.go b/cmd/setup-node/commands/root.go index 4c94e34d4..7de9a7384 100644 --- a/cmd/setup-node/commands/root.go +++ b/cmd/setup-node/commands/root.go @@ -11,7 +11,7 @@ import ( "os" "github.com/prometheus/client_golang/prometheus/promhttp" - logrus_syslog "github.com/sirupsen/logrus/hooks/syslog" + logrussyslog "github.com/sirupsen/logrus/hooks/syslog" "github.com/skycoin/skycoin/src/util/logging" "github.com/spf13/cobra" @@ -33,7 +33,7 @@ var rootCmd = &cobra.Command{ logger := logging.MustGetLogger(tag) if syslogAddr != "" { - hook, err := logrus_syslog.NewSyslogHook("udp", syslogAddr, syslog.LOG_INFO, tag) + hook, err := logrussyslog.NewSyslogHook("udp", syslogAddr, syslog.LOG_INFO, tag) if err != nil { logger.Fatalf("Unable to connect to syslog daemon on %v", syslogAddr) } diff --git a/cmd/skywire-cli/commands/node/gen-config.go b/cmd/skywire-cli/commands/node/gen-config.go index 0f16fe65a..b4544c94b 100644 --- a/cmd/skywire-cli/commands/node/gen-config.go +++ b/cmd/skywire-cli/commands/node/gen-config.go @@ -98,8 +98,12 @@ func defaultConfig() *node.Config { conf.Transport.LogStore.Location = "./skywire/transport_logs" conf.Routing.RouteFinder = "https://routefinder.skywire.skycoin.net/" + + const defaultSetupNodePK = "0324579f003e6b4048bae2def4365e634d8e0e3054a20fc7af49daf2a179658557" sPK := cipher.PubKey{} - sPK.UnmarshalText([]byte("0324579f003e6b4048bae2def4365e634d8e0e3054a20fc7af49daf2a179658557")) // nolint: errcheck + if err := sPK.UnmarshalText([]byte(defaultSetupNodePK)); err != nil { + log.WithError(err).Warnf("Failed to unmarshal default setup node public key %s", defaultSetupNodePK) + } conf.Routing.SetupNodes = []cipher.PubKey{sPK} conf.Routing.Table.Type = "boltdb" conf.Routing.Table.Location = "./skywire/routing.db" diff --git a/cmd/skywire-cli/commands/root.go b/cmd/skywire-cli/commands/root.go index c08a222bc..2c03ffb3d 100644 --- a/cmd/skywire-cli/commands/root.go +++ b/cmd/skywire-cli/commands/root.go @@ -1,6 +1,8 @@ package commands import ( + "log" + "github.com/spf13/cobra" "github.com/skycoin/skywire/cmd/skywire-cli/commands/mdisc" @@ -25,5 +27,7 @@ func init() { // Execute executes root CLI command. func Execute() { - _ = rootCmd.Execute() //nolint:errcheck + if err := rootCmd.Execute(); err != nil { + log.Fatal("Failed to execute command: ", err) + } } diff --git a/cmd/skywire-node/commands/root.go b/cmd/skywire-node/commands/root.go index 0a2fd686d..a53d4297e 100644 --- a/cmd/skywire-node/commands/root.go +++ b/cmd/skywire-node/commands/root.go @@ -8,21 +8,20 @@ import ( "io/ioutil" "log" "log/syslog" + "net/http" + _ "net/http/pprof" // used for HTTP profiling "os" "os/signal" + "path/filepath" "strings" "syscall" "time" - logrus_syslog "github.com/sirupsen/logrus/hooks/syslog" + "github.com/pkg/profile" + logrussyslog "github.com/sirupsen/logrus/hooks/syslog" "github.com/skycoin/skycoin/src/util/logging" "github.com/spf13/cobra" - "net/http" - _ "net/http/pprof" //no_lint - - "github.com/pkg/profile" - "github.com/skycoin/skywire/pkg/node" "github.com/skycoin/skywire/pkg/util/pathutil" ) @@ -111,7 +110,7 @@ func (cfg *runCfg) startLogger() *runCfg { cfg.logger = cfg.masterLogger.PackageLogger(cfg.tag) if cfg.syslogAddr != "none" { - hook, err := logrus_syslog.NewSyslogHook("udp", cfg.syslogAddr, syslog.LOG_INFO, cfg.tag) + hook, err := logrussyslog.NewSyslogHook("udp", cfg.syslogAddr, syslog.LOG_INFO, cfg.tag) if err != nil { cfg.logger.Error("Unable to connect to syslog daemon:", err) } else { @@ -127,7 +126,7 @@ func (cfg *runCfg) readConfig() *runCfg { var err error if !cfg.cfgFromStdin { configPath := pathutil.FindConfigPath(cfg.args, 0, configEnv, pathutil.NodeDefaults()) - rdr, err = os.Open(configPath) + rdr, err = os.Open(filepath.Clean(configPath)) if err != nil { cfg.logger.Fatalf("Failed to open config: %s", err) } diff --git a/cmd/therealssh-cli/commands/root.go b/cmd/therealssh-cli/commands/root.go index 646ef6ba6..12fcfdd55 100644 --- a/cmd/therealssh-cli/commands/root.go +++ b/cmd/therealssh-cli/commands/root.go @@ -21,9 +21,7 @@ import ( ssh "github.com/skycoin/skywire/internal/therealssh" ) -var ( - rpcAddr string -) +var rpcAddr string var rootCmd = &cobra.Command{ Use: "SSH-cli [user@]remotePK [command] [args...]", @@ -55,7 +53,11 @@ var rootCmd = &cobra.Command{ if err := client.Call("RPCClient.RequestPTY", ptyArgs, &channelID); err != nil { log.Fatal("Failed to request PTY:", err) } - defer client.Call("RPCClient.Close", &channelID, nil) // nolint: errcheck + defer func() { + if err := client.Call("RPCClient.Close", &channelID, nil); err != nil { + log.Printf("Failed to close RPC client: %v", err) + } + }() var socketPath string execArgs := &ssh.ExecArgs{ChannelID: channelID, CommandWithArgs: args[1:]} @@ -97,7 +99,11 @@ var rootCmd = &cobra.Command{ if err != nil { log.Fatal("Failed to set terminal to raw mode:", err) } - defer terminal.Restore(int(os.Stdin.Fd()), oldState) // nolint + defer func() { + if err := terminal.Restore(int(os.Stdin.Fd()), oldState); err != nil { + log.Printf("Failed to restore terminal: %v", err) + } + }() go func() { if _, err := io.Copy(conn, os.Stdin); err != nil { diff --git a/docs/Tests.Detection-of-unstable-tests.md b/docs/Tests.Detection-of-unstable-tests.md index ceb63313d..1d3a9c0e2 100644 --- a/docs/Tests.Detection-of-unstable-tests.md +++ b/docs/Tests.Detection-of-unstable-tests.md @@ -1,8 +1,8 @@ # Tests. Detection of unstable tests -## Synopsys +## Synopsis -This document describe procedure to detect tests that FAILs with some probability, e.g. 10-15% of runs. +This document describes a procedure to detect tests that FAIL with some probability, e.g. 10-15% of runs. Such tests are questionable themselves and they add instability of CI builds. @@ -17,7 +17,7 @@ go test ./pkg/... -list "Test*|Example*" > ./logs/list-of-pkg-tests.txt # use a go test ./internal/... -list "Test*|Example*" > ./logs/list-of-internal-tests.txt ``` -You will get output simlar to: +You will get output similar to: ```text TestClient @@ -114,7 +114,7 @@ Either fix it or tag it with `no_ci` tag. **Detected unstable test** -It was observed that travic_ci builds randomly fails. +It was observed that Travis CI builds randomly fail. Narrowing search it was found that problem arises in `pkg/messaging` tests. diff --git a/integration/InteractiveEnvironments.md b/integration/InteractiveEnvironments.md index 10f3b78bc..b757c5a98 100644 --- a/integration/InteractiveEnvironments.md +++ b/integration/InteractiveEnvironments.md @@ -22,11 +22,11 @@ ```text integration -├── generic # Generic environmnent +├── generic # Generic environment │   ├── env-vars.sh # │   ├── nodeA.json # │   └── nodeC.json # -├── messaging # Messaging testing environment +├── messaging # Messaging testing environment │   ├── env-vars.sh # │   ├── nodeA.json # │   └── nodeC.json # @@ -37,7 +37,7 @@ integration ├── ssh # ssh testing environment │   ├── env-vars.sh # │   ├── nodeA.json # -│   └── nodeC.json #S +│   └── nodeC.json # ├── InteractiveEnvironments.md # You're reading it ├── intermediary-nodeB.json # NodeB configurationS ├── run-base-env.sh # base environment in detached tmux session diff --git a/internal/httpauth/auth.go b/internal/httpauth/auth.go index 03a3a8cb0..a3417faca 100644 --- a/internal/httpauth/auth.go +++ b/internal/httpauth/auth.go @@ -31,7 +31,7 @@ func AuthFromHeaders(hdr http.Header) (*Auth, error) { } key := cipher.PubKey{} if err := key.UnmarshalText([]byte(v)); err != nil { - return nil, fmt.Errorf("Error parsing SW-Public: %s", err.Error()) + return nil, fmt.Errorf("error parsing SW-Public: %s", err.Error()) } a.Key = key @@ -40,7 +40,7 @@ func AuthFromHeaders(hdr http.Header) (*Auth, error) { } sig := cipher.Sig{} if err := sig.UnmarshalText([]byte(v)); err != nil { - return nil, fmt.Errorf("Error parsing SW-Sig:'%s': %s", v, err.Error()) + return nil, fmt.Errorf("error parsing SW-Sig:'%s': %s", v, err.Error()) } a.Sig = sig @@ -51,10 +51,10 @@ func AuthFromHeaders(hdr http.Header) (*Auth, error) { nonceUint, err := strconv.ParseUint(nonceStr, 10, 64) if err != nil { if numErr, ok := err.(*strconv.NumError); ok { - return nil, fmt.Errorf("Error parsing SW-Nonce: %s", numErr.Err.Error()) + return nil, fmt.Errorf("error parsing SW-Nonce: %s", numErr.Err.Error()) } - return nil, fmt.Errorf("Error parsing SW-Nonce: %s", err.Error()) + return nil, fmt.Errorf("error parsing SW-Nonce: %s", err.Error()) } a.Nonce = Nonce(nonceUint) diff --git a/internal/httpauth/client.go b/internal/httpauth/client.go index 5c1823c95..2d861922c 100644 --- a/internal/httpauth/client.go +++ b/internal/httpauth/client.go @@ -15,12 +15,15 @@ import ( "sync/atomic" "github.com/skycoin/dmsg/cipher" + "github.com/skycoin/skycoin/src/util/logging" ) const ( invalidNonceErrorMessage = "SW-Nonce does not match" ) +var log = logging.MustGetLogger("httpauth") + // NextNonceResponse represents a ServeHTTP response for json encoding type NextNonceResponse struct { Edge cipher.PubKey `json:"edge"` @@ -81,7 +84,9 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) { if err != nil { return nil, err } - req.Body.Close() + if err := req.Body.Close(); err != nil { + log.WithError(err).Warn("Failed to close HTTP request body") + } req.Body = ioutil.NopCloser(bytes.NewBuffer(auxBody)) body = auxBody } @@ -103,7 +108,9 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) { } c.SetNonce(nonce) - res.Body.Close() + if err := res.Body.Close(); err != nil { + log.WithError(err).Warn("Failed to close HTTP response body") + } res, err = c.doRequest(req, body) if err != nil { return nil, err @@ -126,10 +133,16 @@ func (c *Client) Nonce(ctx context.Context, key cipher.PubKey) (Nonce, error) { req = req.WithContext(ctx) resp, err := c.client.Do(req) + if resp != nil { + defer func() { + if err := resp.Body.Close(); err != nil { + log.WithError(err).Warn("Failed to close HTTP response body") + } + }() + } if err != nil { return 0, err } - defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return 0, fmt.Errorf("error getting current nonce: status: %d <- %v", resp.StatusCode, extractError(resp.Body)) @@ -186,7 +199,9 @@ func isNonceValid(res *http.Response) (bool, error) { if err != nil { return false, err } - res.Body.Close() + if err := res.Body.Close(); err != nil { + return false, err + } res.Body = ioutil.NopCloser(bytes.NewBuffer(auxRespBody)) if err := json.Unmarshal(auxRespBody, &serverResponse); err != nil || serverResponse.Error == nil { diff --git a/internal/httpauth/client_test.go b/internal/httpauth/client_test.go index 4697d88c6..d3cff01c9 100644 --- a/internal/httpauth/client_test.go +++ b/internal/httpauth/client_test.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "io/ioutil" - "log" "net/http" "net/http/httptest" "os" @@ -43,7 +42,7 @@ func TestClient(t *testing.T) { pk, sk := cipher.GenerateKeyPair() headerCh := make(chan http.Header, 1) - ts := newTestServer(pk, headerCh) + ts := newTestServer(t, pk, headerCh) defer ts.Close() c, err := NewClient(context.TODO(), ts.URL, pk, sk) @@ -55,8 +54,10 @@ func TestClient(t *testing.T) { require.NoError(t, err) b, err := ioutil.ReadAll(res.Body) + if b != nil { + require.NoError(t, res.Body.Close()) + } require.NoError(t, err) - res.Body.Close() assert.Equal(t, []byte(payload), b) assert.Equal(t, uint64(2), c.nonce) @@ -69,7 +70,7 @@ func TestClient_BadNonce(t *testing.T) { pk, sk := cipher.GenerateKeyPair() headerCh := make(chan http.Header, 1) - ts := newTestServer(pk, headerCh) + ts := newTestServer(t, pk, headerCh) defer ts.Close() c, err := NewClient(context.TODO(), ts.URL, pk, sk) @@ -83,8 +84,10 @@ func TestClient_BadNonce(t *testing.T) { require.NoError(t, err) b, err := ioutil.ReadAll(res.Body) + if b != nil { + require.NoError(t, res.Body.Close()) + } require.NoError(t, err) - res.Body.Close() assert.Equal(t, uint64(2), c.nonce) headers := <-headerCh @@ -99,17 +102,21 @@ func checkResp(t *testing.T, headers http.Header, body []byte, pk cipher.PubKey, require.NoError(t, cipher.VerifyPubKeySignedPayload(pk, sig, PayloadWithNonce(body, Nonce(nonce)))) } -func newTestServer(pk cipher.PubKey, headerCh chan<- http.Header) *httptest.Server { +func newTestServer(t *testing.T, pk cipher.PubKey, headerCh chan<- http.Header) *httptest.Server { nonce := 1 return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.String() == fmt.Sprintf("/security/nonces/%s", pk) { - json.NewEncoder(w).Encode(&NextNonceResponse{pk, Nonce(nonce)}) // nolint: errcheck + require.NoError(t, json.NewEncoder(w).Encode(&NextNonceResponse{pk, Nonce(nonce)})) } else { body, err := ioutil.ReadAll(r.Body) + if body != nil { + defer func() { + require.NoError(t, r.Body.Close()) + }() + } if err != nil { return } - defer r.Body.Close() respMessage := string(body) if r.Header.Get("Sw-Nonce") != strconv.Itoa(nonce) { respMessage = errorMessage @@ -117,7 +124,8 @@ func newTestServer(pk cipher.PubKey, headerCh chan<- http.Header) *httptest.Serv headerCh <- r.Header nonce++ } - fmt.Fprint(w, respMessage) + _, err = fmt.Fprint(w, respMessage) + require.NoError(t, err) } })) } diff --git a/internal/testhelpers/testhelpers.go b/internal/testhelpers/testhelpers.go new file mode 100644 index 000000000..72f3f92fa --- /dev/null +++ b/internal/testhelpers/testhelpers.go @@ -0,0 +1,19 @@ +// Package testhelpers provides helpers for testing. +package testhelpers + +import ( + "time" +) + +const timeout = 5 * time.Second + +// NoErrorWithinTimeout tries to read an error from error channel within timeout and returns it. +// If timeout exceeds, nil value is returned. +func NoErrorWithinTimeout(ch <-chan error) error { + select { + case err := <-ch: + return err + case <-time.After(timeout): + return nil + } +} diff --git a/internal/therealproxy/client.go b/internal/therealproxy/client.go index 397cc4408..c7f6efbf9 100644 --- a/internal/therealproxy/client.go +++ b/internal/therealproxy/client.go @@ -3,12 +3,14 @@ package therealproxy import ( "fmt" "io" - "log" "net" "github.com/hashicorp/yamux" + "github.com/skycoin/skycoin/src/util/logging" ) +var log = logging.MustGetLogger("therealproxy") + // Client implement multiplexing proxy client using yamux. type Client struct { session *yamux.Session @@ -61,11 +63,15 @@ func (c *Client) ListenAndServe(addr string) error { }() for err := range errCh { - conn.Close() - stream.Close() + if err := conn.Close(); err != nil { + log.WithError(err).Warn("Failed to close connection") + } + if err := stream.Close(); err != nil { + log.WithError(err).Warn("Failed to close stream") + } if err != nil { - log.Println("Copy error:", err) + log.Error("Copy error:", err) } } }() diff --git a/internal/therealproxy/server.go b/internal/therealproxy/server.go index 4f0c3a7f3..866abad52 100644 --- a/internal/therealproxy/server.go +++ b/internal/therealproxy/server.go @@ -2,7 +2,6 @@ package therealproxy import ( "fmt" - "log" "net" "github.com/armon/go-socks5" @@ -47,7 +46,7 @@ func (s *Server) Serve(l net.Listener) error { go func() { if err := s.socks.Serve(session); err != nil { - log.Println("Failed to start SOCKS5 server:", err) + log.Error("Failed to start SOCKS5 server:", err) } }() } diff --git a/internal/therealproxy/server_test.go b/internal/therealproxy/server_test.go index c722d370c..f173ca8d5 100644 --- a/internal/therealproxy/server_test.go +++ b/internal/therealproxy/server_test.go @@ -3,7 +3,6 @@ package therealproxy import ( "fmt" "io/ioutil" - "log" "net" "net/http" "net/http/httptest" @@ -36,7 +35,7 @@ func TestProxy(t *testing.T) { srv, err := NewServer("") require.NoError(t, err) - l, err := net.Listen("tcp", ":10081") // nolint: gosec + l, err := net.Listen("tcp", "localhost:10081") require.NoError(t, err) errChan := make(chan error) @@ -46,7 +45,7 @@ func TestProxy(t *testing.T) { time.Sleep(100 * time.Millisecond) - conn, err := net.Dial("tcp", ":10081") + conn, err := net.Dial("tcp", "localhost:10081") require.NoError(t, err) client, err := NewClient(conn) @@ -54,16 +53,17 @@ func TestProxy(t *testing.T) { errChan2 := make(chan error) go func() { - errChan2 <- client.ListenAndServe(":10080") + errChan2 <- client.ListenAndServe("localhost:10080") }() time.Sleep(100 * time.Millisecond) - proxyDial, err := proxy.SOCKS5("tcp", ":10080", nil, proxy.Direct) + proxyDial, err := proxy.SOCKS5("tcp", "localhost:10080", nil, proxy.Direct) require.NoError(t, err) ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintln(w, "Hello, client") + _, err := fmt.Fprintln(w, "Hello, client") + require.NoError(t, err) })) defer ts.Close() diff --git a/internal/therealssh/auth.go b/internal/therealssh/auth.go index c4ed1923c..45cfd8fde 100644 --- a/internal/therealssh/auth.go +++ b/internal/therealssh/auth.go @@ -43,7 +43,7 @@ func NewFileAuthorizer(authFile string) (*FileAuthorizer, error) { return nil, fmt.Errorf("failed to resolve auth file path: %s", err) } - f, err := os.Open(path) + f, err := os.Open(filepath.Clean(path)) if err != nil { if os.IsNotExist(err) { if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { @@ -70,7 +70,11 @@ func (auth *FileAuthorizer) Close() error { // Authorize implements Authorizer for FileAuthorizer func (auth *FileAuthorizer) Authorize(remotePK cipher.PubKey) error { - defer auth.authFile.Seek(0, 0) // nolint + defer func() { + if _, err := auth.authFile.Seek(0, 0); err != nil { + log.WithError(err).Warn("Failed to seek to the beginning of auth file") + } + }() hexPK := remotePK.Hex() scanner := bufio.NewScanner(auth.authFile) diff --git a/internal/therealssh/auth_test.go b/internal/therealssh/auth_test.go index 22c327853..8871d2452 100644 --- a/internal/therealssh/auth_test.go +++ b/internal/therealssh/auth_test.go @@ -9,20 +9,22 @@ import ( "github.com/stretchr/testify/require" ) -func TestListAuthoriser(t *testing.T) { +func TestListAuthorizer(t *testing.T) { pk, _ := cipher.GenerateKeyPair() auth := &ListAuthorizer{[]cipher.PubKey{pk}} require.Error(t, auth.Authorize(cipher.PubKey{})) require.NoError(t, auth.Authorize(pk)) } -func TestFileAuthoriser(t *testing.T) { +func TestFileAuthorizer(t *testing.T) { pk, _ := cipher.GenerateKeyPair() anotherPK, _ := cipher.GenerateKeyPair() tmpfile, err := ioutil.TempFile("", "authorized_keys") require.NoError(t, err) - defer os.Remove(tmpfile.Name()) + defer func() { + require.NoError(t, os.Remove(tmpfile.Name())) + }() _, err = tmpfile.Write([]byte(pk.Hex() + "\n")) require.NoError(t, err) diff --git a/internal/therealssh/channel.go b/internal/therealssh/channel.go index eafa235bf..1d47dd357 100644 --- a/internal/therealssh/channel.go +++ b/internal/therealssh/channel.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "log" "net" "os" "os/user" @@ -149,7 +148,10 @@ func (sshCh *SSHChannel) SocketPath() string { // ServeSocket starts socket handling loop. func (sshCh *SSHChannel) ServeSocket() error { - os.Remove(sshCh.SocketPath()) + if err := os.Remove(sshCh.SocketPath()); err != nil { + log.WithError(err).Warn("Failed to remove SSH channel socket file") + } + debug("waiting for new socket connections on: %s", sshCh.SocketPath()) l, err := net.ListenUnix("unix", &net.UnixAddr{Name: sshCh.SocketPath(), Net: "unix"}) if err != nil { @@ -166,9 +168,15 @@ func (sshCh *SSHChannel) ServeSocket() error { debug("got new socket connection") defer func() { - conn.Close() - sshCh.closeListener() //nolint:errcheck - os.Remove(sshCh.SocketPath()) + if err := conn.Close(); err != nil { + log.WithError(err).Warn("Failed to close connection") + } + if err := sshCh.closeListener(); err != nil { + log.WithError(err).Warn("Failed to close listener") + } + if err := os.Remove(sshCh.SocketPath()); err != nil { + log.WithError(err).Warn("Failed to close SSH channel socket file") + } }() go func() { @@ -214,7 +222,7 @@ func (sshCh *SSHChannel) Start(command string) error { go func() { if err := sshCh.serveSession(); err != nil { - log.Println("Session failure:", err) + log.Error("Session failure:", err) } }() @@ -224,13 +232,17 @@ func (sshCh *SSHChannel) Start(command string) error { func (sshCh *SSHChannel) serveSession() error { defer func() { - sshCh.Send(CmdChannelServerClose, nil) // nolint - sshCh.Close() + if err := sshCh.Send(CmdChannelServerClose, nil); err != nil { + log.WithError(err).Warn("Failed to send to SSH channel") + } + if err := sshCh.Close(); err != nil { + log.WithError(err).Warn("Failed to close SSH channel") + } }() go func() { if _, err := io.Copy(sshCh.session, sshCh); err != nil { - log.Println("PTY copy: ", err) + log.Error("PTY copy: ", err) return } }() diff --git a/internal/therealssh/channel_test.go b/internal/therealssh/channel_test.go index 8cca04e2e..4fb7a1578 100644 --- a/internal/therealssh/channel_test.go +++ b/internal/therealssh/channel_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/skycoin/skywire/internal/testhelpers" "github.com/skycoin/skywire/pkg/app" ) @@ -97,7 +98,10 @@ func TestChannelServeSocket(t *testing.T) { assert.Equal(t, filepath.Join(os.TempDir(), "therealsshd-1"), ch.SocketPath()) - go func() { ch.ServeSocket() }() // nolint + serveErr := make(chan error, 1) + go func() { + serveErr <- ch.ServeSocket() + }() time.Sleep(100 * time.Millisecond) conn, err := net.DialUnix("unix", nil, &net.UnixAddr{Name: ch.SocketPath(), Net: "unix"}) @@ -122,4 +126,5 @@ func TestChannelServeSocket(t *testing.T) { assert.Equal(t, []byte("bar"), buf) require.NoError(t, ch.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErr)) } diff --git a/internal/therealssh/client.go b/internal/therealssh/client.go index 6d4130f8b..c7c1b738f 100644 --- a/internal/therealssh/client.go +++ b/internal/therealssh/client.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "log" "net" "net/rpc" "strings" @@ -73,7 +72,7 @@ func (c *Client) OpenChannel(remotePK cipher.PubKey) (localID uint32, sshCh *SSH go func() { if err := c.serveConn(conn); err != nil { - log.Println(err) + log.Error(err) } }() @@ -156,7 +155,9 @@ func (c *Client) Close() error { } for _, sshCh := range c.chans.dropAll() { - sshCh.Close() + if err := sshCh.Close(); err != nil { + log.WithError(err).Warn("Failed to close SSH channel") + } } return nil @@ -194,11 +195,11 @@ func (rpc *RPCClient) Exec(args *ExecArgs, socketPath *string) error { debug("requesting shell process") if args.CommandWithArgs == nil { if _, err := sshCh.Request(RequestShell, nil); err != nil { - return fmt.Errorf("Shell request failure: %s", err) + return fmt.Errorf("shell request failure: %s", err) } } else { if _, err := sshCh.Request(RequestExec, args.ToBinary()); err != nil { - return fmt.Errorf("Shell request failure: %s", err) + return fmt.Errorf("shell request failure: %s", err) } } @@ -207,7 +208,7 @@ func (rpc *RPCClient) Exec(args *ExecArgs, socketPath *string) error { debug("starting socket listener") waitCh <- true if err := sshCh.ServeSocket(); err != nil { - log.Println("Session failure:", err) + log.Error("Session failure:", err) } }() diff --git a/internal/therealssh/server.go b/internal/therealssh/server.go index 550b5cb22..4028b5e4e 100644 --- a/internal/therealssh/server.go +++ b/internal/therealssh/server.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "log" "net" "github.com/skycoin/dmsg/cipher" @@ -79,14 +78,16 @@ func (s *Server) OpenChannel(remoteAddr *app.Addr, remoteID uint32, conn net.Con debug("sending response") if err := channel.Send(CmdChannelOpenResponse, res); err != nil { - channel.Close() + if err := channel.Close(); err != nil { + log.WithError(err).Warn("Failed to close channel") + } return fmt.Errorf("channel response failure: %s", err) } go func() { debug("listening for channel requests") if err := channel.Serve(); err != nil { - log.Println("channel failure:", err) + log.Error("channel failure:", err) } }() @@ -102,7 +103,7 @@ func (s *Server) HandleRequest(remotePK cipher.PubKey, localID uint32, data []by if s.auth.Authorize(remotePK) != nil || channel.RemoteAddr.PubKey != remotePK { if err := channel.Send(CmdChannelResponse, responseUnauthorized); err != nil { - log.Println("failed to send response: ", err) + log.Error("failed to send response: ", err) } return nil } @@ -182,7 +183,9 @@ func (s *Server) Close() error { } for _, channel := range s.chans.dropAll() { - channel.Close() + if err := channel.Close(); err != nil { + log.WithError(err).Warn("Failed to close channel") + } } return nil diff --git a/internal/therealssh/server_test.go b/internal/therealssh/server_test.go index 96ed97169..d53508b4a 100644 --- a/internal/therealssh/server_test.go +++ b/internal/therealssh/server_test.go @@ -2,7 +2,6 @@ package therealssh import ( "encoding/binary" - "log" "net" "os" "testing" diff --git a/internal/therealssh/session.go b/internal/therealssh/session.go index eefc72649..752ee2d17 100644 --- a/internal/therealssh/session.go +++ b/internal/therealssh/session.go @@ -10,8 +10,11 @@ import ( "syscall" "github.com/kr/pty" + "github.com/skycoin/skycoin/src/util/logging" ) +var log = logging.MustGetLogger("therealssh") + // Session represents PTY sessions. Channel normally handles Session's lifecycle. type Session struct { pty, tty *os.File @@ -34,7 +37,9 @@ func OpenSession(user *user.User, sz *pty.Winsize) (s *Session, err error) { } if err = pty.Setsize(s.pty, sz); err != nil { - s.Close() + if closeErr := s.Close(); closeErr != nil { + log.WithError(closeErr).Warn("Failed to close session") + } err = fmt.Errorf("failed to set PTY size: %s", err) } @@ -43,7 +48,11 @@ func OpenSession(user *user.User, sz *pty.Winsize) (s *Session, err error) { // Start executes command on Session's PTY. func (s *Session) Start(command string) (err error) { - defer s.tty.Close() + defer func() { + if err := s.tty.Close(); err != nil { + log.WithError(err).Warn("Failed to close TTY") + } + }() if command == "shell" { if command, err = resolveShell(s.user); err != nil { @@ -52,7 +61,7 @@ func (s *Session) Start(command string) (err error) { } components := strings.Split(command, " ") - cmd := exec.Command(components[0], components[1:]...) // nolint + cmd := exec.Command(components[0], components[1:]...) // nolint:gosec cmd.Dir = s.user.HomeDir cmd.Stdout = s.tty cmd.Stdin = s.tty diff --git a/internal/therealssh/shell.go b/internal/therealssh/shell.go index d752f3678..cb823a123 100644 --- a/internal/therealssh/shell.go +++ b/internal/therealssh/shell.go @@ -10,7 +10,7 @@ import ( ) func resolveShell(u *user.User) (string, error) { - out, err := exec.Command("getent", "passwd", u.Uid).Output() // nolint + out, err := exec.Command("getent", "passwd", u.Uid).Output() if err != nil { return "", fmt.Errorf("getent failure: %s", err) } diff --git a/internal/therealssh/shell_darwin.go b/internal/therealssh/shell_darwin.go index b1dd41eae..8115aa9b9 100644 --- a/internal/therealssh/shell_darwin.go +++ b/internal/therealssh/shell_darwin.go @@ -9,7 +9,7 @@ import ( func resolveShell(u *user.User) (string, error) { dir := "Local/Default/Users/" + u.Username - out, err := exec.Command("dscl", "localhost", "-read", dir, "UserShell").Output() + out, err := exec.Command("dscl", "localhost", "-read", dir, "UserShell").Output() // nolint:gosec if err != nil { return "", err } @@ -18,7 +18,7 @@ func resolveShell(u *user.User) (string, error) { matched := re.FindStringSubmatch(string(out)) shell := matched[1] if shell == "" { - return "", fmt.Errorf("Invalid output: %s", string(out)) + return "", fmt.Errorf("invalid output: %s", string(out)) } return shell, nil diff --git a/pkg/app/app.go b/pkg/app/app.go index d0f40a270..def4da9a5 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -13,6 +13,8 @@ import ( "os/exec" "path/filepath" "sync" + + "github.com/skycoin/skycoin/src/util/logging" ) const ( @@ -23,6 +25,10 @@ const ( DefaultOut = uintptr(4) ) +var ( + log = logging.MustGetLogger("app") +) + // Config defines configuration parameters for App type Config struct { AppName string `json:"app-name"` @@ -52,7 +58,7 @@ func Command(config *Config, appsPath string, args []string) (net.Conn, *exec.Cm } binaryPath := filepath.Join(appsPath, fmt.Sprintf("%s.v%s", config.AppName, config.AppVersion)) - cmd := exec.Command(binaryPath, args...) + cmd := exec.Command(binaryPath, args...) // nolint:gosec cmd.ExtraFiles = []*os.File{clientConn.inFile, clientConn.outFile} return srvConn, cmd, nil @@ -77,7 +83,9 @@ func SetupFromPipe(config *Config, inFD, outFD uintptr) (*App, error) { go app.handleProto() if err := app.proto.Send(FrameInit, config, nil); err != nil { - app.Close() + if err := app.Close(); err != nil { + log.WithError(err).Warn("Failed to close app") + } return nil, fmt.Errorf("INIT handshake failed: %s", err) } @@ -103,8 +111,12 @@ func (app *App) Close() error { app.mu.Lock() for addr, conn := range app.conns { - app.proto.Send(FrameClose, &addr, nil) // nolint: errcheck - conn.Close() + if err := app.proto.Send(FrameClose, &addr, nil); err != nil { + log.WithError(err).Warn("Failed to send command frame") + } + if err := conn.Close(); err != nil { + log.WithError(err).Warn("Failed to close connection") + } } app.mu.Unlock() @@ -170,7 +182,11 @@ func (app *App) handleProto() { } func (app *App) serveConn(addr *LoopAddr, conn io.ReadWriteCloser) { - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + log.WithError(err).Warn("failed to close connection") + } + }() for { buf := make([]byte, 32*1024) @@ -187,7 +203,9 @@ func (app *App) serveConn(addr *LoopAddr, conn io.ReadWriteCloser) { app.mu.Lock() if _, ok := app.conns[*addr]; ok { - app.proto.Send(FrameClose, &addr, nil) // nolint: errcheck + if err := app.proto.Send(FrameClose, &addr, nil); err != nil { + log.WithError(err).Warn("Failed to send command frame") + } } delete(app.conns, *addr) app.mu.Unlock() diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index b22181aeb..70f25858c 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "io" - "log" "net" "os" "testing" @@ -14,6 +13,8 @@ import ( "github.com/skycoin/skycoin/src/util/logging" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/skycoin/skywire/internal/testhelpers" ) func TestMain(m *testing.M) { @@ -41,18 +42,22 @@ func TestAppDial(t *testing.T) { go app.handleProto() dataCh := make(chan []byte) - go proto.Serve(func(f Frame, p []byte) (interface{}, error) { // nolint: errcheck - if f == FrameCreateLoop { - return &Addr{PubKey: lpk, Port: 2}, nil - } + serveErrCh := make(chan error, 1) + go func() { + f := func(f Frame, p []byte) (interface{}, error) { + if f == FrameCreateLoop { + return &Addr{PubKey: lpk, Port: 2}, nil + } - if f == FrameClose { - go func() { dataCh <- p }() - return nil, nil - } + if f == FrameClose { + go func() { dataCh <- p }() + return nil, nil + } - return nil, errors.New("unexpected frame") - }) + return nil, errors.New("unexpected frame") + } + serveErrCh <- proto.Serve(f) + }() conn, err := app.Dial(&Addr{PubKey: rpk, Port: 3}) require.NoError(t, err) require.NotNil(t, conn) @@ -75,6 +80,7 @@ func TestAppDial(t *testing.T) { require.Len(t, app.conns, 0) app.mu.Unlock() require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } func TestAppAccept(t *testing.T) { @@ -85,7 +91,10 @@ func TestAppAccept(t *testing.T) { go app.handleProto() proto := NewProtocol(out) - go proto.Serve(nil) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- proto.Serve(nil) + }() connCh := make(chan net.Conn) errCh := make(chan error) @@ -95,7 +104,7 @@ func TestAppAccept(t *testing.T) { connCh <- conn }() - require.NoError(t, proto.Send(FrameConfirmLoop, [2]*Addr{&Addr{lpk, 2}, &Addr{rpk, 3}}, nil)) + require.NoError(t, proto.Send(FrameConfirmLoop, [2]*Addr{{lpk, 2}, {rpk, 3}}, nil)) require.NoError(t, <-errCh) conn := <-connCh @@ -110,7 +119,7 @@ func TestAppAccept(t *testing.T) { connCh <- conn }() - require.NoError(t, proto.Send(FrameConfirmLoop, [2]*Addr{&Addr{lpk, 2}, &Addr{rpk, 2}}, nil)) + require.NoError(t, proto.Send(FrameConfirmLoop, [2]*Addr{{lpk, 2}, {rpk, 2}}, nil)) require.NoError(t, <-errCh) conn = <-connCh @@ -118,6 +127,8 @@ func TestAppAccept(t *testing.T) { assert.Equal(t, rpk.Hex()+":2", conn.RemoteAddr().String()) assert.Equal(t, lpk.Hex()+":2", conn.LocalAddr().String()) require.Len(t, app.conns, 2) + require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } func TestAppWrite(t *testing.T) { @@ -130,15 +141,17 @@ func TestAppWrite(t *testing.T) { proto := NewProtocol(out) dataCh := make(chan []byte) + serveErrCh := make(chan error, 1) go func() { - proto.Serve(func(f Frame, p []byte) (interface{}, error) { // nolint: errcheck + f := func(f Frame, p []byte) (interface{}, error) { if f != FrameSend { return nil, errors.New("unexpected frame") } go func() { dataCh <- p }() return nil, nil - }) + } + serveErrCh <- proto.Serve(f) }() n, err := appOut.Write([]byte("foo")) @@ -153,6 +166,7 @@ func TestAppWrite(t *testing.T) { assert.Equal(t, []byte("foo"), packet.Payload) require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) require.NoError(t, appOut.Close()) } @@ -164,7 +178,10 @@ func TestAppRead(t *testing.T) { go app.handleProto() proto := NewProtocol(out) - go proto.Serve(nil) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- proto.Serve(nil) + }() errCh := make(chan error) go func() { @@ -180,6 +197,7 @@ func TestAppRead(t *testing.T) { require.NoError(t, <-errCh) require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) require.NoError(t, appOut.Close()) } @@ -187,19 +205,23 @@ func TestAppSetup(t *testing.T) { srvConn, clientConn, err := OpenPipeConn() require.NoError(t, err) - srvConn.SetDeadline(time.Now().Add(time.Second)) // nolint: errcheck - clientConn.SetDeadline(time.Now().Add(time.Second)) // nolint: errcheck + require.NoError(t, srvConn.SetDeadline(time.Now().Add(time.Second))) + require.NoError(t, clientConn.SetDeadline(time.Now().Add(time.Second))) proto := NewProtocol(srvConn) dataCh := make(chan []byte) - go proto.Serve(func(f Frame, p []byte) (interface{}, error) { // nolint: errcheck, unparam - if f != FrameInit { - return nil, errors.New("unexpected frame") - } + serveErrCh := make(chan error, 1) + go func() { + f := func(f Frame, p []byte) (interface{}, error) { + if f != FrameInit { + return nil, errors.New("unexpected frame") + } - go func() { dataCh <- p }() - return nil, nil - }) + go func() { dataCh <- p }() + return nil, nil + } + serveErrCh <- proto.Serve(f) + }() inFd, outFd := clientConn.Fd() _, err = SetupFromPipe(&Config{AppName: "foo", AppVersion: "0.0.1", ProtocolVersion: "0.0.1"}, inFd, outFd) @@ -212,6 +234,7 @@ func TestAppSetup(t *testing.T) { assert.Equal(t, "0.0.1", config.ProtocolVersion) require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } func TestAppCloseConn(t *testing.T) { @@ -222,7 +245,10 @@ func TestAppCloseConn(t *testing.T) { go app.handleProto() proto := NewProtocol(out) - go proto.Serve(nil) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- proto.Serve(nil) + }() errCh := make(chan error) go func() { @@ -232,6 +258,9 @@ func TestAppCloseConn(t *testing.T) { _, err := appOut.Read(make([]byte, 3)) require.Equal(t, io.EOF, err) require.Len(t, app.conns, 0) + + require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } func TestAppClose(t *testing.T) { @@ -243,15 +272,19 @@ func TestAppClose(t *testing.T) { proto := NewProtocol(out) dataCh := make(chan []byte) - go proto.Serve(func(f Frame, p []byte) (interface{}, error) { // nolint: errcheck, unparam - if f != FrameClose { - return nil, errors.New("unexpected frame") - } + serveErrCh := make(chan error, 1) + go func() { + f := func(f Frame, p []byte) (interface{}, error) { + if f != FrameClose { + return nil, errors.New("unexpected frame") + } - go func() { dataCh <- p }() - return nil, nil - }) + go func() { dataCh <- p }() + return nil, nil + } + serveErrCh <- proto.Serve(f) + }() require.NoError(t, app.Close()) _, err := appOut.Read(make([]byte, 3)) @@ -262,6 +295,9 @@ func TestAppClose(t *testing.T) { assert.Equal(t, uint16(2), addr.Port) assert.Equal(t, pk, addr.Remote.PubKey) assert.Equal(t, uint16(3), addr.Remote.Port) + + require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } func TestAppCommand(t *testing.T) { diff --git a/pkg/app/packet_test.go b/pkg/app/packet_test.go index 0020ae0ee..d4cea1a7c 100644 --- a/pkg/app/packet_test.go +++ b/pkg/app/packet_test.go @@ -15,7 +15,7 @@ func ExamplePacket() { fmt.Printf("%v\n", addr) fmt.Printf("%v\n", loopAddr) - //Output: skywire + // Output: skywire // {000000000000000000000000000000000000000000000000000000000000000000 0} // {0 {000000000000000000000000000000000000000000000000000000000000000000 0}} } diff --git a/pkg/app/protocol.go b/pkg/app/protocol.go index dcdd20cde..6c80784d2 100644 --- a/pkg/app/protocol.go +++ b/pkg/app/protocol.go @@ -109,17 +109,23 @@ func (p *Protocol) Serve(handleFunc func(Frame, []byte) (interface{}, error)) er go func() { if handleFunc == nil { - p.writeFrame(FrameSuccess, id, nil) // nolint: errcheck + if err := p.writeFrame(FrameSuccess, id, nil); err != nil { + log.WithError(err).Warn("Failed to write frame") + } return } res, err := handleFunc(fType, frame[2:]) if err != nil { - p.writeFrame(FrameFailure, id, err) // nolint: errcheck + if err := p.writeFrame(FrameFailure, id, err); err != nil { + log.WithError(err).Warn("Failed to write frame") + } return } - p.writeFrame(FrameSuccess, id, res) // nolint: errcheck + if err := p.writeFrame(FrameSuccess, id, res); err != nil { + log.WithError(err).Warn("Failed to write frame") + } }() } } diff --git a/pkg/httputil/httputil.go b/pkg/httputil/httputil.go index 635f0b52c..75e3fa62d 100644 --- a/pkg/httputil/httputil.go +++ b/pkg/httputil/httputil.go @@ -8,15 +8,18 @@ import ( "net/http" "github.com/gorilla/handlers" + "github.com/skycoin/skycoin/src/util/logging" ) +var log = logging.MustGetLogger("httputil") + // WriteJSON writes a json object on a http.ResponseWriter with the given code, // panics on marshaling error func WriteJSON(w http.ResponseWriter, r *http.Request, code int, v interface{}) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(code) enc := json.NewEncoder(w) - if pretty, _ := BoolFromQuery(r, "pretty", false); pretty { //nolint:errcheck + if pretty, err := BoolFromQuery(r, "pretty", false); err == nil && pretty { enc.SetIndent("", " ") } if err, ok := v.(error); ok { @@ -56,8 +59,11 @@ func WriteLog(writer io.Writer, params handlers.LogFormatterParams) { host = params.Request.RemoteAddr } - fmt.Fprintf( + _, err = fmt.Fprintf( writer, "%s - \"%s %s %s\" %d\n", host, params.Request.Method, params.URL.String(), params.Request.Proto, params.StatusCode, ) + if err != nil { + log.WithError(err).Warn("Failed to write log") + } } diff --git a/pkg/manager/config.go b/pkg/manager/config.go index d3bfe20de..e580b1590 100644 --- a/pkg/manager/config.go +++ b/pkg/manager/config.go @@ -92,7 +92,7 @@ func (c *Config) Parse(path string) error { if path, err = filepath.Abs(path); err != nil { return err } - f, err := os.Open(path) + f, err := os.Open(filepath.Clean(path)) if err != nil { return err } diff --git a/pkg/manager/node.go b/pkg/manager/node.go index c03fd31ec..66257b98a 100644 --- a/pkg/manager/node.go +++ b/pkg/manager/node.go @@ -93,7 +93,10 @@ type MockConfig struct { func (m *Node) AddMockData(config MockConfig) error { r := rand.New(rand.NewSource(time.Now().UnixNano())) for i := 0; i < config.Nodes; i++ { - pk, client := node.NewMockRPCClient(r, config.MaxTpsPerNode, config.MaxRoutesPerNode) + pk, client, err := node.NewMockRPCClient(r, config.MaxTpsPerNode, config.MaxRoutesPerNode) + if err != nil { + return err + } m.mu.Lock() m.nodes[pk] = appNodeConn{ Addr: &noise.Addr{ diff --git a/pkg/manager/user.go b/pkg/manager/user.go index 2438f1beb..bf3a24e71 100644 --- a/pkg/manager/user.go +++ b/pkg/manager/user.go @@ -102,7 +102,7 @@ func NewBoltUserStore(path string) (*BoltUserStore, error) { // User obtains a single user. Returns true if user exists. func (s *BoltUserStore) User(name string) (user User, ok bool) { - catch(s.View(func(tx *bbolt.Tx) error { //nolint:unparam + catch(s.View(func(tx *bbolt.Tx) error { users := tx.Bucket([]byte(boltUserBucketName)) rawUser := users.Get([]byte(name)) if rawUser == nil { diff --git a/pkg/manager/user_manager.go b/pkg/manager/user_manager.go index 7795a0bfd..4ff18a520 100644 --- a/pkg/manager/user_manager.go +++ b/pkg/manager/user_manager.go @@ -88,7 +88,7 @@ func (s *UserManager) Login() http.HandlerFunc { User: rb.Username, Expiry: time.Now().Add(s.c.ExpiresDuration), }) - //http.SetCookie() + // http.SetCookie() httputil.WriteJSON(w, r, http.StatusOK, ok) } } diff --git a/pkg/node/config.go b/pkg/node/config.go index 8184ec1e3..d216384a5 100644 --- a/pkg/node/config.go +++ b/pkg/node/config.go @@ -110,7 +110,7 @@ func (c *Config) RoutingTable() (routing.Table, error) { // AppsConfig decodes AppsConfig from a local json config file. func (c *Config) AppsConfig() ([]AppConfig, error) { - apps := []AppConfig{} + apps := make([]AppConfig, 0) for _, app := range c.Apps { if app.Version == "" { app.Version = c.Version diff --git a/pkg/node/config_test.go b/pkg/node/config_test.go index abc7a6a74..ed8a2618c 100644 --- a/pkg/node/config_test.go +++ b/pkg/node/config_test.go @@ -38,7 +38,7 @@ func TestMessagingDiscovery(t *testing.T) { func TestTransportDiscovery(t *testing.T) { pk, _ := cipher.GenerateKeyPair() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - json.NewEncoder(w).Encode(&httpauth.NextNonceResponse{Edge: pk, NextNonce: 1}) // nolint: errcheck + require.NoError(t, json.NewEncoder(w).Encode(&httpauth.NextNonceResponse{Edge: pk, NextNonce: 1})) })) defer srv.Close() @@ -53,7 +53,9 @@ func TestTransportDiscovery(t *testing.T) { func TestTransportLogStore(t *testing.T) { dir := filepath.Join(os.TempDir(), "foo") - defer os.RemoveAll(dir) + defer func() { + require.NoError(t, os.RemoveAll(dir)) + }() conf := Config{} conf.Transport.LogStore.Type = "file" @@ -72,7 +74,9 @@ func TestTransportLogStore(t *testing.T) { func TestRoutingTable(t *testing.T) { tmpfile, err := ioutil.TempFile("", "routing") require.NoError(t, err) - defer os.Remove(tmpfile.Name()) + defer func() { + require.NoError(t, os.Remove(tmpfile.Name())) + }() conf := Config{} conf.Routing.Table.Type = "boltdb" @@ -114,7 +118,9 @@ func TestAppsDir(t *testing.T) { dir, err := conf.AppsDir() require.NoError(t, err) - defer os.Remove(dir) + defer func() { + require.NoError(t, os.Remove(dir)) + }() _, err = os.Stat(dir) assert.NoError(t, err) @@ -125,7 +131,9 @@ func TestLocalDir(t *testing.T) { dir, err := conf.LocalDir() require.NoError(t, err) - defer os.Remove(dir) + defer func() { + require.NoError(t, os.Remove(dir)) + }() _, err = os.Stat(dir) assert.NoError(t, err) diff --git a/pkg/node/node.go b/pkg/node/node.go index d70655219..718272539 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -32,6 +32,8 @@ import ( "github.com/skycoin/skywire/pkg/util/pathutil" ) +var log = logging.MustGetLogger("node") + // AppStatus defines running status of an App. type AppStatus int @@ -265,18 +267,22 @@ func (node *Node) closePreviousApps() { node.logger.Info("killing previously ran apps if any...") pids := node.pidFile() - defer pids.Close() // nocheck: err + defer func() { + if err := pids.Close(); err != nil { + node.logger.Warnf("error closing PID file: %s", err) + } + }() scanner := bufio.NewScanner(pids) for scanner.Scan() { appInfo := strings.Split(scanner.Text(), " ") if len(appInfo) != 2 { - node.logger.Fatal("error parsing %s. Err: %s", pids.Name(), errors.New("line should be: [app name] [pid]")) + node.logger.Fatalf("error parsing %s. Err: %s", pids.Name(), errors.New("line should be: [app name] [pid]")) } pid, err := strconv.Atoi(appInfo[1]) if err != nil { - node.logger.Fatal("error parsing %s. Err: %s", pids.Name(), err) + node.logger.Fatalf("error parsing %s. Err: %s", pids.Name(), err) } node.stopUnhandledApp(appInfo[0], pid) @@ -341,7 +347,7 @@ func (node *Node) Close() (err error) { // Apps returns list of AppStates for all registered apps. func (node *Node) Apps() []*AppState { - res := []*AppState{} + res := make([]*AppState, 0) for _, app := range node.appsConf { state := &AppState{app.App, app.AutoStart, app.Port, AppStatusStopped} node.startedMu.RLock() @@ -376,7 +382,7 @@ func (node *Node) StartApp(appName string) error { } // SpawnApp configures and starts new App. -func (node *Node) SpawnApp(config *AppConfig, startCh chan<- struct{}) error { +func (node *Node) SpawnApp(config *AppConfig, startCh chan<- struct{}) (err error) { node.logger.Infof("Starting %s.v%s", config.App, config.Version) conn, cmd, err := app.Command( &app.Config{ProtocolVersion: supportedProtocolVersion, AppName: config.App, AppVersion: config.Version}, @@ -403,7 +409,11 @@ func (node *Node) SpawnApp(config *AppConfig, startCh chan<- struct{}) error { // TODO: make PackageLogger return *Entry. FieldLogger doesn't expose Writer. logger := node.logger.WithField("_module", fmt.Sprintf("%s.v%s", config.App, config.Version)).Writer() - defer logger.Close() + defer func() { + if logErr := logger.Close(); err == nil && logErr != nil { + err = logErr + } + }() cmd.Stdout = logger cmd.Stderr = logger @@ -464,7 +474,9 @@ func (node *Node) SpawnApp(config *AppConfig, startCh chan<- struct{}) error { func (node *Node) persistPID(name string, pid int) { pidF := node.pidFile() pidFName := pidF.Name() - pidF.Close() + if err := pidF.Close(); err != nil { + log.WithError(err).Warn("Failed to close PID file") + } pathutil.AtomicAppendToFile(pidFName, []byte(fmt.Sprintf("%s %d\n", name, pid))) } @@ -496,7 +508,7 @@ func (node *Node) SetAutoStart(appName string, autoStart bool) error { func (node *Node) stopApp(app string, bind *appBind) (err error) { node.logger.Infof("Stopping app %s and closing ports", app) - if excErr := node.executer.Stop(bind.pid); excErr != nil && err == nil { + if excErr := node.executer.Stop(bind.pid); excErr != nil { node.logger.Warn("Failed to stop app: ", excErr) err = excErr } diff --git a/pkg/node/node_test.go b/pkg/node/node_test.go index 59768e3e5..928592df3 100644 --- a/pkg/node/node_test.go +++ b/pkg/node/node_test.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "io/ioutil" - "log" "net" "net/http" "net/http/httptest" @@ -49,7 +48,7 @@ func TestMain(m *testing.M) { func TestNewNode(t *testing.T) { pk, sk := cipher.GenerateKeyPair() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - json.NewEncoder(w).Encode(&httpauth.NextNonceResponse{Edge: pk, NextNonce: 1}) // nolint: errcheck + require.NoError(t, json.NewEncoder(w).Encode(&httpauth.NextNonceResponse{Edge: pk, NextNonce: 1})) })) defer srv.Close() @@ -64,7 +63,9 @@ func TestNewNode(t *testing.T) { {App: "bar", AutoStart: true, Port: 2}, } - defer os.RemoveAll("local") + defer func() { + require.NoError(t, os.RemoveAll("local")) + }() node, err := NewNode(&conf, masterLogger) require.NoError(t, err) @@ -83,7 +84,11 @@ func TestNodeStartClose(t *testing.T) { {App: "skychat", Version: "1.0", AutoStart: true, Port: 1}, {App: "foo", Version: "1.0", AutoStart: false}, } - defer os.RemoveAll("skychat") + + defer func() { + require.NoError(t, os.RemoveAll("skychat")) + }() + node := &Node{config: &Config{}, router: r, executer: executer, appsConf: conf, startedApps: map[string]*appBind{}, logger: logging.MustGetLogger("test")} mConf := &dmsg.Config{PubKey: cipher.PubKey{}, SecKey: cipher.SecKey{}, Discovery: disc.NewMock()} @@ -114,13 +119,17 @@ func TestNodeSpawnApp(t *testing.T) { pk, _ := cipher.GenerateKeyPair() r := new(mockRouter) executer := &MockExecuter{} - defer os.RemoveAll("skychat") + defer func() { + require.NoError(t, os.RemoveAll("skychat")) + }() apps := []AppConfig{{App: "skychat", Version: "1.0", AutoStart: false, Port: 10, Args: []string{"foo"}}} node := &Node{router: r, executer: executer, appsConf: apps, startedApps: map[string]*appBind{}, logger: logging.MustGetLogger("test"), config: &Config{}} node.config.Node.StaticPubKey = pk pathutil.EnsureDir(node.dir()) - defer os.RemoveAll(node.dir()) + defer func() { + require.NoError(t, os.RemoveAll(node.dir())) + }() require.NoError(t, node.StartApp("skychat")) time.Sleep(100 * time.Millisecond) @@ -145,7 +154,9 @@ func TestNodeSpawnAppValidations(t *testing.T) { conn, _ := net.Pipe() r := new(mockRouter) executer := &MockExecuter{err: errors.New("foo")} - defer os.RemoveAll("skychat") + defer func() { + require.NoError(t, os.RemoveAll("skychat")) + }() node := &Node{router: r, executer: executer, startedApps: map[string]*appBind{"skychat": {conn, 10}}, logger: logging.MustGetLogger("test")} diff --git a/pkg/node/rpc_client.go b/pkg/node/rpc_client.go index 638d85722..091c9e354 100644 --- a/pkg/node/rpc_client.go +++ b/pkg/node/rpc_client.go @@ -178,7 +178,7 @@ type mockRPCClient struct { } // NewMockRPCClient creates a new mock RPCClient. -func NewMockRPCClient(r *rand.Rand, maxTps int, maxRules int) (cipher.PubKey, RPCClient) { +func NewMockRPCClient(r *rand.Rand, maxTps int, maxRules int) (cipher.PubKey, RPCClient, error) { log := logging.MustGetLogger("mock-rpc-client") types := []string{"messaging", "native"} @@ -203,8 +203,12 @@ func NewMockRPCClient(r *rand.Rand, maxTps int, maxRules int) (cipher.PubKey, RP for i := 0; i < r.Intn(maxRules+1); i++ { remotePK, _ := cipher.GenerateKeyPair() var lpRaw, rpRaw [2]byte - r.Read(lpRaw[:]) - r.Read(rpRaw[:]) + if _, err := r.Read(lpRaw[:]); err != nil { + return cipher.PubKey{}, nil, err + } + if _, err := r.Read(rpRaw[:]); err != nil { + return cipher.PubKey{}, nil, err + } lp := binary.BigEndian.Uint16(lpRaw[:]) rp := binary.BigEndian.Uint16(rpRaw[:]) fwdRule := routing.ForwardRule(ruleExp, routing.RouteID(r.Uint32()), uuid.New()) @@ -221,7 +225,7 @@ func NewMockRPCClient(r *rand.Rand, maxTps int, maxRules int) (cipher.PubKey, RP log.Infof("rt[%2db]: %v %v", i, appRID, appRule.Summary().AppFields) } log.Printf("rtCount: %d", rt.Count()) - return localPK, &mockRPCClient{ + client := &mockRPCClient{ s: &Summary{ PubKey: localPK, NodeVersion: Version, @@ -236,6 +240,7 @@ func NewMockRPCClient(r *rand.Rand, maxTps int, maxRules int) (cipher.PubKey, RP tpTypes: types, rt: rt, } + return localPK, client, nil } func (mc *mockRPCClient) do(write bool, f func() error) error { diff --git a/pkg/node/rpc_test.go b/pkg/node/rpc_test.go index b8ec54c11..ec26c2a3a 100644 --- a/pkg/node/rpc_test.go +++ b/pkg/node/rpc_test.go @@ -52,13 +52,17 @@ func TestStartStopApp(t *testing.T) { pk, _ := cipher.GenerateKeyPair() router := new(mockRouter) executer := new(MockExecuter) - defer os.RemoveAll("skychat") + defer func() { + require.NoError(t, os.RemoveAll("skychat")) + }() apps := []AppConfig{{App: "foo", Version: "1.0", AutoStart: false, Port: 10}} node := &Node{router: router, executer: executer, appsConf: apps, startedApps: map[string]*appBind{}, logger: logging.MustGetLogger("test"), config: &Config{}} node.config.Node.StaticPubKey = pk pathutil.EnsureDir(node.dir()) - defer os.RemoveAll(node.dir()) + defer func() { + require.NoError(t, os.RemoveAll(node.dir())) + }() rpc := &RPC{node: node} unknownApp := "bar" @@ -95,7 +99,9 @@ func TestStartStopApp(t *testing.T) { func TestRPC(t *testing.T) { r := new(mockRouter) executer := new(MockExecuter) - defer os.RemoveAll("skychat") + defer func() { + require.NoError(t, os.RemoveAll("skychat")) + }() pk1, _, tm1, tm2, errCh, err := transport.MockTransportManagersPair() require.NoError(t, err) @@ -126,7 +132,11 @@ func TestRPC(t *testing.T) { logger: logging.MustGetLogger("test"), } pathutil.EnsureDir(node.dir()) - defer os.RemoveAll(node.dir()) + defer func() { + if err := os.RemoveAll(node.dir()); err != nil { + log.WithError(err).Warn(err) + } + }() require.NoError(t, node.StartApp("foo")) require.NoError(t, node.StartApp("bar")) @@ -144,9 +154,9 @@ func TestRPC(t *testing.T) { require.NoError(t, svr.RegisterName(RPCPrefix, gateway)) go svr.ServeConn(sConn) - //client := RPCClient{Client: rpc.NewClient(cConn)} + // client := RPCClient{Client: rpc.NewClient(cConn)} - print := func(t *testing.T, name string, v interface{}) { + printFunc := func(t *testing.T, name string, v interface{}) { j, err := json.MarshalIndent(v, name+": ", " ") require.NoError(t, err) t.Log(string(j)) @@ -157,45 +167,45 @@ func TestRPC(t *testing.T) { assert.Equal(t, pk1, summary.PubKey) assert.Len(t, summary.Apps, 2) assert.Len(t, summary.Transports, 1) - print(t, "Summary", summary) + printFunc(t, "Summary", summary) } t.Run("RPCServer", func(t *testing.T) { var summary Summary require.NoError(t, gateway.Summary(&struct{}{}, &summary)) test(t, &summary) }) - //t.Run("RPCClient", func(t *testing.T) { + // t.Run("RPCClient", func(t *testing.T) { // summary, err := client.Summary() // require.NoError(t, err) // test(t, summary) - //}) + // }) }) t.Run("Apps", func(t *testing.T) { test := func(t *testing.T, apps []*AppState) { assert.Len(t, apps, 2) - print(t, "Apps", apps) + printFunc(t, "Apps", apps) } t.Run("RPCServer", func(t *testing.T) { var apps []*AppState require.NoError(t, gateway.Apps(&struct{}{}, &apps)) test(t, apps) }) - //t.Run("RPCClient", func(t *testing.T) { + // t.Run("RPCClient", func(t *testing.T) { // apps, err := client.Apps() // require.NoError(t, err) // test(t, apps) - //}) + // }) }) // TODO(evanlinjin): For some reason, this freezes. - //t.Run("StopStartApp", func(t *testing.T) { + // t.Run("StopStartApp", func(t *testing.T) { // appName := "foo" // require.NoError(t, gateway.StopApp(&appName, &struct{}{})) // require.NoError(t, gateway.StartApp(&appName, &struct{}{})) // require.NoError(t, client.StopApp(appName)) // require.NoError(t, client.StartApp(appName)) - //}) + // }) t.Run("SetAutoStart", func(t *testing.T) { unknownAppName := "whoAmI" @@ -219,15 +229,15 @@ func TestRPC(t *testing.T) { // Test with RPC Client - //err = client.SetAutoStart(in1.AppName, in1.AutoStart) - //require.Error(t, err) - //assert.Equal(t, ErrUnknownApp.Error(), err.Error()) + // err = client.SetAutoStart(in1.AppName, in1.AutoStart) + // require.Error(t, err) + // assert.Equal(t, ErrUnknownApp.Error(), err.Error()) // - //require.NoError(t, client.SetAutoStart(in2.AppName, in2.AutoStart)) - //assert.True(t, node.appsConf[0].AutoStart) + // require.NoError(t, client.SetAutoStart(in2.AppName, in2.AutoStart)) + // assert.True(t, node.appsConf[0].AutoStart) // - //require.NoError(t, client.SetAutoStart(in3.AppName, in3.AutoStart)) - //assert.False(t, node.appsConf[0].AutoStart) + // require.NoError(t, client.SetAutoStart(in3.AppName, in3.AutoStart)) + // assert.False(t, node.appsConf[0].AutoStart) }) t.Run("TransportTypes", func(t *testing.T) { @@ -238,9 +248,9 @@ func TestRPC(t *testing.T) { assert.Len(t, out, 1) assert.Equal(t, "mock", out[0].Type) - //out2, err := client.Transports(in.FilterTypes, in.FilterPubKeys, in.ShowLogs) - //require.NoError(t, err) - //assert.Equal(t, out, out2) + // out2, err := client.Transports(in.FilterTypes, in.FilterPubKeys, in.ShowLogs) + // require.NoError(t, err) + // assert.Equal(t, out, out2) }) t.Run("Transport", func(t *testing.T) { @@ -254,9 +264,9 @@ func TestRPC(t *testing.T) { var summary TransportSummary require.NoError(t, gateway.Transport(&id, &summary)) - //summary2, err := client.Transport(id) - //require.NoError(t, err) - //require.Equal(t, summary, *summary2) + // summary2, err := client.Transport(id) + // require.NoError(t, err) + // require.Equal(t, summary, *summary2) } }) diff --git a/pkg/route-finder/client/client.go b/pkg/route-finder/client/client.go index fea92a846..96439bf0c 100644 --- a/pkg/route-finder/client/client.go +++ b/pkg/route-finder/client/client.go @@ -12,12 +12,15 @@ import ( "time" "github.com/skycoin/dmsg/cipher" + "github.com/skycoin/skycoin/src/util/logging" "github.com/skycoin/skywire/pkg/routing" ) const defaultContextTimeout = 10 * time.Second +var log = logging.MustGetLogger("route-finder") + // GetRoutesRequest parses json body for /routes endpoint request type GetRoutesRequest struct { SrcPK cipher.PubKey `json:"src_pk,omitempty"` @@ -93,6 +96,13 @@ func (c *apiClient) PairedRoutes(source, destiny cipher.PubKey, minHops, maxHops req = req.WithContext(ctx) res, err := c.client.Do(req) + if res != nil { + defer func() { + if err := res.Body.Close(); err != nil { + log.WithError(err).Warn("Failed to close HTTP response body") + } + }() + } if err != nil { return nil, nil, err } @@ -104,7 +114,6 @@ func (c *apiClient) PairedRoutes(source, destiny cipher.PubKey, minHops, maxHops if err != nil { return nil, nil, err } - defer res.Body.Close() return nil, nil, errors.New(apiErr.Error.Message) } @@ -114,7 +123,6 @@ func (c *apiClient) PairedRoutes(source, destiny cipher.PubKey, minHops, maxHops if err != nil { return nil, nil, err } - defer res.Body.Close() return routes.Forward, routes.Reverse, nil } diff --git a/pkg/router/app_manager_test.go b/pkg/router/app_manager_test.go index 041866db2..aa243e3d8 100644 --- a/pkg/router/app_manager_test.go +++ b/pkg/router/app_manager_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/skycoin/skywire/internal/testhelpers" "github.com/skycoin/skywire/pkg/app" ) @@ -25,7 +26,10 @@ func TestAppManagerInit(t *testing.T) { go func() { srvCh <- am.Serve() }() proto := app.NewProtocol(in) - go proto.Serve(nil) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- proto.Serve(nil) + }() tcs := []struct { conf *app.Config @@ -49,6 +53,8 @@ func TestAppManagerInit(t *testing.T) { require.NoError(t, in.Close()) require.NoError(t, <-srvCh) + require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } func TestAppManagerSetupLoop(t *testing.T) { @@ -68,7 +74,10 @@ func TestAppManagerSetupLoop(t *testing.T) { go func() { srvCh <- am.Serve() }() proto := app.NewProtocol(in) - go proto.Serve(nil) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- proto.Serve(nil) + }() var laddr *app.Addr pk, _ := cipher.GenerateKeyPair() @@ -79,6 +88,8 @@ func TestAppManagerSetupLoop(t *testing.T) { require.NoError(t, in.Close()) require.NoError(t, <-srvCh) + require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } func TestAppManagerCloseLoop(t *testing.T) { @@ -100,7 +111,10 @@ func TestAppManagerCloseLoop(t *testing.T) { go func() { srvCh <- am.Serve() }() proto := app.NewProtocol(in) - go proto.Serve(nil) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- proto.Serve(nil) + }() pk, _ := cipher.GenerateKeyPair() addr := &app.LoopAddr{Port: 2, Remote: app.Addr{PubKey: pk, Port: 3}} @@ -110,6 +124,8 @@ func TestAppManagerCloseLoop(t *testing.T) { require.NoError(t, in.Close()) require.NoError(t, <-srvCh) + require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } func TestAppManagerForward(t *testing.T) { @@ -131,7 +147,10 @@ func TestAppManagerForward(t *testing.T) { go func() { srvCh <- am.Serve() }() proto := app.NewProtocol(in) - go proto.Serve(nil) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- proto.Serve(nil) + }() pk, _ := cipher.GenerateKeyPair() packet := &app.Packet{Payload: []byte("foo"), Addr: &app.LoopAddr{Port: 2, Remote: app.Addr{PubKey: pk, Port: 3}}} @@ -141,4 +160,6 @@ func TestAppManagerForward(t *testing.T) { require.NoError(t, in.Close()) require.NoError(t, <-srvCh) + require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } diff --git a/pkg/router/managed_routing_table.go b/pkg/router/managed_routing_table.go index 86a8f141a..91362f1b5 100644 --- a/pkg/router/managed_routing_table.go +++ b/pkg/router/managed_routing_table.go @@ -31,7 +31,7 @@ func (rt *managedRoutingTable) Rule(routeID routing.RouteID) (routing.Rule, erro } func (rt *managedRoutingTable) Cleanup() error { - expiredIDs := []routing.RouteID{} + expiredIDs := make([]routing.RouteID, 0) rt.mu.Lock() err := rt.RangeRules(func(routeID routing.RouteID, rule routing.Rule) bool { if rule.Expiry().Before(time.Now()) { diff --git a/pkg/router/port_manager.go b/pkg/router/port_manager.go index b5d201968..cef42774f 100644 --- a/pkg/router/port_manager.go +++ b/pkg/router/port_manager.go @@ -40,7 +40,7 @@ func (pm *portManager) SetLoop(port uint16, raddr *app.Addr, l *loop) error { } func (pm *portManager) AppConns() []*app.Protocol { - res := []*app.Protocol{} + res := make([]*app.Protocol, 0) set := map[*app.Protocol]struct{}{} for _, bind := range pm.ports.all() { if _, ok := set[bind.conn]; !ok { @@ -52,7 +52,7 @@ func (pm *portManager) AppConns() []*app.Protocol { } func (pm *portManager) AppPorts(appConn *app.Protocol) []uint16 { - res := []uint16{} + res := make([]uint16, 0) for port, bind := range pm.ports.all() { if bind.conn == appConn { res = append(res, port) diff --git a/pkg/router/route_manager.go b/pkg/router/route_manager.go index e489c4606..e0fc8df68 100644 --- a/pkg/router/route_manager.go +++ b/pkg/router/route_manager.go @@ -106,7 +106,7 @@ func (rm *routeManager) Serve(rw io.ReadWriter) error { } func (rm *routeManager) addRoutingRules(data []byte) ([]routing.RouteID, error) { - rules := []routing.Rule{} + var rules []routing.Rule if err := json.Unmarshal(data, &rules); err != nil { return nil, err } @@ -126,7 +126,7 @@ func (rm *routeManager) addRoutingRules(data []byte) ([]routing.RouteID, error) } func (rm *routeManager) deleteRoutingRules(data []byte) ([]routing.RouteID, error) { - ruleIDs := []routing.RouteID{} + var ruleIDs []routing.RouteID if err := json.Unmarshal(data, &ruleIDs); err != nil { return nil, err } diff --git a/pkg/router/router.go b/pkg/router/router.go index 6f4dbeaa3..daa5996a9 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -28,6 +28,8 @@ const ( maxHops = 50 ) +var log = logging.MustGetLogger("router") + // Config configures Router. type Config struct { Logger *logging.Logger @@ -97,7 +99,11 @@ func (r *Router) Serve(ctx context.Context) error { } go func(tp transport.Transport) { - defer tp.Close() + defer func() { + if err := tp.Close(); err != nil { + r.Logger.Warnf("Failed to close transport: %s", err) + } + }() for { if err := serve(tp); err != nil { if err != io.EOF { @@ -146,7 +152,9 @@ func (r *Router) ServeApp(conn net.Conn, port uint16, appConf *app.Config) error for _, port := range r.pm.AppPorts(appProto) { for _, addr := range r.pm.Close(port) { - r.closeLoop(appProto, &app.LoopAddr{Port: port, Remote: addr}) // nolint: errcheck + if err := r.closeLoop(appProto, &app.LoopAddr{Port: port, Remote: addr}); err != nil { + log.WithError(err).Warn("Failed to close loop") + } } } @@ -170,7 +178,9 @@ func (r *Router) Close() error { r.expiryTicker.Stop() for _, conn := range r.pm.AppConns() { - conn.Close() + if err := conn.Close(); err != nil { + log.WithError(err).Warn("Failed to close connection") + } } r.wg.Wait() @@ -220,7 +230,10 @@ func (r *Router) consumePacket(payload []byte, rule routing.Rule) error { raddr := &app.Addr{PubKey: rule.RemotePK(), Port: rule.RemotePort()} p := &app.Packet{Addr: &app.LoopAddr{Port: rule.LocalPort(), Remote: *raddr}, Payload: payload} - b, _ := r.pm.Get(rule.LocalPort()) // nolint: errcheck + b, err := r.pm.Get(rule.LocalPort()) + if err != nil { + return err + } if err := b.conn.Send(app.FrameSend, p, nil); err != nil { return err } @@ -294,7 +307,11 @@ func (r *Router) requestLoop(appConn *app.Protocol, raddr *app.Addr) (*app.Addr, if err != nil { return nil, err } - defer tr.Close() + defer func() { + if err := tr.Close(); err != nil { + r.Logger.Warnf("Failed to close transport: %s", err) + } + }() if err := setup.CreateLoop(proto, l); err != nil { return nil, fmt.Errorf("route setup: %s", err) @@ -328,7 +345,7 @@ func (r *Router) confirmLoop(addr *app.LoopAddr, rule routing.Rule) error { return err } - addrs := [2]*app.Addr{&app.Addr{PubKey: r.config.PubKey, Port: addr.Port}, &addr.Remote} + addrs := [2]*app.Addr{{PubKey: r.config.PubKey, Port: addr.Port}, &addr.Remote} if err = b.conn.Send(app.FrameConfirmLoop, addrs, nil); err != nil { r.Logger.Warnf("Failed to notify App about new loop: %s", err) } @@ -345,7 +362,11 @@ func (r *Router) closeLoop(appConn *app.Protocol, addr *app.LoopAddr) error { if err != nil { return err } - defer tr.Close() + defer func() { + if err := tr.Close(); err != nil { + r.Logger.Warnf("Failed to close transport: %s", err) + } + }() ld := &setup.LoopData{RemotePK: addr.Remote.PubKey, RemotePort: addr.Remote.Port, LocalPort: addr.Port} if err := setup.CloseLoop(proto, ld); err != nil { @@ -380,7 +401,9 @@ func (r *Router) destroyLoop(addr *app.LoopAddr) error { r.mu.Unlock() if ok { - r.pm.RemoveLoop(addr.Port, &addr.Remote) // nolint: errcheck + if err := r.pm.RemoveLoop(addr.Port, &addr.Remote); err != nil { + log.WithError(err).Warn("Failed to remove loop") + } } else { r.pm.Close(addr.Port) } diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index 0fa8baabc..6b941e3c1 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "log" "net" "os" "testing" @@ -16,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/skycoin/skywire/internal/testhelpers" "github.com/skycoin/skywire/pkg/app" routeFinder "github.com/skycoin/skywire/pkg/route-finder/client" "github.com/skycoin/skywire/pkg/routing" @@ -133,12 +133,16 @@ func TestRouterAppInit(t *testing.T) { }() proto := app.NewProtocol(rw) - go proto.Serve(nil) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- proto.Serve(nil) + }() require.NoError(t, proto.Send(app.FrameInit, &app.Config{AppName: "foo", AppVersion: "0.0.1", ProtocolVersion: "0.0.1"}, nil)) require.Error(t, proto.Send(app.FrameInit, &app.Config{AppName: "foo1", AppVersion: "0.0.1", ProtocolVersion: "0.0.1"}, nil)) require.NoError(t, proto.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) require.NoError(t, r.Close()) require.NoError(t, <-errCh) } @@ -160,7 +164,10 @@ func TestRouterApp(t *testing.T) { m2, err := transport.NewManager(c2, f2) require.NoError(t, err) - go m2.Serve(context.TODO()) // nolint + trServeErrCh := make(chan error, 1) + go func() { + trServeErrCh <- m2.Serve(context.TODO()) + }() rt := routing.InMemoryRoutingTable() conf := &Config{ @@ -177,13 +184,20 @@ func TestRouterApp(t *testing.T) { }() rw, rwIn := net.Pipe() - go r.ServeApp(rwIn, 6, &app.Config{}) // nolint: errcheck + serveAppErrCh := make(chan error, 1) + go func() { + serveAppErrCh <- r.ServeApp(rwIn, 6, &app.Config{}) + }() proto := app.NewProtocol(rw) dataCh := make(chan []byte) - go proto.Serve(func(_ app.Frame, p []byte) (interface{}, error) { // nolint: errcheck,unparam - go func() { dataCh <- p }() - return nil, nil - }) + protoServeErrCh := make(chan error, 1) + go func() { + f := func(_ app.Frame, p []byte) (interface{}, error) { + go func() { dataCh <- p }() + return nil, nil + } + protoServeErrCh <- proto.Serve(f) + }() time.Sleep(100 * time.Millisecond) @@ -198,7 +212,10 @@ func TestRouterApp(t *testing.T) { require.NoError(t, r.pm.SetLoop(6, raddr, &loop{tr.Entry.ID, 4})) tr2 := m2.Transport(tr.Entry.ID) - go proto.Send(app.FrameSend, &app.Packet{Addr: &app.LoopAddr{Port: 6, Remote: *raddr}, Payload: []byte("bar")}, nil) // nolint: errcheck + sendErrCh := make(chan error, 1) + go func() { + sendErrCh <- proto.Send(app.FrameSend, &app.Packet{Addr: &app.LoopAddr{Port: 6, Remote: *raddr}, Payload: []byte("bar")}, nil) + }() packet := make(routing.Packet, 9) _, err = tr2.Read(packet) @@ -221,6 +238,12 @@ func TestRouterApp(t *testing.T) { require.NoError(t, r.Close()) require.NoError(t, <-errCh) + + require.NoError(t, m2.Close()) + require.NoError(t, testhelpers.NoErrorWithinTimeout(trServeErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(protoServeErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(sendErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveAppErrCh)) } func TestRouterLocalApp(t *testing.T) { @@ -245,20 +268,39 @@ func TestRouterLocalApp(t *testing.T) { }() rw1, rw1In := net.Pipe() - go r.ServeApp(rw1In, 5, &app.Config{}) // nolint: errcheck + serveAppErr1Ch := make(chan error, 1) + go func() { + serveAppErr1Ch <- r.ServeApp(rw1In, 5, &app.Config{}) + }() proto1 := app.NewProtocol(rw1) - go proto1.Serve(nil) // nolint: errcheck + protoServeErr1Ch := make(chan error, 1) + go func() { + protoServeErr1Ch <- proto1.Serve(nil) + }() rw2, rw2In := net.Pipe() - go r.ServeApp(rw2In, 6, &app.Config{}) // nolint: errcheck + serveAppErr2Ch := make(chan error, 1) + go func() { + serveAppErr2Ch <- r.ServeApp(rw2In, 6, &app.Config{}) + }() proto2 := app.NewProtocol(rw2) dataCh := make(chan []byte) - go proto2.Serve(func(_ app.Frame, p []byte) (interface{}, error) { // nolint: errcheck,unparam - go func() { dataCh <- p }() - return nil, nil - }) + protoServeErr2Ch := make(chan error, 1) + go func() { + f := func(_ app.Frame, p []byte) (interface{}, error) { + go func() { dataCh <- p }() + return nil, nil + } + protoServeErr2Ch <- proto2.Serve(f) + }() - go proto1.Send(app.FrameSend, &app.Packet{Addr: &app.LoopAddr{Port: 5, Remote: app.Addr{PubKey: pk, Port: 6}}, Payload: []byte("foo")}, nil) // nolint: errcheck + sendErrCh := make(chan error, 1) + go func() { + packet := &app.Packet{ + Addr: &app.LoopAddr{Port: 5, Remote: app.Addr{PubKey: pk, Port: 6}}, Payload: []byte("foo"), + } + sendErrCh <- proto1.Send(app.FrameSend, packet, nil) + }() time.Sleep(100 * time.Millisecond) @@ -272,6 +314,12 @@ func TestRouterLocalApp(t *testing.T) { require.NoError(t, r.Close()) require.NoError(t, <-errCh) + + require.NoError(t, testhelpers.NoErrorWithinTimeout(protoServeErr1Ch)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(protoServeErr2Ch)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(sendErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveAppErr1Ch)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveAppErr2Ch)) } func TestRouterSetup(t *testing.T) { @@ -311,21 +359,35 @@ func TestRouterSetup(t *testing.T) { sProto := setup.NewSetupProtocol(tr) rw1, rwIn1 := net.Pipe() - go r.ServeApp(rwIn1, 2, &app.Config{}) // nolint: errcheck + serveAppErr1Ch := make(chan error, 1) + go func() { + serveAppErr1Ch <- r.ServeApp(rwIn1, 2, &app.Config{}) + }() appProto1 := app.NewProtocol(rw1) dataCh := make(chan []byte) - go appProto1.Serve(func(_ app.Frame, p []byte) (interface{}, error) { // nolint: errcheck,unparam - go func() { dataCh <- p }() - return nil, nil - }) + protoServeErr1Ch := make(chan error, 1) + go func() { + f := func(_ app.Frame, p []byte) (interface{}, error) { + go func() { dataCh <- p }() + return nil, nil + } + protoServeErr1Ch <- appProto1.Serve(f) + }() rw2, rwIn2 := net.Pipe() - go r.ServeApp(rwIn2, 4, &app.Config{}) // nolint: errcheck + serveAppErr2Ch := make(chan error, 1) + go func() { + serveAppErr2Ch <- r.ServeApp(rwIn2, 4, &app.Config{}) + }() appProto2 := app.NewProtocol(rw2) - go appProto2.Serve(func(_ app.Frame, p []byte) (interface{}, error) { // nolint: errcheck,unparam - go func() { dataCh <- p }() - return nil, nil - }) + protoServeErr2Ch := make(chan error, 1) + go func() { + f := func(_ app.Frame, p []byte) (interface{}, error) { + go func() { dataCh <- p }() + return nil, nil + } + protoServeErr2Ch <- appProto2.Serve(f) + }() var routeID routing.RouteID t.Run("add route", func(t *testing.T) { @@ -420,6 +482,11 @@ func TestRouterSetup(t *testing.T) { require.NoError(t, err) assert.Nil(t, rule) }) + + require.NoError(t, testhelpers.NoErrorWithinTimeout(protoServeErr1Ch)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(protoServeErr2Ch)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveAppErr1Ch)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveAppErr2Ch)) } func TestRouterSetupLoop(t *testing.T) { @@ -438,7 +505,10 @@ func TestRouterSetupLoop(t *testing.T) { m2, err := transport.NewManager(&transport.ManagerConfig{PubKey: pk2, SecKey: sk2, DiscoveryClient: client, LogStore: logStore}, f2) require.NoError(t, err) - go m2.Serve(context.TODO()) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- m2.Serve(context.TODO()) + }() conf := &Config{ Logger: logging.MustGetLogger("routesetup"), @@ -486,9 +556,15 @@ func TestRouterSetupLoop(t *testing.T) { }() rw, rwIn := net.Pipe() - go r.ServeApp(rwIn, 5, &app.Config{}) // nolint: errcheck + serveAppErrCh := make(chan error, 1) + go func() { + serveAppErrCh <- r.ServeApp(rwIn, 5, &app.Config{}) + }() appProto := app.NewProtocol(rw) - go appProto.Serve(nil) // nolint: errcheck + protoServeErrCh := make(chan error, 1) + go func() { + protoServeErrCh <- appProto.Serve(nil) + }() addr := &app.Addr{} require.NoError(t, appProto.Send(app.FrameCreateLoop, &app.Addr{PubKey: pk2, Port: 6}, addr)) @@ -500,6 +576,10 @@ func TestRouterSetupLoop(t *testing.T) { assert.Equal(t, pk1, addr.PubKey) assert.Equal(t, uint16(10), addr.Port) + + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveAppErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(protoServeErrCh)) } func TestRouterSetupLoopLocal(t *testing.T) { @@ -512,9 +592,15 @@ func TestRouterSetupLoopLocal(t *testing.T) { r := New(conf) rw, rwIn := net.Pipe() - go r.ServeApp(rwIn, 5, &app.Config{}) // nolint: errcheck + serveAppErrCh := make(chan error, 1) + go func() { + serveAppErrCh <- r.ServeApp(rwIn, 5, &app.Config{}) + }() proto := app.NewProtocol(rw) - go proto.Serve(nil) // nolint: errcheck + protoServeErrCh := make(chan error, 1) + go func() { + protoServeErrCh <- proto.Serve(nil) + }() addr := &app.Addr{} require.NoError(t, proto.Send(app.FrameCreateLoop, &app.Addr{PubKey: pk, Port: 5}, addr)) @@ -525,6 +611,9 @@ func TestRouterSetupLoopLocal(t *testing.T) { assert.Equal(t, pk, addr.PubKey) assert.Equal(t, uint16(10), addr.Port) + + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveAppErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(protoServeErrCh)) } func TestRouterCloseLoop(t *testing.T) { @@ -543,7 +632,10 @@ func TestRouterCloseLoop(t *testing.T) { m2, err := transport.NewManager(&transport.ManagerConfig{PubKey: pk2, SecKey: sk2, DiscoveryClient: client, LogStore: logStore}, f2) require.NoError(t, err) - go m2.Serve(context.TODO()) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- m2.Serve(context.TODO()) + }() rt := routing.InMemoryRoutingTable() rule := routing.AppRule(time.Now().Add(time.Hour), 4, pk3, 6, 5) @@ -597,9 +689,15 @@ func TestRouterCloseLoop(t *testing.T) { }() rw, rwIn := net.Pipe() - go r.ServeApp(rwIn, 5, &app.Config{}) // nolint: errcheck + serveAppErrCh := make(chan error, 1) + go func() { + serveAppErrCh <- r.ServeApp(rwIn, 5, &app.Config{}) + }() proto := app.NewProtocol(rw) - go proto.Serve(nil) // nolint: errcheck + protoServeErrCh := make(chan error, 1) + go func() { + protoServeErrCh <- proto.Serve(nil) + }() time.Sleep(100 * time.Millisecond) @@ -619,6 +717,10 @@ func TestRouterCloseLoop(t *testing.T) { rule, err = rt.Rule(routeID) require.NoError(t, err) require.Nil(t, rule) + + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveAppErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(protoServeErrCh)) } func TestRouterCloseLoopOnAppClose(t *testing.T) { @@ -637,7 +739,10 @@ func TestRouterCloseLoopOnAppClose(t *testing.T) { m2, err := transport.NewManager(&transport.ManagerConfig{PubKey: pk2, SecKey: sk2, DiscoveryClient: client, LogStore: logStore}, f2) require.NoError(t, err) - go m2.Serve(context.TODO()) // nolint: errcheck + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- m2.Serve(context.TODO()) + }() rt := routing.InMemoryRoutingTable() rule := routing.AppRule(time.Now().Add(time.Hour), 4, pk3, 6, 5) @@ -689,9 +794,15 @@ func TestRouterCloseLoopOnAppClose(t *testing.T) { }() rw, rwIn := net.Pipe() - go r.ServeApp(rwIn, 5, &app.Config{}) // nolint: errcheck + serveAppErrCh := make(chan error, 1) + go func() { + serveAppErrCh <- r.ServeApp(rwIn, 5, &app.Config{}) + }() proto := app.NewProtocol(rw) - go proto.Serve(nil) // nolint: errcheck + protoServeErrCh := make(chan error, 1) + go func() { + protoServeErrCh <- proto.Serve(nil) + }() time.Sleep(100 * time.Millisecond) @@ -709,6 +820,10 @@ func TestRouterCloseLoopOnAppClose(t *testing.T) { rule, err = rt.Rule(routeID) require.NoError(t, err) require.Nil(t, rule) + + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveAppErrCh)) + require.NoError(t, testhelpers.NoErrorWithinTimeout(protoServeErrCh)) } func TestRouterRouteExpiration(t *testing.T) { @@ -733,10 +848,15 @@ func TestRouterRouteExpiration(t *testing.T) { } r := New(conf) r.expiryTicker = time.NewTicker(100 * time.Millisecond) - go r.Serve(context.TODO()) // nolint + serveErrCh := make(chan error, 1) + go func() { + serveErrCh <- r.Serve(context.TODO()) + }() time.Sleep(110 * time.Millisecond) assert.Equal(t, 0, rt.Count()) require.NoError(t, r.Close()) + + require.NoError(t, testhelpers.NoErrorWithinTimeout(serveErrCh)) } diff --git a/pkg/routing/boltdb_routing_table.go b/pkg/routing/boltdb_routing_table.go index 1e08a13bd..009f7325e 100644 --- a/pkg/routing/boltdb_routing_table.go +++ b/pkg/routing/boltdb_routing_table.go @@ -6,17 +6,19 @@ import ( "fmt" "math" + "github.com/skycoin/skycoin/src/util/logging" "go.etcd.io/bbolt" ) var boltDBBucket = []byte("routing") +var log = logging.MustGetLogger("routing") // BoltDBRoutingTable implements RoutingTable on top of BoltDB. type boltDBRoutingTable struct { db *bbolt.DB } -// BoltDBRoutingTable consturcts a new BoldDBRoutingTable. +// BoltDBRoutingTable constructs a new BoldDBRoutingTable. func BoltDBRoutingTable(path string) (Table, error) { db, err := bbolt.Open(path, 0600, nil) if err != nil { @@ -41,7 +43,10 @@ func BoltDBRoutingTable(path string) (Table, error) { func (rt *boltDBRoutingTable) AddRule(rule Rule) (routeID RouteID, err error) { err = rt.db.Update(func(tx *bbolt.Tx) error { b := tx.Bucket(boltDBBucket) - nextID, _ := b.NextSequence() // nolint + nextID, err := b.NextSequence() + if err != nil { + return err + } if nextID > math.MaxUint32 { return errors.New("no available routeIDs") @@ -66,7 +71,7 @@ func (rt *boltDBRoutingTable) SetRule(routeID RouteID, rule Rule) error { // Rule returns RoutingRule with a given RouteID. func (rt *boltDBRoutingTable) Rule(routeID RouteID) (Rule, error) { var rule Rule - err := rt.db.View(func(tx *bbolt.Tx) error { // nolint: unparam + err := rt.db.View(func(tx *bbolt.Tx) error { b := tx.Bucket(boltDBBucket) rule = b.Get(binaryID(routeID)) return nil @@ -77,15 +82,18 @@ func (rt *boltDBRoutingTable) Rule(routeID RouteID) (Rule, error) { // RangeRules iterates over all rules and yields values to the rangeFunc until `next` is false. func (rt *boltDBRoutingTable) RangeRules(rangeFunc RangeFunc) error { - return rt.db.View(func(tx *bbolt.Tx) error { // nolint: unparam + return rt.db.View(func(tx *bbolt.Tx) error { b := tx.Bucket(boltDBBucket) - b.ForEach(func(k, v []byte) error { // nolint + f := func(k, v []byte) error { if !rangeFunc(RouteID(binary.BigEndian.Uint32(k)), v) { return errors.New("iterator stopped") } return nil - }) + } + if err := b.ForEach(f); err != nil { + log.Warn(err) + } return nil }) } @@ -93,7 +101,7 @@ func (rt *boltDBRoutingTable) RangeRules(rangeFunc RangeFunc) error { // Rules returns RoutingRules for a given RouteIDs. func (rt *boltDBRoutingTable) Rules(routeIDs ...RouteID) (rules []Rule, err error) { rules = []Rule{} - err = rt.db.View(func(tx *bbolt.Tx) error { // nolint: unparam + err = rt.db.View(func(tx *bbolt.Tx) error { b := tx.Bucket(boltDBBucket) for _, routeID := range routeIDs { @@ -126,7 +134,7 @@ func (rt *boltDBRoutingTable) DeleteRules(routeIDs ...RouteID) error { // Count returns the number of routing rules stored. func (rt *boltDBRoutingTable) Count() (count int) { - err := rt.db.View(func(tx *bbolt.Tx) error { // nolint: unparam + err := rt.db.View(func(tx *bbolt.Tx) error { b := tx.Bucket(boltDBBucket) stats := b.Stats() diff --git a/pkg/routing/boltdb_routing_table_test.go b/pkg/routing/boltdb_routing_table_test.go index 554ab899a..65506604b 100644 --- a/pkg/routing/boltdb_routing_table_test.go +++ b/pkg/routing/boltdb_routing_table_test.go @@ -2,7 +2,6 @@ package routing import ( "io/ioutil" - "log" "os" "testing" @@ -16,7 +15,9 @@ func TestBoltDBRoutingTable(t *testing.T) { log.Fatal(err) } - defer os.Remove(dbfile.Name()) + defer func() { + require.NoError(t, os.Remove(dbfile.Name())) + }() tbl, err := BoltDBRoutingTable(dbfile.Name()) require.NoError(t, err) diff --git a/pkg/routing/packet.go b/pkg/routing/packet.go index c152a4c9d..24fa300f8 100644 --- a/pkg/routing/packet.go +++ b/pkg/routing/packet.go @@ -12,7 +12,7 @@ type RouteID uint32 type Packet []byte // MakePacket constructs a new Packet. If payload size is more than -// uint16, PutUvarint will panic. +// uint16, MakePacket will panic. func MakePacket(id RouteID, payload []byte) Packet { if len(payload) > math.MaxUint16 { panic("packet size exceeded") diff --git a/pkg/routing/routing_table_test.go b/pkg/routing/routing_table_test.go index 031447e8b..a0459c99f 100644 --- a/pkg/routing/routing_table_test.go +++ b/pkg/routing/routing_table_test.go @@ -1,7 +1,6 @@ package routing import ( - "log" "os" "testing" "time" @@ -51,7 +50,7 @@ func RoutingTableSuite(t *testing.T, tbl Table) { require.NoError(t, err) assert.Equal(t, rule, r) - ids := []RouteID{} + ids := make([]RouteID, 0) err = tbl.RangeRules(func(routeID RouteID, _ Rule) bool { ids = append(ids, routeID) return true diff --git a/pkg/routing/rule.go b/pkg/routing/rule.go index b2b3c0e8e..c2c3b843a 100644 --- a/pkg/routing/rule.go +++ b/pkg/routing/rule.go @@ -74,7 +74,9 @@ func (r Rule) RemotePK() cipher.PubKey { } pk := cipher.PubKey{} - pk.UnmarshalBinary(r[13:46]) // nolint: errcheck + if err := pk.UnmarshalBinary(r[13:46]); err != nil { + log.WithError(err).Warn("Failed to unmarshal public key") + } return pk } diff --git a/pkg/setup/node.go b/pkg/setup/node.go index dcf001787..74f15b6e3 100644 --- a/pkg/setup/node.go +++ b/pkg/setup/node.go @@ -229,7 +229,11 @@ func (sn *Node) connectLoop(on cipher.PubKey, ld *LoopData) error { if err != nil { return fmt.Errorf("transport: %s", err) } - defer tr.Close() + defer func() { + if err := tr.Close(); err != nil { + sn.Logger.Warnf("Failed to close transport: %s", err) + } + }() proto := NewSetupProtocol(tr) if err := ConfirmLoop(proto, ld); err != nil { @@ -245,7 +249,11 @@ func (sn *Node) closeLoop(on cipher.PubKey, ld *LoopData) error { if err != nil { return fmt.Errorf("transport: %s", err) } - defer tr.Close() + defer func() { + if err := tr.Close(); err != nil { + sn.Logger.Warnf("Failed to close transport: %s", err) + } + }() proto := NewSetupProtocol(tr) if err := LoopClosed(proto, ld); err != nil { @@ -262,7 +270,11 @@ func (sn *Node) setupRule(pubKey cipher.PubKey, rule routing.Rule) (routeID rout err = fmt.Errorf("transport: %s", err) return } - defer tr.Close() + defer func() { + if err := tr.Close(); err != nil { + sn.Logger.Warnf("Failed to close transport: %s", err) + } + }() proto := NewSetupProtocol(tr) routeID, err = AddRule(proto, rule) diff --git a/pkg/setup/node_test.go b/pkg/setup/node_test.go index 0713fb7da..d249e643e 100644 --- a/pkg/setup/node_test.go +++ b/pkg/setup/node_test.go @@ -84,12 +84,19 @@ func TestCreateLoop(t *testing.T) { mS, err := transport.NewManager(cS, fS) require.NoError(t, err) - n1 := newMockNode(m1) - go n1.serve() // nolint: errcheck - n2 := newMockNode(m2) - go n2.serve() // nolint: errcheck - n3 := newMockNode(m3) - go n3.serve() // nolint: errcheck + var serveErr1, serveErr2, serveErr3 error + n1 := newMockNode(t, m1) + go func() { + serveErr1 = n1.serve() + }() + n2 := newMockNode(t, m2) + go func() { + serveErr2 = n2.serve() + }() + n3 := newMockNode(t, m3) + go func() { + serveErr3 = n3.serve() + }() tr1, err := m1.CreateTransport(context.TODO(), pk2, "mock", true) require.NoError(t, err) @@ -161,6 +168,10 @@ func TestCreateLoop(t *testing.T) { require.NoError(t, sn.Close()) require.NoError(t, <-errChan) + + require.NoError(t, serveErr1) + require.NoError(t, serveErr2) + require.NoError(t, serveErr3) } func TestCloseLoop(t *testing.T) { @@ -192,8 +203,11 @@ func TestCloseLoop(t *testing.T) { mS, err := transport.NewManager(cS, fS) require.NoError(t, err) - n3 := newMockNode(m3) - go n3.serve() // nolint: errcheck + n3 := newMockNode(t, m3) + var serveErr error + go func() { + serveErr = n3.serve() + }() time.Sleep(100 * time.Millisecond) @@ -219,6 +233,8 @@ func TestCloseLoop(t *testing.T) { require.NoError(t, sn.Close()) require.NoError(t, <-errChan) + + require.NoError(t, serveErr) } type muxFactory struct { @@ -285,18 +301,23 @@ func (f *muxFactory) Type() string { type mockNode struct { sync.Mutex - rules map[routing.RouteID]routing.Rule - tm *transport.Manager + testing *testing.T + rules map[routing.RouteID]routing.Rule + tm *transport.Manager } -func newMockNode(tm *transport.Manager) *mockNode { - return &mockNode{tm: tm, rules: make(map[routing.RouteID]routing.Rule)} +func newMockNode(t *testing.T, tm *transport.Manager) *mockNode { + return &mockNode{testing: t, tm: tm, rules: make(map[routing.RouteID]routing.Rule)} } func (n *mockNode) serve() error { go func() { for tr := range n.tm.TrChan { - go func(t transport.Transport) { n.serveTransport(t) }(tr) // nolint: errcheck + go func(t transport.Transport) { + if err := n.serveTransport(t); err != nil { + n.testing.Error(err) + } + }(tr) } }() @@ -330,8 +351,10 @@ func (n *mockNode) serveTransport(tr transport.Transport) error { var res interface{} switch sp { case PacketAddRules: - rules := []routing.Rule{} - json.Unmarshal(data, &rules) // nolint: errcheck + var rules []routing.Rule + if err = json.Unmarshal(data, &rules); err != nil { + return err + } for _, rule := range rules { for i := routing.RouteID(1); i < 255; i++ { if n.rules[i] == nil { @@ -343,7 +366,9 @@ func (n *mockNode) serveTransport(tr transport.Transport) error { } case PacketConfirmLoop: ld := LoopData{} - json.Unmarshal(data, &ld) // nolint: errcheck + if err = json.Unmarshal(data, &ld); err != nil { + return err + } for _, rule := range n.rules { if rule.Type() == routing.RuleApp && rule.RemotePK() == ld.RemotePK && rule.RemotePort() == ld.RemotePort && rule.LocalPort() == ld.LocalPort { @@ -354,7 +379,9 @@ func (n *mockNode) serveTransport(tr transport.Transport) error { } case PacketLoopClosed: ld := &LoopData{} - json.Unmarshal(data, ld) // nolint: errcheck + if err = json.Unmarshal(data, ld); err != nil { + return err + } for routeID, rule := range n.rules { if rule.Type() == routing.RuleApp && rule.RemotePK() == ld.RemotePK && rule.RemotePort() == ld.RemotePort && rule.LocalPort() == ld.LocalPort { diff --git a/pkg/setup/protocol_test.go b/pkg/setup/protocol_test.go index a27b6941e..357a9e9d6 100644 --- a/pkg/setup/protocol_test.go +++ b/pkg/setup/protocol_test.go @@ -3,6 +3,7 @@ package setup import ( "encoding/json" "fmt" + "log" "net" "testing" @@ -12,7 +13,11 @@ import ( func ExampleNewSetupProtocol() { in, _ := net.Pipe() - defer in.Close() + defer func() { + if err := in.Close(); err != nil { + log.Println("Failed to close connection: ", err) + } + }() sProto := NewSetupProtocol(in) fmt.Printf("Success: %v\n", sProto != nil) diff --git a/pkg/transport-discovery/client/client.go b/pkg/transport-discovery/client/client.go index 2fe923a6d..02c0f967d 100644 --- a/pkg/transport-discovery/client/client.go +++ b/pkg/transport-discovery/client/client.go @@ -13,11 +13,14 @@ import ( "github.com/google/uuid" "github.com/skycoin/dmsg/cipher" + "github.com/skycoin/skycoin/src/util/logging" "github.com/skycoin/skywire/internal/httpauth" "github.com/skycoin/skywire/pkg/transport" ) +var log = logging.MustGetLogger("transport-discovery") + // Error is the object returned to the client when there's an error. type Error struct { Error string `json:"error"` @@ -77,10 +80,16 @@ func (c *apiClient) RegisterTransports(ctx context.Context, entries ...*transpor } resp, err := c.Post(ctx, "/transports/", entries) + if resp != nil { + defer func() { + if err := resp.Body.Close(); err != nil { + log.WithError(err).Warn("Failed to close HTTP response body") + } + }() + } if err != nil { return err } - defer resp.Body.Close() if resp.StatusCode == http.StatusCreated { return nil @@ -92,10 +101,16 @@ func (c *apiClient) RegisterTransports(ctx context.Context, entries ...*transpor // GetTransportByID returns Transport for corresponding ID. func (c *apiClient) GetTransportByID(ctx context.Context, id uuid.UUID) (*transport.EntryWithStatus, error) { resp, err := c.Get(ctx, fmt.Sprintf("/transports/id:%s", id.String())) + if resp != nil { + defer func() { + if err := resp.Body.Close(); err != nil { + log.WithError(err).Warn("Failed to close HTTP response body") + } + }() + } if err != nil { return nil, err } - defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("status: %d, error: %v", resp.StatusCode, extractError(resp.Body)) @@ -112,21 +127,27 @@ func (c *apiClient) GetTransportByID(ctx context.Context, id uuid.UUID) (*transp // GetTransportsByEdge returns all Transports registered for the edge. func (c *apiClient) GetTransportsByEdge(ctx context.Context, pk cipher.PubKey) ([]*transport.EntryWithStatus, error) { resp, err := c.Get(ctx, fmt.Sprintf("/transports/edge:%s", pk)) + if resp != nil { + defer func() { + if err := resp.Body.Close(); err != nil { + log.WithError(err).Warn("Failed to close HTTP response body") + } + }() + } if err != nil { return nil, err } - defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("status: %d, error: %v", resp.StatusCode, extractError(resp.Body)) } - entry := []*transport.EntryWithStatus{} - if err := json.NewDecoder(resp.Body).Decode(&entry); err != nil { + var entries []*transport.EntryWithStatus + if err := json.NewDecoder(resp.Body).Decode(&entries); err != nil { return nil, fmt.Errorf("json: %s", err) } - return entry, nil + return entries, nil } // UpdateStatuses updates statuses of transports in discovery. @@ -136,16 +157,22 @@ func (c *apiClient) UpdateStatuses(ctx context.Context, statuses ...*transport.S } resp, err := c.Post(ctx, "/statuses", statuses) + if resp != nil { + defer func() { + if err := resp.Body.Close(); err != nil { + log.WithError(err).Warn("Failed to close HTTP response body") + } + }() + } if err != nil { return nil, err } - defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("status: %d, error: %v", resp.StatusCode, extractError(resp.Body)) } - entries := []*transport.EntryWithStatus{} + var entries []*transport.EntryWithStatus if err := json.NewDecoder(resp.Body).Decode(&entries); err != nil { return nil, fmt.Errorf("json: %s", err) } diff --git a/pkg/transport-discovery/client/client_test.go b/pkg/transport-discovery/client/client_test.go index 9b7c0afab..65f79d45e 100644 --- a/pkg/transport-discovery/client/client_test.go +++ b/pkg/transport-discovery/client/client_test.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "log" "net/http" "net/http/httptest" "os" @@ -63,7 +62,8 @@ func TestClientAuth(t *testing.T) { assert.NotEmpty(t, r.Header.Get("SW-Sig")) // TODO: check for the right key case fmt.Sprintf("/security/nonces/%s", testPubKey): - fmt.Fprintf(w, `{"edge": "%s", "next_nonce": 1}`, testPubKey) + _, err := fmt.Fprintf(w, `{"edge": "%s", "next_nonce": 1}`, testPubKey) + require.NoError(t, err) default: t.Errorf("Don't know how to handle URL = '%s'", url) @@ -122,7 +122,8 @@ func TestRegisterTransportResponses(t *testing.T) { "NonJSONError", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) - fmt.Fprintf(w, "boom") + _, err := fmt.Fprintf(w, "boom") + require.NoError(t, err) }, func(err error) { require.Error(t, err) @@ -144,7 +145,7 @@ func TestRegisterTransportResponses(t *testing.T) { t.Run(test.name, func(_ *testing.T) { wg.Add(1) - srv := httptest.NewServer(authHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(authHandler(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { defer wg.Done() test.handler(w, r) }))) @@ -166,9 +167,9 @@ func TestRegisterTransports(t *testing.T) { // Signatures does not matter in this test sEntry := &transport.SignedEntry{Entry: newTestEntry()} - srv := httptest.NewServer(authHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(authHandler(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/transports/", r.URL.String()) - entries := []*transport.SignedEntry{} + var entries []*transport.SignedEntry require.NoError(t, json.NewDecoder(r.Body).Decode(&entries)) require.Len(t, entries, 1) assert.Equal(t, sEntry.Entry, entries[0].Entry) @@ -183,9 +184,9 @@ func TestRegisterTransports(t *testing.T) { func TestGetTransportByID(t *testing.T) { entry := &transport.EntryWithStatus{Entry: newTestEntry(), IsUp: true} - srv := httptest.NewServer(authHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(authHandler(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, fmt.Sprintf("/transports/id:%s", entry.Entry.ID), r.URL.String()) - json.NewEncoder(w).Encode(entry) // nolint: errcheck + require.NoError(t, json.NewEncoder(w).Encode(entry)) }))) defer srv.Close() @@ -200,9 +201,9 @@ func TestGetTransportByID(t *testing.T) { func TestGetTransportsByEdge(t *testing.T) { entry := &transport.EntryWithStatus{Entry: newTestEntry(), IsUp: true} - srv := httptest.NewServer(authHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(authHandler(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, fmt.Sprintf("/transports/edge:%s", entry.Entry.Edges()[0]), r.URL.String()) - json.NewEncoder(w).Encode([]*transport.EntryWithStatus{entry}) // nolint: errcheck + require.NoError(t, json.NewEncoder(w).Encode([]*transport.EntryWithStatus{entry})) }))) defer srv.Close() @@ -218,13 +219,13 @@ func TestGetTransportsByEdge(t *testing.T) { func TestUpdateStatuses(t *testing.T) { entry := &transport.EntryWithStatus{Entry: newTestEntry(), IsUp: true} - srv := httptest.NewServer(authHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(authHandler(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/statuses", r.URL.String()) - statuses := []*transport.Status{} + statuses := make([]*transport.Status, 0) require.NoError(t, json.NewDecoder(r.Body).Decode(&statuses)) require.Len(t, statuses, 1) assert.Equal(t, entry.Entry.ID, statuses[0].ID) - json.NewEncoder(w).Encode([]*transport.EntryWithStatus{entry}) // nolint: errcheck + require.NoError(t, json.NewEncoder(w).Encode([]*transport.EntryWithStatus{entry})) }))) defer srv.Close() @@ -237,11 +238,11 @@ func TestUpdateStatuses(t *testing.T) { assert.Equal(t, entry.Entry, entries[0].Entry) } -func authHandler(next http.Handler) http.Handler { +func authHandler(t *testing.T, next http.Handler) http.Handler { m := http.NewServeMux() m.Handle("/security/nonces/", http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { - json.NewEncoder(w).Encode(&httpauth.NextNonceResponse{Edge: testPubKey, NextNonce: 1}) // nolint: errcheck + require.NoError(t, json.NewEncoder(w).Encode(&httpauth.NextNonceResponse{Edge: testPubKey, NextNonce: 1})) }, )) m.Handle("/", next) diff --git a/pkg/transport/discovery.go b/pkg/transport/discovery.go index 1597dc53f..e58341a8d 100644 --- a/pkg/transport/discovery.go +++ b/pkg/transport/discovery.go @@ -64,7 +64,7 @@ func (td *mockDiscoveryClient) GetTransportByID(ctx context.Context, id uuid.UUI func (td *mockDiscoveryClient) GetTransportsByEdge(ctx context.Context, pk cipher.PubKey) ([]*EntryWithStatus, error) { td.Lock() - res := []*EntryWithStatus{} + res := make([]*EntryWithStatus, 0) for _, entry := range td.entries { if entry.Entry.Edges()[0] == pk || entry.Entry.Edges()[1] == pk { e := &EntryWithStatus{} @@ -82,7 +82,7 @@ func (td *mockDiscoveryClient) GetTransportsByEdge(ctx context.Context, pk ciphe } func (td *mockDiscoveryClient) UpdateStatuses(ctx context.Context, statuses ...*Status) ([]*EntryWithStatus, error) { - res := []*EntryWithStatus{} + res := make([]*EntryWithStatus, 0) for _, status := range statuses { entry, err := td.GetTransportByID(ctx, status.ID) if err != nil { diff --git a/pkg/transport/handshake_test.go b/pkg/transport/handshake_test.go index 2d2cf6f00..3a2dcdccc 100644 --- a/pkg/transport/handshake_test.go +++ b/pkg/transport/handshake_test.go @@ -87,7 +87,7 @@ func Example_newHsMock() { // err2 is nil: true } -//func Example_validateEntry() { +// func Example_validateEntry() { // pk1, sk1 := cipher.GenerateKeyPair() // pk2, _ := cipher.GenerateKeyPair() // pk3, _ := cipher.GenerateKeyPair() @@ -111,9 +111,9 @@ func Example_newHsMock() { // } // // // Output: invalid entry edges -//} +// } -//func TestValidateEntry(t *testing.T) { +// func TestValidateEntry(t *testing.T) { // pk1, sk1 := cipher.GenerateKeyPair() // pk2, sk2 := cipher.GenerateKeyPair() // pk3, _ := cipher.GenerateKeyPair() @@ -164,7 +164,7 @@ func Example_newHsMock() { // require.True(t, sEntry.Sign(pk2, sk2)) // // require.NoError(t, validateSignedEntry(sEntry, tr, pk1)) -//} +// } func TestSettlementHandshake(t *testing.T) { mockEnv := newHsMockEnv() @@ -273,7 +273,7 @@ func TestSettlementHandshakeExistingTransport(t *testing.T) { } -//func Example_validateSignedEntry() { +// func Example_validateSignedEntry() { // mockEnv := newHsMockEnv() // // tm, tr := mockEnv.m1, mockEnv.tr1 @@ -288,7 +288,7 @@ func TestSettlementHandshakeExistingTransport(t *testing.T) { // // fmt.Printf("System is working") // // Output: System is working -//} +// } func Example_settlementInitiatorHandshake() { mockEnv := newHsMockEnv() diff --git a/pkg/transport/log.go b/pkg/transport/log.go index 1983b0ab4..e545d2384 100644 --- a/pkg/transport/log.go +++ b/pkg/transport/log.go @@ -125,7 +125,11 @@ func (tls *fileTransportLogStore) Entry(id uuid.UUID) (*LogEntry, error) { if err != nil { return nil, fmt.Errorf("open: %s", err) } - defer func() { _ = f.Close() }() //nolint:errcheck + defer func() { + if err := f.Close(); err != nil { + log.WithError(err).Warn("Failed to close file") + } + }() entry := &LogEntry{} if err := json.NewDecoder(f).Decode(entry); err != nil { @@ -140,7 +144,11 @@ func (tls *fileTransportLogStore) Record(id uuid.UUID, entry *LogEntry) error { if err != nil { return fmt.Errorf("open: %s", err) } - defer func() { _ = f.Close() }() //nolint:errcheck + defer func() { + if err := f.Close(); err != nil { + log.WithError(err).Warn("Failed to close file") + } + }() if err := json.NewEncoder(f).Encode(entry); err != nil { return fmt.Errorf("json: %s", err) diff --git a/pkg/transport/log_test.go b/pkg/transport/log_test.go index 1c3f57772..ad0cdce10 100644 --- a/pkg/transport/log_test.go +++ b/pkg/transport/log_test.go @@ -43,7 +43,9 @@ func TestInMemoryTransportLogStore(t *testing.T) { func TestFileTransportLogStore(t *testing.T) { dir, err := ioutil.TempDir("", "log_store") require.NoError(t, err) - defer os.RemoveAll(dir) + defer func() { + require.NoError(t, os.RemoveAll(dir)) + }() ls, err := transport.FileTransportLogStore(dir) require.NoError(t, err) diff --git a/pkg/transport/manager.go b/pkg/transport/manager.go index 8614c3961..7ff61e158 100644 --- a/pkg/transport/manager.go +++ b/pkg/transport/manager.go @@ -42,7 +42,10 @@ type Manager struct { // NewManager creates a Manager with the provided configuration and transport factories. // 'factories' should be ordered by preference. func NewManager(config *ManagerConfig, factories ...Factory) (*Manager, error) { - entries, _ := config.DiscoveryClient.GetTransportsByEdge(context.Background(), config.PubKey) // nolint + entries, err := config.DiscoveryClient.GetTransportsByEdge(context.Background(), config.PubKey) + if err != nil { + entries = make([]*EntryWithStatus, 0) + } mEntries := make(map[Entry]struct{}) for _, entry := range entries { @@ -209,7 +212,9 @@ func (tm *Manager) CreateTransport(ctx context.Context, remote cipher.PubKey, tp func (tm *Manager) DeleteTransport(id uuid.UUID) error { tm.mu.Lock() if tr, ok := tm.transports[id]; ok { - _ = tr.Close() //nolint:errcheck + if err := tr.Close(); err != nil { + tm.Logger.Warnf("Failed to close transport: %s", err) + } delete(tm.transports, id) } tm.mu.Unlock() @@ -239,7 +244,11 @@ func (tm *Manager) Close() error { } statuses = append(statuses, &Status{ID: tr.Entry.ID, IsUp: false}) - tr.Close() + go func(tr io.Closer) { + if err := tr.Close(); err != nil { + tm.Logger.Warnf("Failed to close transport: %s", err) + } + }(tr) } tm.mu.Unlock() @@ -248,14 +257,17 @@ func (tm *Manager) Close() error { } for _, f := range tm.factories { - go f.Close() + go func(f io.Closer) { + if err := f.Close(); err != nil { + tm.Logger.Warnf("Failed to close factory: %s", err) + } + }(f) } return nil } func (tm *Manager) dialTransport(ctx context.Context, factory Factory, remote cipher.PubKey, public bool) (Transport, *Entry, error) { - if tm.isClosing() { return nil, nil, errors.New("transport.Manager is closing. Skipping dialing transport") } @@ -267,7 +279,11 @@ func (tm *Manager) dialTransport(ctx context.Context, factory Factory, remote ci entry, err := settlementInitiatorHandshake(public).Do(tm, tr, time.Minute) if err != nil { - tr.Close() + go func() { + if err := tr.Close(); err != nil { + tm.Logger.Warnf("Failed to close transport: %s", err) + } + }() return nil, nil, err } @@ -318,7 +334,11 @@ func (tm *Manager) acceptTransport(ctx context.Context, factory Factory) (*Manag entry, err := settlementResponderHandshake().Do(tm, tr, 30*time.Second) if err != nil { - tr.Close() + go func() { + if err = tr.Close(); err != nil { + tm.Logger.Warnf("Failed to close transport: %s", err) + } + }() return nil, err } @@ -434,7 +454,9 @@ func (tm *Manager) manageTransport(ctx context.Context, mTr *ManagedTransport, f } tm.Logger.Infof("Updating transport %s", mTr.Entry.ID) - _ = mTr.updateTransport(ctx, tr, tm.config.DiscoveryClient) //nolint:errcheck + if err = mTr.updateTransport(ctx, tr, tm.config.DiscoveryClient); err != nil { + tm.Logger.Warnf("Failed to update transport: %s", err) + } } } } diff --git a/pkg/transport/manager_test.go b/pkg/transport/manager_test.go index 4f0b30f72..71ed0c7cc 100644 --- a/pkg/transport/manager_test.go +++ b/pkg/transport/manager_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "io" - "log" "os" "sync" "testing" @@ -15,6 +14,8 @@ import ( "github.com/skycoin/skycoin/src/util/logging" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/skycoin/skywire/internal/testhelpers" ) func TestMain(m *testing.M) { @@ -175,7 +176,7 @@ func TestTransportManagerReEstablishTransports(t *testing.T) { m2errCh := make(chan error, 1) go func() { m2errCh <- m2.Serve(context.TODO()) }() - //time.Sleep(time.Second * 1) // TODO: this time.Sleep looks fishy - figure out later + // time.Sleep(time.Second * 1) // TODO: this time.Sleep looks fishy - figure out later dEntry3, err := client.GetTransportByID(context.TODO(), tr2.Entry.ID) require.NoError(t, err) @@ -221,7 +222,11 @@ func TestTransportManagerLogs(t *testing.T) { tr1 := m1.Transport(tr2.Entry.ID) require.NotNil(t, tr1) - go tr1.Write([]byte("foo")) // nolint + writeErrCh := make(chan error, 1) + go func() { + _, writeErr := tr1.Write([]byte("foo")) + writeErrCh <- writeErr + }() buf := make([]byte, 3) _, err = tr2.Read(buf) require.NoError(t, err) @@ -242,6 +247,7 @@ func TestTransportManagerLogs(t *testing.T) { require.NoError(t, m2.Close()) require.NoError(t, m1.Close()) require.NoError(t, <-errCh) + require.NoError(t, testhelpers.NoErrorWithinTimeout(writeErrCh)) } func ExampleSortPubKeys() { diff --git a/pkg/transport/mock.go b/pkg/transport/mock.go index f612a8c63..8ccfed658 100644 --- a/pkg/transport/mock.go +++ b/pkg/transport/mock.go @@ -141,7 +141,6 @@ func (m *MockTransport) Edges() [2]cipher.PubKey { // SetDeadline sets a deadline for the write/read operations of the mock transport func (m *MockTransport) SetDeadline(t time.Time) error { - // nolint ctx, cancel := context.WithDeadline(m.context, t) m.context = ctx diff --git a/pkg/transport/tcp_transport.go b/pkg/transport/tcp_transport.go index 906bab428..3b5c01560 100644 --- a/pkg/transport/tcp_transport.go +++ b/pkg/transport/tcp_transport.go @@ -133,7 +133,7 @@ func FilePubKeyTable(dbFile string) (PubKeyTable, error) { return nil, err } - f, err := os.Open(path) + f, err := os.Open(filepath.Clean(path)) if err != nil { return nil, err } @@ -168,7 +168,11 @@ func (t *filePKTable) RemotePK(remoteIP net.IP) cipher.PubKey { } func (t *filePKTable) Seek(seekFunc func(pk cipher.PubKey, addr *net.TCPAddr) bool) { - defer t.dbFile.Seek(0, 0) // nolint + defer func() { + if _, err := t.dbFile.Seek(0, 0); err != nil { + log.WithError(err).Warn("Failed to seek to the beginning of DB") + } + }() scanner := bufio.NewScanner(t.dbFile) for scanner.Scan() { diff --git a/pkg/transport/tcp_transport_test.go b/pkg/transport/tcp_transport_test.go index ac2c5c08f..97919889c 100644 --- a/pkg/transport/tcp_transport_test.go +++ b/pkg/transport/tcp_transport_test.go @@ -72,7 +72,9 @@ func TestFilePKTable(t *testing.T) { tmpfile, err := ioutil.TempFile("", "pktable") require.NoError(t, err) - defer os.Remove(tmpfile.Name()) + defer func() { + assert.NoError(t, os.Remove(tmpfile.Name())) + }() addr, err := net.ResolveTCPAddr("tcp", "127.0.0.1:9000") require.NoError(t, err) diff --git a/pkg/transport/transport.go b/pkg/transport/transport.go index 96c4061c7..bc8b66760 100644 --- a/pkg/transport/transport.go +++ b/pkg/transport/transport.go @@ -8,8 +8,11 @@ import ( "github.com/google/uuid" "github.com/skycoin/dmsg/cipher" + "github.com/skycoin/skycoin/src/util/logging" ) +var log = logging.MustGetLogger("transport") + // Transport represents communication between two nodes via a single hop. type Transport interface { diff --git a/pkg/util/pathutil/homedir.go b/pkg/util/pathutil/homedir.go index 439eea8db..b814615c0 100644 --- a/pkg/util/pathutil/homedir.go +++ b/pkg/util/pathutil/homedir.go @@ -62,14 +62,16 @@ func AtomicWriteFile(filename string, data []byte) { } if err != nil { - os.Remove(f.Name()) // nolint: errcheck + if err = os.Remove(f.Name()); err != nil { + log.WithError(err).Warnf("Failed to remove file %s", f.Name()) + } panic(err) } } // AtomicAppendToFile calls AtomicWriteFile but appends new data to destiny file func AtomicAppendToFile(filename string, data []byte) { - oldFile, err := ioutil.ReadFile(filename) + oldFile, err := ioutil.ReadFile(filepath.Clean(filename)) if err != nil { panic(err) } diff --git a/vendor/github.com/skycoin/dmsg/client.go b/vendor/github.com/skycoin/dmsg/client.go index c933e1b89..d76496050 100644 --- a/vendor/github.com/skycoin/dmsg/client.go +++ b/vendor/github.com/skycoin/dmsg/client.go @@ -256,18 +256,14 @@ func (c *ClientConn) DialTransport(ctx context.Context, clientPK cipher.PubKey) } func (c *ClientConn) close() (closed bool) { - if c == nil { - return false - } c.once.Do(func() { closed = true c.log.WithField("remoteServer", c.remoteSrv).Infoln("ClosingConnection") close(c.done) c.mx.Lock() for _, tp := range c.tps { - // Nil check is required here to keep 8192 running goroutines limit in tests with -race flag. if tp != nil { - go tp.Close() // nolint:errcheck + go tp.Close() //nolint:errcheck } } _ = c.Conn.Close() //nolint:errcheck @@ -278,9 +274,6 @@ func (c *ClientConn) close() (closed bool) { // Close closes the connection to dms_server. func (c *ClientConn) Close() error { - if c == nil { - return nil - } if c.close() { c.wg.Wait() } @@ -546,10 +539,6 @@ func (c *Client) Type() string { // Close closes the dms_client and associated connections. // TODO(evaninjin): proper error handling. func (c *Client) Close() error { - if c == nil { - return nil - } - c.once.Do(func() { close(c.done) for { diff --git a/vendor/github.com/skycoin/dmsg/server.go b/vendor/github.com/skycoin/dmsg/server.go index f49612613..80ae54b31 100644 --- a/vendor/github.com/skycoin/dmsg/server.go +++ b/vendor/github.com/skycoin/dmsg/server.go @@ -293,10 +293,6 @@ func (s *Server) connCount() int { // Close closes the dms_server. func (s *Server) Close() (err error) { - if s == nil { - return nil - } - if err = s.lis.Close(); err != nil { return err } diff --git a/vendor/github.com/skycoin/dmsg/transport.go b/vendor/github.com/skycoin/dmsg/transport.go index e40dc8835..f795b5218 100644 --- a/vendor/github.com/skycoin/dmsg/transport.go +++ b/vendor/github.com/skycoin/dmsg/transport.go @@ -83,10 +83,6 @@ func (tp *Transport) serve() (started bool) { // 3. Our worry now, is writing to `inCh`/`bufCh` AFTER they have been closed. // 4. But as, under the mutexes protecting `inCh`/`bufCh`, checking `done` comes first, and we know that `done` is closed before `inCh`/`bufCh`, we can guarantee that it avoids writing to closed chan. func (tp *Transport) close() (closed bool) { - if tp == nil { - return false - } - tp.doneOnce.Do(func() { closed = true @@ -109,9 +105,6 @@ func (tp *Transport) close() (closed bool) { // Close closes the dmsg_tp. func (tp *Transport) Close() error { - if tp == nil { - return nil - } if tp.close() { _ = writeFrame(tp.Conn, MakeFrame(CloseType, tp.id, []byte{0})) //nolint:errcheck }