From be4d5fa0a3d662f1cb5a2e057732c3ae5a4e6c61 Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Mon, 19 Aug 2024 21:30:43 -0400 Subject: [PATCH 1/2] Add a method to set a public-facing message --- README.md | 1 + builder.go | 17 +++++++++++++---- error.go | 34 +++++++++++++++++++++++++++------- oops_test.go | 26 +++++++++++++++++++++++--- 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index ae665a1..7b83641 100644 --- a/README.md +++ b/README.md @@ -249,6 +249,7 @@ The `oops.OopsError` builder must finish with either `.Errorf(...)`, `.Wrap(...) | `.Trace(string)` | `err.Trace() string` | Add a transaction id, trace id, correlation id... (default: ULID) | | `.Span(string)` | `err.Span() string` | Add a span representing a unit of work or operation... (default: ULID) | | `.Hint(string)` | `err.Hint() string` | Set a hint for faster debugging | +| `.Public(string)` | `err.Public() string` | Set a message that is safe to show to an end user | | `.Owner(string)` | `err.Owner() (string)` | Set the name/email of the collegue/team responsible for handling this error. Useful for alerting purpose | | `.User(string, any...)` | `err.User() (string, map[string]any)` | Supply user id and a chain of key/value | | `.Tenant(string, any...)` | `err.Tenant() (string, map[string]any)` | Supply tenant id and a chain of key/value | diff --git a/builder.go b/builder.go index 2fdb136..f86ab9a 100644 --- a/builder.go +++ b/builder.go @@ -50,8 +50,9 @@ func new() OopsErrorBuilder { trace: "", span: "", - hint: "", - owner: "", + hint: "", + public: "", + owner: "", // user userID: "", @@ -83,8 +84,9 @@ func (o OopsErrorBuilder) copy() OopsErrorBuilder { trace: o.trace, span: o.span, - hint: o.hint, - owner: o.owner, + hint: o.hint, + public: o.public, + owner: o.owner, userID: o.userID, userData: lo.Assign(map[string]any{}, o.userData), @@ -287,6 +289,13 @@ func (o OopsErrorBuilder) Hint(hint string) OopsErrorBuilder { return o2 } +// Public represents a message that is safe to be shown to an end-user. +func (o OopsErrorBuilder) Public(public string) OopsErrorBuilder { + o2 := o.copy() + o2.public = public + return o2 +} + // Owner set the name/email of the collegue/team responsible for handling this error. // Useful for alerting purpose. func (o OopsErrorBuilder) Owner(owner string) OopsErrorBuilder { diff --git a/error.go b/error.go index 8d0f50e..6abfc9f 100644 --- a/error.go +++ b/error.go @@ -3,20 +3,21 @@ package oops import ( "encoding/json" "fmt" + "log/slog" "net/http" "net/http/httputil" "strings" "time" - "log/slog" - "github.com/oklog/ulid/v2" "github.com/samber/lo" ) -var SourceFragmentsHidden = true -var DereferencePointers = true -var Local *time.Location = time.UTC +var ( + SourceFragmentsHidden = true + DereferencePointers = true + Local *time.Location = time.UTC +) var _ error = (*OopsError)(nil) @@ -35,8 +36,9 @@ type OopsError struct { trace string span string - hint string - owner string + hint string + public string + owner string // user userID string @@ -170,6 +172,16 @@ func (o OopsError) Hint() string { ) } +// Public returns a message that is safe to show to an end user. +func (o OopsError) Public() string { + return getDeepestErrorAttribute( + o, + func(e OopsError) string { + return e.public + }, + ) +} + // Owner identify the owner responsible for resolving the error. func (o OopsError) Owner() string { return getDeepestErrorAttribute( @@ -355,6 +367,10 @@ func (o OopsError) LogValuer() slog.Value { attrs = append(attrs, slog.String("hint", hint)) } + if public := o.Public(); public != "" { + attrs = append(attrs, slog.String("public", public)) + } + if owner := o.Owner(); owner != "" { attrs = append(attrs, slog.String("owner", owner)) } @@ -471,6 +487,10 @@ func (o OopsError) ToMap() map[string]any { payload["hint"] = hint } + if public := o.Public(); public != "" { + payload["public"] = public + } + if owner := o.Owner(); owner != "" { payload["owner"] = owner } diff --git a/oops_test.go b/oops_test.go index fee3915..5c3a0c8 100644 --- a/oops_test.go +++ b/oops_test.go @@ -4,13 +4,12 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" "strings" "testing" "time" - "log/slog" - "github.com/samber/lo" "github.com/stretchr/testify/assert" ) @@ -197,6 +196,15 @@ func TestOopsHint(t *testing.T) { is.Equal("Runbook: https://doc.acme.org/doc/abcd.md", err.(OopsError).hint) } +func TestOopsPublic(t *testing.T) { + is := assert.New(t) + + err := new().Public("a public facing message").Wrap(assert.AnError) + is.Error(err) + is.Equal(assert.AnError, err.(OopsError).err) + is.Equal("a public facing message", err.(OopsError).public) +} + func TestOopsOwner(t *testing.T) { is := assert.New(t) @@ -309,6 +317,7 @@ func TestOopsMixed(t *testing.T) { With("user_id", 1234). WithContext(context.WithValue(context.Background(), "foo", "bar"), "foo"). //nolint:staticcheck Hint("Runbook: https://doc.acme.org/doc/abcd.md"). + Public("public facing message"). Owner("authz-team@acme.org"). User("user-123", "firstname", "john", "lastname", "doe"). Tenant("workspace-123", "name", "little project"). @@ -322,6 +331,7 @@ func TestOopsMixed(t *testing.T) { is.Equal(err.(OopsError).trace, "1234") is.Equal(err.(OopsError).context, map[string]any{"user_id": 1234, "foo": "bar"}) is.Equal(err.(OopsError).hint, "Runbook: https://doc.acme.org/doc/abcd.md") + is.Equal(err.(OopsError).public, "public facing message") is.Equal(err.(OopsError).owner, "authz-team@acme.org") is.Equal(err.(OopsError).userID, "user-123") is.Equal(err.(OopsError).userData, map[string]any{"firstname": "john", "lastname": "doe"}) @@ -347,6 +357,7 @@ func TestOopsMixedWithGetters(t *testing.T) { Trace("1234"). With("user_id", 1234). Hint("Runbook: https://doc.acme.org/doc/1234.md"). + Public("public facing message"). Owner("authz-team@acme.org"). User("user-123", "firstname", "bob", "lastname", "martin"). Tenant("workspace-123", "name", "little project"). @@ -361,6 +372,7 @@ func TestOopsMixedWithGetters(t *testing.T) { Trace("abcd"). With("workspace_id", 5678). Hint("Runbook: https://doc.acme.org/doc/abcd.md"). + Public("public facing message"). Owner("iam-team@acme.org"). User("user-123", "firstname", "john", "lastname", "doe", "email", "john@doe.org"). Tenant("workspace-123", "name", "little project", "deleted", false). @@ -376,6 +388,7 @@ func TestOopsMixedWithGetters(t *testing.T) { is.Equal(err.(OopsError).Trace(), "1234") is.Equal(err.(OopsError).Context(), map[string]any{"user_id": 1234, "workspace_id": 5678}) is.Equal(err.(OopsError).Hint(), "Runbook: https://doc.acme.org/doc/1234.md") + is.Equal(err.(OopsError).Public(), "public facing message") is.Equal(err.(OopsError).Owner(), "authz-team@acme.org") is.Equal(lo.T2(err.(OopsError).User()), lo.T2("user-123", map[string]any{"firstname": "bob", "lastname": "martin", "email": "john@doe.org"})) is.Equal(lo.T2(err.(OopsError).Tenant()), lo.T2("workspace-123", map[string]any{"name": "little project", "deleted": false})) @@ -391,6 +404,7 @@ func TestOopsMixedWithGetters(t *testing.T) { is.Equal(err.(OopsError).trace, "abcd") is.Equal(err.(OopsError).context, map[string]any{"workspace_id": 5678}) is.Equal(err.(OopsError).hint, "Runbook: https://doc.acme.org/doc/abcd.md") + is.Equal(err.(OopsError).public, "public facing message") is.Equal(err.(OopsError).owner, "iam-team@acme.org") is.Equal(err.(OopsError).userID, "user-123") is.Equal(err.(OopsError).userData, map[string]any{"email": "john@doe.org", "firstname": "john", "lastname": "doe"}) @@ -408,6 +422,7 @@ func TestOopsMixedWithGetters(t *testing.T) { is.Equal(err.(OopsError).Unwrap().(OopsError).trace, "1234") is.Equal(err.(OopsError).Unwrap().(OopsError).context, map[string]any{"user_id": 1234}) is.Equal(err.(OopsError).Unwrap().(OopsError).hint, "Runbook: https://doc.acme.org/doc/1234.md") + is.Equal(err.(OopsError).Unwrap().(OopsError).public, "public facing message") is.Equal(err.(OopsError).Unwrap().(OopsError).owner, "authz-team@acme.org") is.Equal(err.(OopsError).Unwrap().(OopsError).userID, "user-123") is.Equal(err.(OopsError).Unwrap().(OopsError).userData, map[string]any{"firstname": "bob", "lastname": "martin"}) @@ -433,6 +448,7 @@ func TestOopsLogValuer(t *testing.T) { Trace("1234"). With("user_id", 1234). Hint("Runbook: https://doc.acme.org/doc/abcd.md"). + Public("public facing message"). Owner("authz-team@acme.org"). User("user-123", "firstname", "john"). Tenant("workspace-123", "name", "little project"). @@ -452,6 +468,7 @@ func TestOopsLogValuer(t *testing.T) { slog.Any("tags", []string{"iam", "authz"}), slog.String("trace", "1234"), slog.String("hint", "Runbook: https://doc.acme.org/doc/abcd.md"), + slog.String("public", "public facing message"), slog.String("owner", "authz-team@acme.org"), slog.Group( "context", @@ -493,6 +510,7 @@ func TestOopsFormatSummary(t *testing.T) { Trace("1234"). With("user_id", 1234). Hint("Runbook: https://doc.acme.org/doc/abcd.md"). + Public("public facing message"). Owner("authz-team@acme.org"). User("user-123", "firstname", "john", "lastname", "doe"). Tenant("workspace-123", "name", "little project"). @@ -517,6 +535,7 @@ func TestOopsFormatVerbose(t *testing.T) { Trace("1234"). With("user_id", 1234). Hint("Runbook: https://doc.acme.org/doc/abcd.md"). + Public("public facing message"). Owner("authz-team@acme.org"). User("user-123", "firstname", "john"). Tenant("workspace-123", "name", "little project"). @@ -568,12 +587,13 @@ func TestOopsMarshalJSON(t *testing.T) { Trace("1234"). With("user_id", 1234). Hint("Runbook: https://doc.acme.org/doc/abcd.md"). + Public("public facing message"). User("user-123", "firstname", "john", "lastname", "doe"). Tenant("workspace-123", "name", "little project"). Request(req, true). Wrapf(assert.AnError, "a message %d", 42) - expected := `{"code":"iam_missing_permission","context":{"user_id":1234},"domain":"authz","duration":"1s","error":"a message 42: assert.AnError general error for testing","hint":"Runbook: https://doc.acme.org/doc/abcd.md","request":"POST /foobar HTTP/1.1\r\nHost: localhost:1337\r\nUser-Agent: Go-http-client/1.1\r\nContent-Length: 11\r\nAccept-Encoding: gzip\r\n\r\nhello world","tenant":{"id":"workspace-123","name":"little project"},"time":"2023-05-02T05:26:48.570837Z","trace":"1234","user":{"firstname":"john","id":"user-123","lastname":"doe"}}` + expected := `{"code":"iam_missing_permission","context":{"user_id":1234},"domain":"authz","duration":"1s","error":"a message 42: assert.AnError general error for testing","hint":"Runbook: https://doc.acme.org/doc/abcd.md","public":"public facing message","request":"POST /foobar HTTP/1.1\r\nHost: localhost:1337\r\nUser-Agent: Go-http-client/1.1\r\nContent-Length: 11\r\nAccept-Encoding: gzip\r\n\r\nhello world","tenant":{"id":"workspace-123","name":"little project"},"time":"2023-05-02T05:26:48.570837Z","trace":"1234","user":{"firstname":"john","id":"user-123","lastname":"doe"}}` got, err := json.Marshal(withoutStacktrace(err.(OopsError))) is.NoError(err) From 6dced1a3435f888dad51233424c67a714b7e030f Mon Sep 17 00:00:00 2001 From: Ken Powers Date: Tue, 20 Aug 2024 14:45:46 -0400 Subject: [PATCH 2/2] Add oops.GetPublic function --- oops.go | 15 +++++++++++++++ oops_test.go | 10 ++++++++++ 2 files changed, 25 insertions(+) diff --git a/oops.go b/oops.go index 8931f11..c00ee56 100644 --- a/oops.go +++ b/oops.go @@ -2,6 +2,7 @@ package oops import ( "context" + "errors" "net/http" "time" ) @@ -140,3 +141,17 @@ func Request(req *http.Request, withBody bool) OopsErrorBuilder { func Response(res *http.Response, withBody bool) OopsErrorBuilder { return new().Response(res, withBody) } + +// GetPublic returns a message that is safe to show to an end user, or a default generic message. +func GetPublic(err error, defaultPublicMessage string) string { + var oopsError OopsError + + if errors.As(err, &oopsError) { + msg := oopsError.Public() + if len(msg) > 0 { + return msg + } + } + + return defaultPublicMessage +} diff --git a/oops_test.go b/oops_test.go index 5c3a0c8..e26bcc7 100644 --- a/oops_test.go +++ b/oops_test.go @@ -599,3 +599,13 @@ func TestOopsMarshalJSON(t *testing.T) { is.NoError(err) is.Equal(expected, string(got)) } + +func TestOopsGetPublic(t *testing.T) { + is := assert.New(t) + + err := new().Public("public facing message").Wrap(assert.AnError) + is.Error(err) + is.Equal(assert.AnError, err.(OopsError).err) + is.Equal("public facing message", GetPublic(err, "default message")) + is.Equal("default message", GetPublic(assert.AnError, "default message")) +}