From 15fc5292a8f08c4d2b95eb8b6653478597b24f16 Mon Sep 17 00:00:00 2001 From: Gus Date: Wed, 15 Oct 2025 10:36:25 +0800 Subject: [PATCH 1/7] Enhance HTTP error sentry context --- pkg/http/handler.go | 97 ++++++++++++++++++++++++++++++++++++++-- pkg/http/handler_test.go | 50 +++++++++++++++++++++ pkg/http/response.go | 3 ++ pkg/http/schema.go | 9 ++++ pkg/http/schema_test.go | 27 ++++++++++- 5 files changed, 181 insertions(+), 5 deletions(-) diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 26bbf868..3f66a04f 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -2,11 +2,15 @@ package http import ( "encoding/json" + "errors" + "fmt" "log/slog" baseHttp "net/http" "strconv" + "strings" "github.com/getsentry/sentry-go" + "github.com/oullin/pkg/portal" ) func MakeApiHandler(fn ApiHandler) baseHttp.HandlerFunc { @@ -42,20 +46,28 @@ func captureApiError(r *baseHttp.Request, apiErr *ApiError) { level = sentry.LevelError } + errToCapture := error(apiErr) + if apiErr.Err != nil { + errToCapture = apiErr.Err + } + notify := func(hub *sentry.Hub) { hub.WithScope(func(scope *sentry.Scope) { scope.SetLevel(level) scope.SetTag("http.method", r.Method) scope.SetTag("http.status_code", strconv.Itoa(apiErr.Status)) scope.SetTag("http.route", r.URL.Path) + if requestID := requestIDFrom(r); requestID != "" { + scope.SetTag("http.request_id", requestID) + scope.SetExtra("http_request_id", requestID) + } scope.SetRequest(r) scope.SetExtra("api_error_status_text", baseHttp.StatusText(apiErr.Status)) + scope.SetExtra("api_error_message", apiErr.Message) - if apiErr.Data != nil { - scope.SetExtra("api_error_data", apiErr.Data) - } + enrichScopeWithApiError(scope, r, apiErr) - hub.CaptureException(apiErr) + hub.CaptureException(errToCapture) }) } @@ -66,3 +78,80 @@ func captureApiError(r *baseHttp.Request, apiErr *ApiError) { notify(sentry.CurrentHub()) } + +func enrichScopeWithApiError(scope *sentry.Scope, r *baseHttp.Request, apiErr *ApiError) { + if apiErr.Data != nil { + scope.SetExtra("api_error_data", apiErr.Data) + } + + if apiErr.Err != nil { + scope.SetExtra("api_error_cause", apiErr.Err.Error()) + scope.SetTag("api.error.cause_type", fmt.Sprintf("%T", apiErr.Err)) + + if chain := buildErrorChain(apiErr.Err); len(chain) > 0 { + scope.SetExtra("api_error_cause_chain", chain) + } + } + + if accountName := accountNameFrom(r); accountName != "" { + scope.SetExtra("api_account_name", accountName) + } + + if username := headerValue(r, portal.UsernameHeader); username != "" { + scope.SetExtra("api_username_header", username) + } + + if origin := headerValue(r, portal.IntendedOriginHeader); origin != "" { + scope.SetExtra("api_intended_origin", origin) + } + + if ts := headerValue(r, portal.TimestampHeader); ts != "" { + scope.SetExtra("api_request_timestamp", ts) + } + + if nonce := headerValue(r, portal.NonceHeader); nonce != "" { + scope.SetExtra("api_request_nonce", nonce) + } + + if publicKey := headerValue(r, portal.TokenHeader); publicKey != "" { + scope.SetExtra("api_public_key", publicKey) + } + + if clientIP := strings.TrimSpace(portal.ParseClientIP(r)); clientIP != "" { + scope.SetExtra("http_client_ip", clientIP) + } +} + +func requestIDFrom(r *baseHttp.Request) string { + if v, ok := r.Context().Value(portal.RequestIDKey).(string); ok { + if id := strings.TrimSpace(v); id != "" { + return id + } + } + + return headerValue(r, portal.RequestIDHeader) +} + +func accountNameFrom(r *baseHttp.Request) string { + if v, ok := r.Context().Value(portal.AuthAccountNameKey).(string); ok { + if name := strings.TrimSpace(v); name != "" { + return name + } + } + + return headerValue(r, portal.UsernameHeader) +} + +func headerValue(r *baseHttp.Request, key string) string { + return strings.TrimSpace(r.Header.Get(key)) +} + +func buildErrorChain(err error) []string { + chain := make([]string, 0, 4) + + for current := err; current != nil; current = errors.Unwrap(current) { + chain = append(chain, current.Error()) + } + + return chain +} diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index 87e33c80..f202a2af 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -1,10 +1,15 @@ package http import ( + "context" "encoding/json" + "errors" + "fmt" "net/http" "net/http/httptest" "testing" + + "github.com/oullin/pkg/portal" ) func TestMakeApiHandler(t *testing.T) { @@ -33,3 +38,48 @@ func TestMakeApiHandler(t *testing.T) { t.Fatalf("invalid response") } } + +func TestRequestIDFrom(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set(portal.RequestIDHeader, "header-id") + + if got := requestIDFrom(req); got != "header-id" { + t.Fatalf("expected header request id, got %s", got) + } + + ctxReq := req.WithContext(context.WithValue(req.Context(), portal.RequestIDKey, "context-id")) + + if got := requestIDFrom(ctxReq); got != "context-id" { + t.Fatalf("expected context request id, got %s", got) + } +} + +func TestAccountNameFrom(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set(portal.UsernameHeader, "header-user") + + if got := accountNameFrom(req); got != "header-user" { + t.Fatalf("expected header user, got %s", got) + } + + ctxReq := req.WithContext(context.WithValue(req.Context(), portal.AuthAccountNameKey, "context-user")) + + if got := accountNameFrom(ctxReq); got != "context-user" { + t.Fatalf("expected context user, got %s", got) + } +} + +func TestBuildErrorChain(t *testing.T) { + root := errors.New("root") + wrapped := fmt.Errorf("layer: %w", root) + + chain := buildErrorChain(wrapped) + + if len(chain) != 2 { + t.Fatalf("expected 2 errors in chain, got %d", len(chain)) + } + + if chain[0] != wrapped.Error() || chain[1] != root.Error() { + t.Fatalf("unexpected error chain: %#v", chain) + } +} diff --git a/pkg/http/response.go b/pkg/http/response.go index 188d41f8..2636fd31 100644 --- a/pkg/http/response.go +++ b/pkg/http/response.go @@ -98,6 +98,7 @@ func LogInternalError(msg string, err error) *ApiError { return &ApiError{ Message: fmt.Sprintf("Internal server error: %s", msg), Status: baseHttp.StatusInternalServerError, + Err: err, } } @@ -114,6 +115,7 @@ func LogBadRequestError(msg string, err error) *ApiError { return &ApiError{ Message: fmt.Sprintf("Bad request error: %s", msg), Status: baseHttp.StatusBadRequest, + Err: err, } } @@ -123,6 +125,7 @@ func LogUnauthorisedError(msg string, err error) *ApiError { return &ApiError{ Message: fmt.Sprintf("Unauthorised request: %s", msg), Status: baseHttp.StatusUnauthorized, + Err: err, } } diff --git a/pkg/http/schema.go b/pkg/http/schema.go index 08c879ef..5492fd36 100644 --- a/pkg/http/schema.go +++ b/pkg/http/schema.go @@ -12,6 +12,7 @@ type ApiError struct { Message string `json:"message"` Status int `json:"status"` Data map[string]any `json:"data"` + Err error `json:"-"` } func (e *ApiError) Error() string { @@ -22,6 +23,14 @@ func (e *ApiError) Error() string { return e.Message } +func (e *ApiError) Unwrap() error { + if e == nil { + return nil + } + + return e.Err +} + type ApiHandler func(baseHttp.ResponseWriter, *baseHttp.Request) *ApiError type Middleware func(ApiHandler) ApiHandler diff --git a/pkg/http/schema_test.go b/pkg/http/schema_test.go index a68ae0f7..f34a07e5 100644 --- a/pkg/http/schema_test.go +++ b/pkg/http/schema_test.go @@ -1,6 +1,9 @@ package http -import "testing" +import ( + "errors" + "testing" +) func TestApiErrorError(t *testing.T) { e := &ApiError{ @@ -18,3 +21,25 @@ func TestApiErrorError(t *testing.T) { t.Fatalf("nil error wrong") } } + +func TestApiErrorUnwrap(t *testing.T) { + cause := errors.New("root cause") + e := &ApiError{ + Message: "boom", + Status: 500, + Err: cause, + } + + if !errors.Is(e, cause) { + t.Fatalf("expected errors.Is to match the wrapped cause") + } + + if got := e.Unwrap(); got != cause { + t.Fatalf("expected unwrap to return the cause") + } + + var nilErr *ApiError + if nilErr.Unwrap() != nil { + t.Fatalf("expected nil unwrap to be nil") + } +} From 0631f7739de3ab6b637765b61e45a8c4b5839d74 Mon Sep 17 00:00:00 2001 From: Gus Date: Wed, 15 Oct 2025 10:51:01 +0800 Subject: [PATCH 2/7] Refactor Sentry scope helper for API errors --- pkg/http/handler.go | 89 ++-------------------------- pkg/http/handler_test.go | 24 +++++--- pkg/http/scope_api_error.go | 114 ++++++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 91 deletions(-) create mode 100644 pkg/http/scope_api_error.go diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 3f66a04f..98c73ef4 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -2,15 +2,11 @@ package http import ( "encoding/json" - "errors" - "fmt" "log/slog" baseHttp "net/http" "strconv" - "strings" "github.com/getsentry/sentry-go" - "github.com/oullin/pkg/portal" ) func MakeApiHandler(fn ApiHandler) baseHttp.HandlerFunc { @@ -57,15 +53,19 @@ func captureApiError(r *baseHttp.Request, apiErr *ApiError) { scope.SetTag("http.method", r.Method) scope.SetTag("http.status_code", strconv.Itoa(apiErr.Status)) scope.SetTag("http.route", r.URL.Path) - if requestID := requestIDFrom(r); requestID != "" { + + scopeApiError := NewScopeApiError(scope, r, apiErr) + + if requestID := scopeApiError.RequestID(); requestID != "" { scope.SetTag("http.request_id", requestID) scope.SetExtra("http_request_id", requestID) } + scope.SetRequest(r) scope.SetExtra("api_error_status_text", baseHttp.StatusText(apiErr.Status)) scope.SetExtra("api_error_message", apiErr.Message) - enrichScopeWithApiError(scope, r, apiErr) + scopeApiError.Enrich() hub.CaptureException(errToCapture) }) @@ -78,80 +78,3 @@ func captureApiError(r *baseHttp.Request, apiErr *ApiError) { notify(sentry.CurrentHub()) } - -func enrichScopeWithApiError(scope *sentry.Scope, r *baseHttp.Request, apiErr *ApiError) { - if apiErr.Data != nil { - scope.SetExtra("api_error_data", apiErr.Data) - } - - if apiErr.Err != nil { - scope.SetExtra("api_error_cause", apiErr.Err.Error()) - scope.SetTag("api.error.cause_type", fmt.Sprintf("%T", apiErr.Err)) - - if chain := buildErrorChain(apiErr.Err); len(chain) > 0 { - scope.SetExtra("api_error_cause_chain", chain) - } - } - - if accountName := accountNameFrom(r); accountName != "" { - scope.SetExtra("api_account_name", accountName) - } - - if username := headerValue(r, portal.UsernameHeader); username != "" { - scope.SetExtra("api_username_header", username) - } - - if origin := headerValue(r, portal.IntendedOriginHeader); origin != "" { - scope.SetExtra("api_intended_origin", origin) - } - - if ts := headerValue(r, portal.TimestampHeader); ts != "" { - scope.SetExtra("api_request_timestamp", ts) - } - - if nonce := headerValue(r, portal.NonceHeader); nonce != "" { - scope.SetExtra("api_request_nonce", nonce) - } - - if publicKey := headerValue(r, portal.TokenHeader); publicKey != "" { - scope.SetExtra("api_public_key", publicKey) - } - - if clientIP := strings.TrimSpace(portal.ParseClientIP(r)); clientIP != "" { - scope.SetExtra("http_client_ip", clientIP) - } -} - -func requestIDFrom(r *baseHttp.Request) string { - if v, ok := r.Context().Value(portal.RequestIDKey).(string); ok { - if id := strings.TrimSpace(v); id != "" { - return id - } - } - - return headerValue(r, portal.RequestIDHeader) -} - -func accountNameFrom(r *baseHttp.Request) string { - if v, ok := r.Context().Value(portal.AuthAccountNameKey).(string); ok { - if name := strings.TrimSpace(v); name != "" { - return name - } - } - - return headerValue(r, portal.UsernameHeader) -} - -func headerValue(r *baseHttp.Request, key string) string { - return strings.TrimSpace(r.Header.Get(key)) -} - -func buildErrorChain(err error) []string { - chain := make([]string, 0, 4) - - for current := err; current != nil; current = errors.Unwrap(current) { - chain = append(chain, current.Error()) - } - - return chain -} diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index f202a2af..a2001783 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -39,41 +39,49 @@ func TestMakeApiHandler(t *testing.T) { } } -func TestRequestIDFrom(t *testing.T) { +func TestScopeApiErrorRequestID(t *testing.T) { req := httptest.NewRequest("GET", "/", nil) req.Header.Set(portal.RequestIDHeader, "header-id") - if got := requestIDFrom(req); got != "header-id" { + scopeApiError := &ScopeApiError{request: req} + + if got := scopeApiError.RequestID(); got != "header-id" { t.Fatalf("expected header request id, got %s", got) } ctxReq := req.WithContext(context.WithValue(req.Context(), portal.RequestIDKey, "context-id")) - if got := requestIDFrom(ctxReq); got != "context-id" { + scopeApiError.request = ctxReq + + if got := scopeApiError.RequestID(); got != "context-id" { t.Fatalf("expected context request id, got %s", got) } } -func TestAccountNameFrom(t *testing.T) { +func TestScopeApiErrorAccountName(t *testing.T) { req := httptest.NewRequest("GET", "/", nil) req.Header.Set(portal.UsernameHeader, "header-user") - if got := accountNameFrom(req); got != "header-user" { + scopeApiError := &ScopeApiError{request: req} + + if got := scopeApiError.accountName(); got != "header-user" { t.Fatalf("expected header user, got %s", got) } ctxReq := req.WithContext(context.WithValue(req.Context(), portal.AuthAccountNameKey, "context-user")) - if got := accountNameFrom(ctxReq); got != "context-user" { + scopeApiError.request = ctxReq + + if got := scopeApiError.accountName(); got != "context-user" { t.Fatalf("expected context user, got %s", got) } } -func TestBuildErrorChain(t *testing.T) { +func TestScopeApiErrorBuildErrorChain(t *testing.T) { root := errors.New("root") wrapped := fmt.Errorf("layer: %w", root) - chain := buildErrorChain(wrapped) + chain := (&ScopeApiError{}).buildErrorChain(wrapped) if len(chain) != 2 { t.Fatalf("expected 2 errors in chain, got %d", len(chain)) diff --git a/pkg/http/scope_api_error.go b/pkg/http/scope_api_error.go new file mode 100644 index 00000000..49351e86 --- /dev/null +++ b/pkg/http/scope_api_error.go @@ -0,0 +1,114 @@ +package http + +import ( + "errors" + "fmt" + baseHttp "net/http" + "strings" + + "github.com/getsentry/sentry-go" + "github.com/oullin/pkg/portal" +) + +type ScopeApiError struct { + scope *sentry.Scope + request *baseHttp.Request + apiErr *ApiError +} + +func NewScopeApiError(scope *sentry.Scope, r *baseHttp.Request, apiErr *ApiError) *ScopeApiError { + return &ScopeApiError{scope: scope, request: r, apiErr: apiErr} +} + +func (s *ScopeApiError) RequestID() string { + if s == nil || s.request == nil { + return "" + } + + if v, ok := s.request.Context().Value(portal.RequestIDKey).(string); ok { + if id := strings.TrimSpace(v); id != "" { + return id + } + } + + return s.headerValue(portal.RequestIDHeader) +} + +func (s *ScopeApiError) Enrich() { + if s == nil || s.scope == nil || s.request == nil || s.apiErr == nil { + return + } + + if s.apiErr.Data != nil { + s.scope.SetExtra("api_error_data", s.apiErr.Data) + } + + if s.apiErr.Err != nil { + s.scope.SetExtra("api_error_cause", s.apiErr.Err.Error()) + s.scope.SetTag("api.error.cause_type", fmt.Sprintf("%T", s.apiErr.Err)) + + if chain := s.buildErrorChain(s.apiErr.Err); len(chain) > 0 { + s.scope.SetExtra("api_error_cause_chain", chain) + } + } + + if accountName := s.accountName(); accountName != "" { + s.scope.SetExtra("api_account_name", accountName) + } + + if username := s.headerValue(portal.UsernameHeader); username != "" { + s.scope.SetExtra("api_username_header", username) + } + + if origin := s.headerValue(portal.IntendedOriginHeader); origin != "" { + s.scope.SetExtra("api_intended_origin", origin) + } + + if ts := s.headerValue(portal.TimestampHeader); ts != "" { + s.scope.SetExtra("api_request_timestamp", ts) + } + + if nonce := s.headerValue(portal.NonceHeader); nonce != "" { + s.scope.SetExtra("api_request_nonce", nonce) + } + + if publicKey := s.headerValue(portal.TokenHeader); publicKey != "" { + s.scope.SetExtra("api_public_key", publicKey) + } + + if clientIP := strings.TrimSpace(portal.ParseClientIP(s.request)); clientIP != "" { + s.scope.SetExtra("http_client_ip", clientIP) + } +} + +func (s *ScopeApiError) accountName() string { + if s == nil || s.request == nil { + return "" + } + + if v, ok := s.request.Context().Value(portal.AuthAccountNameKey).(string); ok { + if name := strings.TrimSpace(v); name != "" { + return name + } + } + + return s.headerValue(portal.UsernameHeader) +} + +func (s *ScopeApiError) headerValue(key string) string { + if s == nil || s.request == nil { + return "" + } + + return strings.TrimSpace(s.request.Header.Get(key)) +} + +func (s *ScopeApiError) buildErrorChain(err error) []string { + chain := make([]string, 0, 4) + + for current := err; current != nil; current = errors.Unwrap(current) { + chain = append(chain, current.Error()) + } + + return chain +} From 6c260421e27f47740e36790d5b21bcd101a81df6 Mon Sep 17 00:00:00 2001 From: Gus Date: Wed, 15 Oct 2025 11:00:29 +0800 Subject: [PATCH 3/7] Remove redundant API error chain guard --- pkg/http/scope_api_error.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/http/scope_api_error.go b/pkg/http/scope_api_error.go index 49351e86..feb2b4c3 100644 --- a/pkg/http/scope_api_error.go +++ b/pkg/http/scope_api_error.go @@ -47,9 +47,7 @@ func (s *ScopeApiError) Enrich() { s.scope.SetExtra("api_error_cause", s.apiErr.Err.Error()) s.scope.SetTag("api.error.cause_type", fmt.Sprintf("%T", s.apiErr.Err)) - if chain := s.buildErrorChain(s.apiErr.Err); len(chain) > 0 { - s.scope.SetExtra("api_error_cause_chain", chain) - } + s.scope.SetExtra("api_error_cause_chain", s.buildErrorChain(s.apiErr.Err)) } if accountName := s.accountName(); accountName != "" { From 803ed51bd0f9feef9d778828055466bfc1cdbdde Mon Sep 17 00:00:00 2001 From: Gus Date: Wed, 15 Oct 2025 11:33:39 +0800 Subject: [PATCH 4/7] Inline request ID tagging --- pkg/http/handler.go | 5 ----- pkg/http/scope_api_error.go | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 98c73ef4..703f2624 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -56,11 +56,6 @@ func captureApiError(r *baseHttp.Request, apiErr *ApiError) { scopeApiError := NewScopeApiError(scope, r, apiErr) - if requestID := scopeApiError.RequestID(); requestID != "" { - scope.SetTag("http.request_id", requestID) - scope.SetExtra("http_request_id", requestID) - } - scope.SetRequest(r) scope.SetExtra("api_error_status_text", baseHttp.StatusText(apiErr.Status)) scope.SetExtra("api_error_message", apiErr.Message) diff --git a/pkg/http/scope_api_error.go b/pkg/http/scope_api_error.go index feb2b4c3..bc8b7b44 100644 --- a/pkg/http/scope_api_error.go +++ b/pkg/http/scope_api_error.go @@ -39,6 +39,11 @@ func (s *ScopeApiError) Enrich() { return } + if requestID := s.RequestID(); requestID != "" { + s.scope.SetTag("http.request_id", requestID) + s.scope.SetExtra("http_request_id", requestID) + } + if s.apiErr.Data != nil { s.scope.SetExtra("api_error_data", s.apiErr.Data) } From 2a11646a43449c96fd2cf27b9a37fd00312f76b8 Mon Sep 17 00:00:00 2001 From: Gus Date: Wed, 15 Oct 2025 11:45:46 +0800 Subject: [PATCH 5/7] Move request metadata enrichment into ScopeApiError --- pkg/http/handler.go | 4 ---- pkg/http/scope_api_error.go | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 703f2624..bc5a15b4 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -56,10 +56,6 @@ func captureApiError(r *baseHttp.Request, apiErr *ApiError) { scopeApiError := NewScopeApiError(scope, r, apiErr) - scope.SetRequest(r) - scope.SetExtra("api_error_status_text", baseHttp.StatusText(apiErr.Status)) - scope.SetExtra("api_error_message", apiErr.Message) - scopeApiError.Enrich() hub.CaptureException(errToCapture) diff --git a/pkg/http/scope_api_error.go b/pkg/http/scope_api_error.go index bc8b7b44..c46eab60 100644 --- a/pkg/http/scope_api_error.go +++ b/pkg/http/scope_api_error.go @@ -39,6 +39,10 @@ func (s *ScopeApiError) Enrich() { return } + s.scope.SetRequest(s.request) + s.scope.SetExtra("api_error_status_text", baseHttp.StatusText(s.apiErr.Status)) + s.scope.SetExtra("api_error_message", s.apiErr.Message) + if requestID := s.RequestID(); requestID != "" { s.scope.SetTag("http.request_id", requestID) s.scope.SetExtra("http_request_id", requestID) From 5cfa9bc0d1ddefd5c004e074475873890dfd7714 Mon Sep 17 00:00:00 2001 From: Gus Date: Wed, 15 Oct 2025 11:53:53 +0800 Subject: [PATCH 6/7] Refine scope API error enrichment --- pkg/http/handler.go | 11 --------- pkg/http/handler_test.go | 49 +++++++++++++++++++++++++++++++++++++ pkg/http/scope_api_error.go | 11 +++++++++ 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/pkg/http/handler.go b/pkg/http/handler.go index bc5a15b4..278f0545 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -4,7 +4,6 @@ import ( "encoding/json" "log/slog" baseHttp "net/http" - "strconv" "github.com/getsentry/sentry-go" ) @@ -37,11 +36,6 @@ func captureApiError(r *baseHttp.Request, apiErr *ApiError) { return } - level := sentry.LevelWarning - if apiErr.Status >= baseHttp.StatusInternalServerError { - level = sentry.LevelError - } - errToCapture := error(apiErr) if apiErr.Err != nil { errToCapture = apiErr.Err @@ -49,11 +43,6 @@ func captureApiError(r *baseHttp.Request, apiErr *ApiError) { notify := func(hub *sentry.Hub) { hub.WithScope(func(scope *sentry.Scope) { - scope.SetLevel(level) - scope.SetTag("http.method", r.Method) - scope.SetTag("http.status_code", strconv.Itoa(apiErr.Status)) - scope.SetTag("http.route", r.URL.Path) - scopeApiError := NewScopeApiError(scope, r, apiErr) scopeApiError.Enrich() diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index a2001783..17f5bbeb 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "testing" + "github.com/getsentry/sentry-go" "github.com/oullin/pkg/portal" ) @@ -91,3 +92,51 @@ func TestScopeApiErrorBuildErrorChain(t *testing.T) { t.Fatalf("unexpected error chain: %#v", chain) } } + +func TestScopeApiErrorEnrichSetsLevelAndTags(t *testing.T) { + scope := sentry.NewScope() + req := httptest.NewRequest("POST", "/resource", nil) + + apiErr := &ApiError{Status: http.StatusInternalServerError} + + NewScopeApiError(scope, req, apiErr).Enrich() + + event := scope.ApplyToEvent(sentry.NewEvent(), nil, nil) + if event == nil { + t.Fatalf("expected event after scope enrichment") + } + + if event.Level != sentry.LevelError { + t.Fatalf("expected error level, got %s", event.Level) + } + + if got := event.Tags["http.method"]; got != "POST" { + t.Fatalf("expected POST method tag, got %s", got) + } + + if got := event.Tags["http.status_code"]; got != "500" { + t.Fatalf("expected 500 status code tag, got %s", got) + } + + if got := event.Tags["http.route"]; got != "/resource" { + t.Fatalf("expected /resource route tag, got %s", got) + } +} + +func TestScopeApiErrorEnrichSetsWarningLevelForClientErrors(t *testing.T) { + scope := sentry.NewScope() + req := httptest.NewRequest("GET", "/client", nil) + + apiErr := &ApiError{Status: http.StatusBadRequest} + + NewScopeApiError(scope, req, apiErr).Enrich() + + event := scope.ApplyToEvent(sentry.NewEvent(), nil, nil) + if event == nil { + t.Fatalf("expected event after scope enrichment") + } + + if event.Level != sentry.LevelWarning { + t.Fatalf("expected warning level, got %s", event.Level) + } +} diff --git a/pkg/http/scope_api_error.go b/pkg/http/scope_api_error.go index c46eab60..34b10a6c 100644 --- a/pkg/http/scope_api_error.go +++ b/pkg/http/scope_api_error.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" baseHttp "net/http" + "strconv" "strings" "github.com/getsentry/sentry-go" @@ -39,6 +40,16 @@ func (s *ScopeApiError) Enrich() { return } + level := sentry.LevelWarning + if s.apiErr.Status >= baseHttp.StatusInternalServerError { + level = sentry.LevelError + } + + s.scope.SetLevel(level) + s.scope.SetTag("http.method", s.request.Method) + s.scope.SetTag("http.status_code", strconv.Itoa(s.apiErr.Status)) + s.scope.SetTag("http.route", s.request.URL.Path) + s.scope.SetRequest(s.request) s.scope.SetExtra("api_error_status_text", baseHttp.StatusText(s.apiErr.Status)) s.scope.SetExtra("api_error_message", s.apiErr.Message) From 69e5ad2ab4b35abd92eec8cc6945fb972901f22e Mon Sep 17 00:00:00 2001 From: Gus Date: Wed, 15 Oct 2025 12:04:10 +0800 Subject: [PATCH 7/7] Ensure ApiError literals include causes --- pkg/http/handler_test.go | 5 +++-- pkg/http/response.go | 25 +++++++++++++++++++------ pkg/http/schema_test.go | 1 + 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index 17f5bbeb..cbc8d603 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -19,6 +19,7 @@ func TestMakeApiHandler(t *testing.T) { return &ApiError{ Message: "bad", Status: http.StatusBadRequest, + Err: errors.New("bad"), } }) @@ -97,7 +98,7 @@ func TestScopeApiErrorEnrichSetsLevelAndTags(t *testing.T) { scope := sentry.NewScope() req := httptest.NewRequest("POST", "/resource", nil) - apiErr := &ApiError{Status: http.StatusInternalServerError} + apiErr := &ApiError{Status: http.StatusInternalServerError, Err: errors.New("boom")} NewScopeApiError(scope, req, apiErr).Enrich() @@ -127,7 +128,7 @@ func TestScopeApiErrorEnrichSetsWarningLevelForClientErrors(t *testing.T) { scope := sentry.NewScope() req := httptest.NewRequest("GET", "/client", nil) - apiErr := &ApiError{Status: http.StatusBadRequest} + apiErr := &ApiError{Status: http.StatusBadRequest, Err: errors.New("bad request")} NewScopeApiError(scope, req, apiErr).Enrich() diff --git a/pkg/http/response.go b/pkg/http/response.go index 2636fd31..ffeb7548 100644 --- a/pkg/http/response.go +++ b/pkg/http/response.go @@ -2,6 +2,7 @@ package http import ( "encoding/json" + "errors" "fmt" "log/slog" baseHttp "net/http" @@ -86,9 +87,12 @@ func (r *Response) RespondWithNotModified() { } func InternalError(msg string) *ApiError { + message := fmt.Sprintf("Internal server error: %s", msg) + return &ApiError{ - Message: fmt.Sprintf("Internal server error: %s", msg), + Message: message, Status: baseHttp.StatusInternalServerError, + Err: errors.New(message), } } @@ -103,9 +107,12 @@ func LogInternalError(msg string, err error) *ApiError { } func BadRequestError(msg string) *ApiError { + message := fmt.Sprintf("Bad request error: %s", msg) + return &ApiError{ - Message: fmt.Sprintf("Bad request error: %s", msg), + Message: message, Status: baseHttp.StatusBadRequest, + Err: errors.New(message), } } @@ -129,17 +136,23 @@ func LogUnauthorisedError(msg string, err error) *ApiError { } } -func UnprocessableEntity(msg string, errors map[string]any) *ApiError { +func UnprocessableEntity(msg string, errs map[string]any) *ApiError { + message := fmt.Sprintf("Unprocessable entity: %s", msg) + return &ApiError{ - Message: fmt.Sprintf("Unprocessable entity: %s", msg), + Message: message, Status: baseHttp.StatusUnprocessableEntity, - Data: errors, + Data: errs, + Err: errors.New(message), } } func NotFound(msg string) *ApiError { + message := fmt.Sprintf("Not found error: %s", msg) + return &ApiError{ - Message: fmt.Sprintf("Not found error: %s", msg), + Message: message, Status: baseHttp.StatusNotFound, + Err: errors.New(message), } } diff --git a/pkg/http/schema_test.go b/pkg/http/schema_test.go index f34a07e5..9a2d9bd8 100644 --- a/pkg/http/schema_test.go +++ b/pkg/http/schema_test.go @@ -9,6 +9,7 @@ func TestApiErrorError(t *testing.T) { e := &ApiError{ Message: "boom", Status: 500, + Err: errors.New("boom"), } if e.Error() != "boom" {