From 1219541184368926eb819cbf768b581eb2521cef Mon Sep 17 00:00:00 2001
From: Max Leonard Inden <IndenML@gmail.com>
Date: Tue, 14 Aug 2018 19:14:00 +0200
Subject: [PATCH] *.go: Introduce errcheck enforcing error handling

Errcheck [1] enforces error handling accross all go files. Functions can
be excluded via `scripts/errcheck_excludes.txt`.

This patch adds errcheck to the `test` Make target.

[1] https://github.com/kisielk/errcheck

Signed-off-by: Max Leonard Inden <IndenML@gmail.com>
---
 Makefile                      | 9 +++++++++
 api/api.go                    | 9 +++++++--
 cmd/alertmanager/main.go      | 3 ++-
 notify/impl.go                | 6 +++++-
 provider/mem/mem.go           | 3 +--
 scripts/errcheck_excludes.txt | 8 ++++++++
 test/acceptance.go            | 6 +++++-
 test/mock.go                  | 6 +++++-
 8 files changed, 42 insertions(+), 8 deletions(-)
 create mode 100644 scripts/errcheck_excludes.txt

diff --git a/Makefile b/Makefile
index b46bb58a..9e74e570 100644
--- a/Makefile
+++ b/Makefile
@@ -15,6 +15,7 @@ include Makefile.common
 
 FRONTEND_DIR            = $(BIN_DIR)/ui/app
 DOCKER_IMAGE_NAME       ?= alertmanager
+ERRCHECK_BINARY         := $(FIRST_GOPATH)/bin/errcheck
 
 ifdef DEBUG
 	bindata_flags = -debug
@@ -59,3 +60,11 @@ clean:
 	rm template/internal/deftmpl/bindata.go
 	rm ui/bindata.go
 	cd $(FRONTEND_DIR) && $(MAKE) clean
+
+.PHONY: test
+test: common-test $(ERRCHECK_BINARY)
+	@echo ">> running errcheck with exclude file scripts/errcheck_excludes.txt"
+	$(ERRCHECK_BINARY) -verbose -exclude scripts/errcheck_excludes.txt -ignoretests ./...
+
+$(ERRCHECK_BINARY):
+	@go get github.com/kisielk/errcheck
diff --git a/api/api.go b/api/api.go
index da7bc836..1dfa9f43 100644
--- a/api/api.go
+++ b/api/api.go
@@ -786,7 +786,10 @@ func (api *API) respond(w http.ResponseWriter, data interface{}) {
 		level.Error(api.logger).Log("msg", "Error marshalling JSON", "err", err)
 		return
 	}
-	w.Write(b)
+
+	if _, err := w.Write(b); err != nil {
+		level.Error(api.logger).Log("msg", "failed to write data to connection", "err", err)
+	}
 }
 
 func (api *API) respondError(w http.ResponseWriter, apiErr apiError, data interface{}) {
@@ -812,7 +815,9 @@ func (api *API) respondError(w http.ResponseWriter, apiErr apiError, data interf
 	}
 	level.Error(api.logger).Log("msg", "API error", "err", apiErr.Error())
 
-	w.Write(b)
+	if _, err := w.Write(b); err != nil {
+		level.Error(api.logger).Log("msg", "failed to write data to connection", "err", err)
+	}
 }
 
 func (api *API) receive(r *http.Request, v interface{}) error {
diff --git a/cmd/alertmanager/main.go b/cmd/alertmanager/main.go
index 4ee29e43..48fd0178 100644
--- a/cmd/alertmanager/main.go
+++ b/cmd/alertmanager/main.go
@@ -411,7 +411,8 @@ func main() {
 		for {
 			select {
 			case <-hup:
-				reload()
+				// ignore error, already logged in `reload()`
+				_ = reload()
 			case errc := <-webReload:
 				errc <- reload()
 			}
diff --git a/notify/impl.go b/notify/impl.go
index f1d02f77..4801ece1 100644
--- a/notify/impl.go
+++ b/notify/impl.go
@@ -280,7 +280,11 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) {
 			return true, err
 		}
 	}
-	defer c.Quit()
+	defer func() {
+		if err := c.Quit(); err != nil {
+			level.Error(n.logger).Log("msg", "failed to close SMTP connection", "err", err)
+		}
+	}()
 
 	if n.conf.Hello != "" {
 		err := c.Hello(n.conf.Hello)
diff --git a/provider/mem/mem.go b/provider/mem/mem.go
index 7e0875c6..c55244fd 100644
--- a/provider/mem/mem.go
+++ b/provider/mem/mem.go
@@ -90,9 +90,8 @@ func (a *Alerts) runGC() {
 }
 
 // Close the alert provider.
-func (a *Alerts) Close() error {
+func (a *Alerts) Close() {
 	close(a.stopGC)
-	return nil
 }
 
 func max(a, b int) int {
diff --git a/scripts/errcheck_excludes.txt b/scripts/errcheck_excludes.txt
new file mode 100644
index 00000000..5b33751e
--- /dev/null
+++ b/scripts/errcheck_excludes.txt
@@ -0,0 +1,8 @@
+(io.Closer).Close
+(*os.File).Close
+fmt.Fprintf
+fmt.Fprintln
+
+(github.com/prometheus/alertmanager/vendor/github.com/go-kit/kit/log.Logger).Log
+// Allowed log levels are enforced via kingpin
+(*github.com/prometheus/alertmanager/vendor/github.com/prometheus/common/promlog.AllowedLevel).Set
diff --git a/test/acceptance.go b/test/acceptance.go
index 8c24e358..f1eb0cbe 100644
--- a/test/acceptance.go
+++ b/test/acceptance.go
@@ -94,7 +94,11 @@ func freeAddress() string {
 	if err != nil {
 		panic(err)
 	}
-	defer l.Close()
+	defer func() {
+		if err := l.Close(); err != nil {
+			panic(err)
+		}
+	}()
 
 	return l.Addr().String()
 }
diff --git a/test/mock.go b/test/mock.go
index bfe50b90..352b4436 100644
--- a/test/mock.go
+++ b/test/mock.go
@@ -257,7 +257,11 @@ func NewWebhook(c *Collector) *MockWebhook {
 		collector: c,
 		opts:      c.opts,
 	}
-	go http.Serve(l, wh)
+	go func() {
+		if err := http.Serve(l, wh); err != nil {
+			panic(err)
+		}
+	}()
 
 	return wh
 }