Skip to content

Commit

Permalink
Fix how errors are reported/masked
Browse files Browse the repository at this point in the history
  • Loading branch information
darh committed Jan 21, 2022
1 parent 320f069 commit b1b741a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 17 deletions.
22 changes: 15 additions & 7 deletions pkg/errors/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand All @@ -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)

Expand All @@ -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")

Expand All @@ -105,22 +112,23 @@ 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 {
Error interface{} `json:"error"`
}{}
)

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())
}

Expand Down
43 changes: 33 additions & 10 deletions pkg/errors/http_test.go
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -21,22 +24,42 @@ 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, 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)
writeHttpPlain(os.Stdout, err, false)
// Output:
// Error: dummy error
// --------------------------------------------------------------------------------
// a: b
// --------------------------------------------------------------------------------
}

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")
}

0 comments on commit b1b741a

Please sign in to comment.