Improve error handling

CHANGELOG.md

@@ -16,6 +16,8 @@ NOTE: As semantic versioning states all 0.y.z releases can contain breaking chan
 
 ### Changed
 
+- [#82](https://github.com/kobsio/kobs/pull/82): Improve error handling for our API.
+
 ## [v0.4.0](https://github.com/kobsio/kobs/releases/tag/v0.4.0) (2021-07-14)
 
 ### Added

pkg/api/clusters/router.go

@@ -53,13 +53,13 @@ func (router *Router) getNamespaces(w http.ResponseWriter, r *http.Request) {
 	for _, clusterName := range clusterNames {
 		cluster := router.clusters.GetCluster(clusterName)
 		if cluster == nil {
-			render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "invalid cluster name"))
+			errresponse.Render(w, r, nil, http.StatusBadRequest, "Invalid cluster name")
 			return
 		}
 
 		clusterNamespaces, err := cluster.GetNamespaces(r.Context(), cacheDurationNamespaces)
 		if err != nil {
-			render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not get namespaces"))
+			errresponse.Render(w, r, err, http.StatusBadRequest, "Could not get namespaces")
 			return
 		}
 

pkg/api/middleware/errresponse/errresponse.go

@@ -3,35 +3,31 @@
 package errresponse
 
 import (
+	"fmt"
 	"net/http"
 
+	"github.com/kobsio/kobs/pkg/api/middleware/httplog"
+
 	"github.com/go-chi/render"
 )
 
 // ErrResponse renderer type for handling all sorts of errors.
 type ErrResponse struct {
-	Err            error  `json:"-"`     // low-level runtime error
-	HTTPStatusCode int    `json:"-"`     // http response status code
-	StatusText     string `json:"error"` // user-level status message
-}
-
-func (e *ErrResponse) Render(w http.ResponseWriter, r *http.Request) error {
-	render.Status(r, e.HTTPStatusCode)
-	return nil
+	Error string `json:"error"`
 }
 
-func Render(err error, status int, msg string) render.Renderer {
+// Render sets the given status for the response and then returns the error and message as JSON object.
+func Render(w http.ResponseWriter, r *http.Request, err error, status int, msg string) {
 	if err != nil {
-		return &ErrResponse{
-			Err:            err,
-			HTTPStatusCode: status,
-			StatusText:     msg,
-		}
+		msg = fmt.Sprintf("%s: %s", msg, err.Error())
 	}
 
-	return &ErrResponse{
-		Err:            nil,
-		HTTPStatusCode: status,
-		StatusText:     msg,
+	errResponse := &ErrResponse{
+		Error: msg,
 	}
+
+	httplog.LogEntrySetField(r, "error", msg)
+
+	render.Status(r, status)
+	render.JSON(w, r, errResponse)
 }

plugins/applications/applications.go

@@ -81,7 +81,7 @@ func (router *Router) getApplications(w http.ResponseWriter, r *http.Request) {
 					return
 				}
 
-				render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not get applications"))
+				errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not get applications")
 				return
 			}
 
@@ -108,14 +108,14 @@ func (router *Router) getApplications(w http.ResponseWriter, r *http.Request) {
 		for _, clusterName := range clusterNames {
 			cluster := router.clusters.GetCluster(clusterName)
 			if cluster == nil {
-				render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "invalid cluster name"))
+				errresponse.Render(w, r, nil, http.StatusBadRequest, "Invalid cluster name")
 				return
 			}
 
 			for _, namespace := range namespaces {
 				application, err := cluster.GetApplications(r.Context(), namespace)
 				if err != nil {
-					render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not get applications"))
+					errresponse.Render(w, r, err, http.StatusBadRequest, "Could not get applications")
 					return
 				}
 
@@ -153,7 +153,7 @@ func (router *Router) getApplications(w http.ResponseWriter, r *http.Request) {
 				return
 			}
 
-			render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not generate topology"))
+			errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not generate topology")
 			return
 		}
 
@@ -172,7 +172,7 @@ func (router *Router) getApplications(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "invalid view property"))
+	errresponse.Render(w, r, nil, http.StatusBadRequest, "Invalid view property")
 }
 
 // getApplication returns a a single application for the given clusters and namespaces and name. The cluster, namespace
@@ -186,13 +186,13 @@ func (router *Router) getApplication(w http.ResponseWriter, r *http.Request) {
 
 	cluster := router.clusters.GetCluster(clusterName)
 	if cluster == nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "invalid cluster name"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Invalid cluster name")
 		return
 	}
 
 	application, err := cluster.GetApplication(r.Context(), namespace, name)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not get application"))
+		errresponse.Render(w, r, err, http.StatusBadRequest, "Could not get application")
 		return
 	}
 

plugins/dashboards/dashboards.go

@@ -58,7 +58,7 @@ func (router *Router) getAllDashboards(w http.ResponseWriter, r *http.Request) {
 	for _, cluster := range router.clusters.Clusters {
 		dashboard, err := cluster.GetDashboards(r.Context(), "")
 		if err != nil {
-			render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not get dashboards"))
+			errresponse.Render(w, r, err, http.StatusBadRequest, "Could not get dashboards")
 			return
 		}
 
@@ -79,7 +79,7 @@ func (router *Router) getDashboards(w http.ResponseWriter, r *http.Request) {
 
 	err := json.NewDecoder(r.Body).Decode(&data)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not decode request body"))
+		errresponse.Render(w, r, err, http.StatusBadRequest, "Could not decode request body")
 		return
 	}
 
@@ -100,20 +100,20 @@ func (router *Router) getDashboards(w http.ResponseWriter, r *http.Request) {
 
 		cluster := router.clusters.GetCluster(reference.Cluster)
 		if cluster == nil {
-			render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "invalid cluster name"))
+			errresponse.Render(w, r, nil, http.StatusBadRequest, "Invalid cluster name")
 			return
 		}
 
 		dashboard, err := cluster.GetDashboard(r.Context(), reference.Namespace, reference.Name)
 		if err != nil {
-			render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not get dashboard"))
+			errresponse.Render(w, r, err, http.StatusBadRequest, "Could not get dashboard")
 			return
 		}
 
 		if reference.Placeholders != nil {
 			dashboard, err = placeholders.Replace(reference.Placeholders, *dashboard)
 			if err != nil {
-				render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not replace placeholders"))
+				errresponse.Render(w, r, err, http.StatusBadRequest, "Could not replace placeholders")
 				return
 			}
 		}
@@ -136,28 +136,28 @@ func (router *Router) getDashboard(w http.ResponseWriter, r *http.Request) {
 
 	err := json.NewDecoder(r.Body).Decode(&data)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not decode request body"))
+		errresponse.Render(w, r, err, http.StatusBadRequest, "Could not decode request body")
 		return
 	}
 
 	log.WithFields(logrus.Fields{"cluster": data.Cluster, "namespace": data.Namespace, "name": data.Name}).Tracef("getDashboard")
 
 	cluster := router.clusters.GetCluster(data.Cluster)
 	if cluster == nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "invalid cluster name"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Invalid cluster name")
 		return
 	}
 
 	dashboard, err := cluster.GetDashboard(r.Context(), data.Namespace, data.Name)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not get dashboard"))
+		errresponse.Render(w, r, err, http.StatusBadRequest, "Could not get dashboard")
 		return
 	}
 
 	if data.Placeholders != nil {
 		dashboard, err = placeholders.Replace(data.Placeholders, *dashboard)
 		if err != nil {
-			render.Render(w, r, errresponse.Render(err, http.StatusBadRequest, "could not replace placeholders"))
+			errresponse.Render(w, r, err, http.StatusBadRequest, "Could not replace placeholders")
 			return
 		}
 	}

plugins/elasticsearch/elasticsearch.go

@@ -57,25 +57,25 @@ func (router *Router) getLogs(w http.ResponseWriter, r *http.Request) {
 
 	i := router.getInstance(name)
 	if i == nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not find instance name"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not find instance name")
 		return
 	}
 
 	parsedTimeStart, err := strconv.ParseInt(timeStart, 10, 64)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not parse start time"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not parse start time")
 		return
 	}
 
 	parsedTimeEnd, err := strconv.ParseInt(timeEnd, 10, 64)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not parse end time"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not parse end time")
 		return
 	}
 
 	data, err := i.GetLogs(r.Context(), query, scrollID, parsedTimeStart, parsedTimeEnd)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(err, http.StatusInternalServerError, "could not get logs"))
+		errresponse.Render(w, r, err, http.StatusInternalServerError, "Could not get logs")
 		return
 	}
 

plugins/jaeger/jaeger.go

@@ -48,13 +48,13 @@ func (router *Router) getServices(w http.ResponseWriter, r *http.Request) {
 
 	i := router.getInstance(name)
 	if i == nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not find instance name"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not find instance name")
 		return
 	}
 
 	body, err := i.GetServices()
 	if err != nil {
-		render.Render(w, r, errresponse.Render(err, http.StatusInternalServerError, "could not get services"))
+		errresponse.Render(w, r, err, http.StatusInternalServerError, "Could not get services")
 		return
 	}
 
@@ -69,13 +69,13 @@ func (router *Router) getOperations(w http.ResponseWriter, r *http.Request) {
 
 	i := router.getInstance(name)
 	if i == nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not find instance name"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not find instance name")
 		return
 	}
 
 	body, err := i.GetOperations(service)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(err, http.StatusInternalServerError, "could not get operations"))
+		errresponse.Render(w, r, err, http.StatusInternalServerError, "Could not get operations")
 		return
 	}
 
@@ -97,25 +97,25 @@ func (router *Router) getTraces(w http.ResponseWriter, r *http.Request) {
 
 	i := router.getInstance(name)
 	if i == nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not find instance name"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not find instance name")
 		return
 	}
 
 	parsedTimeStart, err := strconv.ParseInt(timeStart, 10, 64)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not parse start time"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not parse start time")
 		return
 	}
 
 	parsedTimeEnd, err := strconv.ParseInt(timeEnd, 10, 64)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not parse end time"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not parse end time")
 		return
 	}
 
 	body, err := i.GetTraces(limit, maxDuration, minDuration, operation, service, tags, parsedTimeStart, parsedTimeEnd)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(err, http.StatusInternalServerError, "could not get traces"))
+		errresponse.Render(w, r, err, http.StatusInternalServerError, "Could not get traces")
 		return
 	}
 
@@ -130,13 +130,13 @@ func (router *Router) getTrace(w http.ResponseWriter, r *http.Request) {
 
 	i := router.getInstance(name)
 	if i == nil {
-		render.Render(w, r, errresponse.Render(nil, http.StatusBadRequest, "could not find instance name"))
+		errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not find instance name")
 		return
 	}
 
 	body, err := i.GetTrace(traceID)
 	if err != nil {
-		render.Render(w, r, errresponse.Render(err, http.StatusInternalServerError, "could not get trace"))
+		errresponse.Render(w, r, err, http.StatusInternalServerError, "Could not get trace")
 		return
 	}