From 665a15093c19b58cd7b692ec8e285547dbcc6e89 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Fri, 21 Jan 2022 15:20:21 +0100 Subject: [PATCH] Fix how errors are reported/masked --- pkg/errors/http.go | 22 ++++++++++++++------- pkg/errors/http_test.go | 43 +++++++++++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/pkg/errors/http.go b/pkg/errors/http.go index 5eef2783c..10d87eb75 100644 --- a/pkg/errors/http.go +++ b/pkg/errors/http.go @@ -43,6 +43,7 @@ func ProperlyServeHTTP(w http.ResponseWriter, r *http.Request, err error, mask b serveHTTP(w, r, code, err, mask) } +// Serves error via func serveHTTP(w http.ResponseWriter, r *http.Request, code int, err error, mask bool) { var ( // Very naive approach on parsing accept headers @@ -53,9 +54,9 @@ func serveHTTP(w http.ResponseWriter, r *http.Request, code int, err error, mask // Prettify error for plain text debug output w.Header().Set("Content-Type", "plain/text") w.WriteHeader(code) - writeHttpPlain(w, err) - fmt.Fprintln(w, "Note: you are seeing this because system is running in development mode") - fmt.Fprintln(w, "and HTTP request is made without \"Accept: .../json\" headers") + writeHttpPlain(w, err, mask) + _, _ = fmt.Fprintln(w, "Note: you are seeing this because system is running in development mode") + _, _ = fmt.Fprintln(w, "and HTTP request is made without \"Accept: .../json\" headers") return } @@ -64,7 +65,7 @@ func serveHTTP(w http.ResponseWriter, r *http.Request, code int, err error, mask writeHttpJSON(r.Context(), w, err, mask) } -func writeHttpPlain(w io.Writer, err error) { +func writeHttpPlain(w io.Writer, err error, mask bool) { var ( dlmt = strings.Repeat("-", 80) @@ -79,6 +80,12 @@ func writeHttpPlain(w io.Writer, err error) { write(err.Error()) write("\n") + if _, is := err.(interface{ Safe() bool }); !is || mask { + // do not output any details on un-safe errors or + // when a masked output is preferred + return + } + if err, is := err.(*Error); is { write(dlmt + "\n") @@ -105,13 +112,14 @@ func writeHttpPlain(w io.Writer, err error) { if we := err.Unwrap(); we != nil { write(dlmt + "\n") write("Wrapped error:\n\n") - writeHttpPlain(w, we) + writeHttpPlain(w, we, mask) } } write(dlmt + "\n") } +// writeHttpJSON func writeHttpJSON(ctx context.Context, w io.Writer, err error, mask bool) { var ( wrap = struct { @@ -119,8 +127,8 @@ func writeHttpJSON(ctx context.Context, w io.Writer, err error, mask bool) { }{} ) - if se, is := err.(interface{ Safe() bool }); !is || !se.Safe() { - // trim error details when not debugging or error is not safe + if se, is := err.(interface{ Safe() bool }); !is || !se.Safe() || mask { + // trim error details when not debugging or error is not safe or maske err = fmt.Errorf(err.Error()) } diff --git a/pkg/errors/http_test.go b/pkg/errors/http_test.go index ba7efb0d7..09ce12185 100644 --- a/pkg/errors/http_test.go +++ b/pkg/errors/http_test.go @@ -1,17 +1,20 @@ package errors import ( + "bytes" "context" "fmt" "os" + "testing" + + "github.com/stretchr/testify/require" ) func Example_writeHttpPlain() { - writeHttpPlain(os.Stdout, fmt.Errorf("dummy error")) + writeHttpPlain(os.Stdout, fmt.Errorf("dummy error"), true) // Output: // Error: dummy error - // -------------------------------------------------------------------------------- } func Example_writeHttpJSON() { @@ -21,10 +24,18 @@ func Example_writeHttpJSON() { // {"error":{"message":"dummy error"}} } -func Example_writeHttpPlain_2() { +func Example_writeHttpPlain_masked() { err := New(0, "dummy error", Meta("a", "b"), Meta(&Error{}, "nope")) err.stack = nil // will not test the stack as file path & line numbers might change - writeHttpPlain(os.Stdout, err) + writeHttpPlain(os.Stdout, err, true) + // Output: + // Error: dummy error +} + +func Example_writeHttpPlain_unmasked() { + err := New(0, "dummy error", Meta("a", "b"), Meta(&Error{}, "nope")) + err.stack = nil // will not test the stack as file path & line numbers might change + writeHttpPlain(os.Stdout, err, false) // Output: // Error: dummy error // -------------------------------------------------------------------------------- @@ -32,11 +43,23 @@ func Example_writeHttpPlain_2() { // -------------------------------------------------------------------------------- } -func Example_writeHttpJSON_2() { - err := New(0, "dummy error", Meta("a", "b"), Meta(&Error{}, "nope")) - err.stack = nil // will not test the stack as file path & line numbers might change - writeHttpJSON(context.Background(), os.Stdout, err, false) +func Test_writeHttpJSON(t *testing.T) { + var ( + err = New(0, "dummy error", Meta("meta", "meta")) + buf = bytes.NewBuffer(nil) + req = require.New(t) + ) - // Output: - // {"error":{"message":"dummy error","meta":{"a":"b"}}} + buf.Truncate(0) + writeHttpJSON(context.Background(), buf, err, false) + req.Contains(buf.String(), "dummy error") + req.Contains(buf.String(), "meta") + req.Contains(buf.String(), "stack") + + // when errors are masked (production env) we do not add meta or stack + buf.Truncate(0) + writeHttpJSON(context.Background(), buf, err, true) + req.Contains(buf.String(), "dummy error") + req.NotContains(buf.String(), "meta") + req.NotContains(buf.String(), "stack") }