logger as package level logger and more

Add better error handling.
Remove the context struct.
Use context.Context to pass db client.
Remove handler options.

api/http/errors.go

@@ -0,0 +1,68 @@
+// Copyright 2022 Democratized Data Foundation
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package http
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"os"
+	"strings"
+)
+
+const (
+	errBadRequest          = "Bad Request"
+	errInternalServerError = "Internal Server Error"
+	errNotFound            = "Not Found"
+)
+
+var env = os.Getenv("ENV")
+
+type errorResponse struct {
+	Status  int    `json:"status"`
+	Message string `json:"message"`
+	Stack   string `json:"stack,omitempty"`
+}
+
+func handleErr(rw http.ResponseWriter, err error, status int) {
+	var message string
+
+	switch status {
+	case http.StatusBadRequest:
+		message = errBadRequest
+
+	case http.StatusInternalServerError:
+		message = errInternalServerError
+		// @TODO: The internal server error log should be sent to a different location
+		// ideally not in the http logs.
+		log.ErrorE(context.Background(), errInternalServerError, err)
+
+	case http.StatusNotFound:
+		message = errNotFound
+	}
+
+	sendJSON(
+		rw,
+		errorResponse{
+			Status:  status,
+			Message: message,
+			Stack:   formatError(err),
+		},
+		status,
+	)
+}
+
+func formatError(err error) string {
+	if strings.ToLower(env) == "dev" || strings.ToLower(env) == "development" {
+		return fmt.Sprintf("[DEV] %+v\n", err)
+	}
+	return ""
+}

api/http/errors_test.go

@@ -0,0 +1,58 @@
+// Copyright 2022 Democratized Data Foundation
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package http
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFormatError(t *testing.T) {
+	env = "prod"
+	s := formatError(errors.New("test error"))
+	assert.Equal(t, "", s)
+
+	env = "dev"
+	s = formatError(errors.New("test error"))
+	lines := strings.Split(s, "\n")
+	assert.Equal(t, "[DEV] test error", lines[0])
+}
+
+func TestHandleErr(t *testing.T) {
+	env = "dev"
+	f := func(rw http.ResponseWriter, req *http.Request) {
+		handleErr(rw, errors.New("test error"), http.StatusBadRequest)
+	}
+	req, err := http.NewRequest("GET", "/test", nil)
+	assert.NoError(t, err)
+
+	rec := httptest.NewRecorder()
+
+	f(rec, req)
+
+	resp := rec.Result()
+
+	errResponse := errorResponse{}
+	err = json.NewDecoder(resp.Body).Decode(&errResponse)
+	assert.NoError(t, err)
+
+	assert.Equal(t, http.StatusBadRequest, errResponse.Status)
+	assert.Equal(t, "Bad Request", errResponse.Message)
+
+	lines := strings.Split(errResponse.Stack, "\n")
+	assert.Equal(t, "[DEV] test error", lines[0])
+}

api/http/handler.go

@@ -11,62 +11,55 @@
 package http
 
 import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
 	"net/http"
 
 	"github.com/go-chi/chi"
+	"github.com/pkg/errors"
 	"github.com/sourcenetwork/defradb/client"
-	"github.com/sourcenetwork/defradb/logging"
 )
 
 type Handler struct {
 	db client.DB
-
 	*chi.Mux
-	*logger
 }
 
-// newHandler returns a handler with the router instantiated and configuration applied.
-func newHandler(db client.DB, options ...func(*Handler)) *Handler {
-	h := &Handler{
-		db: db,
-	}
+type ctxKey string
 
-	// apply options
-	for _, o := range options {
-		o(h)
-	}
+// newHandler returns a handler with the router instantiated.
+func newHandler(db client.DB) *Handler {
+	return setRoutes(&Handler{db: db})
+}
 
-	// ensure we have a logger defined
-	if h.logger == nil {
-		h.logger = defaultLogger()
+func (h *Handler) handle(f http.HandlerFunc) http.HandlerFunc {
+	return func(rw http.ResponseWriter, req *http.Request) {
+		ctx := context.WithValue(req.Context(), ctxKey("DB"), h.db)
+		f(rw, req.WithContext(ctx))
 	}
-
-	h.setRoutes()
-
-	return h
 }
 
-// WithLogger returns an option loading function for logger.
-func WithLogger(l logging.Logger) func(*Handler) {
-	return func(h *Handler) {
-		h.logger = &logger{l}
+func getJSON(r *http.Request, v interface{}) error {
+	err := json.NewDecoder(r.Body).Decode(v)
+	if err != nil {
+		return errors.Wrap(err, "unmarshall error")
 	}
+	return nil
 }
 
-type requestContext struct {
-	res http.ResponseWriter
-	req *http.Request
-	db  client.DB
-	log *logger
-}
+func sendJSON(rw http.ResponseWriter, v interface{}, code int) {
+	rw.Header().Set("Content-Type", "application/json")
 
-func (h *Handler) handle(f func(*requestContext)) http.HandlerFunc {
-	return func(w http.ResponseWriter, r *http.Request) {
-		f(&requestContext{
-			res: w,
-			req: r,
-			db:  h.db,
-			log: h.logger,
-		})
+	b, err := json.Marshal(v)
+	if err != nil {
+		log.Error(context.Background(), fmt.Sprintf("Error while encoding JSON: %v", err))
+		rw.WriteHeader(http.StatusInternalServerError)
+		io.WriteString(rw, `{"error": "Internal server error"}`)
+		return
 	}
+
+	rw.WriteHeader(code)
+	rw.Write(b)
 }

api/http/handler_test.go

@@ -27,18 +27,18 @@ func TestNewHandlerWithLogger(t *testing.T) {
 
 	// send logs to temp file so we can inspect it
 	logFile := path.Join(dir, "http_test.log")
-	h.ApplyConfig(logging.Config{
+	log.ApplyConfig(logging.Config{
 		EncoderFormat: logging.NewEncoderFormatOption(logging.JSON),
 		OutputPaths:   []string{logFile},
 	})
 
 	req, err := http.NewRequest("GET", "/ping", nil)
 	assert.NoError(t, err)
 
-	rec2 := httptest.NewRecorder()
+	rec := httptest.NewRecorder()
 
-	h.logger.middleware(h.handle(ping)).ServeHTTP(rec2, req)
-	assert.Equal(t, 200, rec2.Result().StatusCode)
+	loggerMiddleware(h.handle(ping)).ServeHTTP(rec, req)
+	assert.Equal(t, 200, rec.Result().StatusCode)
 
 	// inspect the log file
 	kv, err := readLog(logFile)
@@ -47,31 +47,3 @@ func TestNewHandlerWithLogger(t *testing.T) {
 	assert.Equal(t, "defra.http", kv["logger"])
 
 }
-
-func TestNewHandlerWithConfigAndLogger(t *testing.T) {
-	h := newHandler(nil, WithLogger(logging.MustNewLogger("defra.http.test")))
-
-	dir := t.TempDir()
-
-	// send logs to temp file so we can inspect it
-	logFile := path.Join(dir, "http_test.log")
-	h.ApplyConfig(logging.Config{
-		EncoderFormat: logging.NewEncoderFormatOption(logging.JSON),
-		OutputPaths:   []string{logFile},
-	})
-
-	req, err := http.NewRequest("GET", "/ping", nil)
-	assert.NoError(t, err)
-
-	rec2 := httptest.NewRecorder()
-
-	h.logger.middleware(h.handle(ping)).ServeHTTP(rec2, req)
-	assert.Equal(t, 200, rec2.Result().StatusCode)
-
-	// inspect the log file
-	kv, err := readLog(logFile)
-	assert.NoError(t, err)
-
-	assert.Equal(t, "defra.http.test", kv["logger"])
-
-}