diff --git a/e2e/tests/issue-338-numeric-project.spec.ts b/e2e/tests/issue-338-numeric-project.spec.ts new file mode 100644 index 0000000..0ccad18 --- /dev/null +++ b/e2e/tests/issue-338-numeric-project.spec.ts @@ -0,0 +1,71 @@ +import { test, expect } from '@playwright/test'; + +const BUGGREGATOR = 'http://localhost:8000'; + +// Regression for issue #338: the Sentry JS SDK forces a numeric DSN project id, +// which must land in the visible "default" project rather than an orphan project +// named after the id. +test.describe.configure({ mode: 'serial' }); + +async function sendSentryStore(eventId: string, value: string): Promise { + const res = await fetch(`${BUGGREGATOR}/api/1/store`, { + method: 'POST', + headers: { 'X-Sentry-Auth': 'Sentry sentry_client=sentry.javascript.browser/8.0.0, sentry_key=abc' }, + body: JSON.stringify({ + event_id: eventId, + platform: 'javascript', + level: 'error', + exception: { values: [{ type: 'TypeError', value }] }, + }), + }); + expect(res.status).toBe(200); +} + +async function sendSentryEnvelope(eventId: string, value: string): Promise { + const body = [ + JSON.stringify({ event_id: eventId }), + JSON.stringify({ type: 'event' }), + JSON.stringify({ event_id: eventId, platform: 'javascript', level: 'error', exception: { values: [{ type: 'ReferenceError', value }] } }), + ].join('\n'); + const res = await fetch(`${BUGGREGATOR}/api/2/envelope/`, { + method: 'POST', + headers: { 'X-Sentry-Auth': 'Sentry sentry_key=abc' }, + body, + }); + expect(res.status).toBe(200); +} + +test.beforeAll(async () => { + await fetch(`${BUGGREGATOR}/api/events`, { method: 'DELETE', headers: { 'Content-Type': 'application/json' }, body: '{}' }).catch(() => {}); + await sendSentryStore('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'Issue 338 numeric DSN error (store)'); + await sendSentryEnvelope('bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', 'Issue 338 numeric DSN error (envelope)'); +}); + +test('numeric-DSN Sentry events render in the default project feed', async ({ page }) => { + await page.goto(BUGGREGATOR); + await page.waitForLoadState('networkidle'); + + const card = page.locator('.preview-card').first(); + await expect(card).toBeVisible({ timeout: 15_000 }); + + const feedText = (await page.locator('body').textContent()) ?? ''; + expect(feedText.toLowerCase()).toContain('sentry'); + + await page.screenshot({ path: 'test-results/issue-338-default-feed.png', fullPage: true }); +}); + +test('no orphan numeric project is created; events are stored under "default"', async () => { + const projects = await (await fetch(`${BUGGREGATOR}/api/projects`)).json(); + const keys = (projects.data ?? []).map((p: { key: string }) => p.key); + expect(keys).toContain('default'); + expect(keys).not.toContain('1'); + expect(keys).not.toContain('2'); + + const events = await (await fetch(`${BUGGREGATOR}/api/events?project=default`)).json(); + const ids = (events.data ?? []).map((e: { uuid: string }) => e.uuid); + expect(ids).toContain('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'); + expect(ids).toContain('bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'); + for (const e of events.data ?? []) { + expect(e.project).toBe('default'); + } +}); diff --git a/internal/server/http/event_service_test.go b/internal/server/http/event_service_test.go index 20e448a..5195183 100644 --- a/internal/server/http/event_service_test.go +++ b/internal/server/http/event_service_test.go @@ -32,8 +32,8 @@ func TestEventService_HandleIncoming_AutoRegistersProject(t *testing.T) { registry := module.NewRegistry() es := serverhttp.NewEventService(store, hub, registry, nil, db) - // Simulate a Sentry event whose project key comes from the DSN path: - // /api/123/envelope/ → project "123". + // An event arriving with an explicit non-default project key (e.g. a named + // DSN path or an X-Buggregator-Project override) must auto-register that key. inc := &event.Incoming{ UUID: "evt-1", Type: "sentry", diff --git a/internal/server/http/ingestion_test.go b/internal/server/http/ingestion_test.go index f9b96de..c90a7d9 100644 --- a/internal/server/http/ingestion_test.go +++ b/internal/server/http/ingestion_test.go @@ -2,6 +2,7 @@ package http_test import ( "context" + "database/sql" "encoding/json" "net/http" "net/http/httptest" @@ -617,10 +618,10 @@ func TestPipeline_NonSentryResponse_KeepsLegacyShape(t *testing.T) { } } -// TestPipeline_AutoCreatesSentryProject ensures the project key embedded in a -// Sentry DSN path (e.g. /api/12345/envelope/ → "12345") becomes a real row in -// the projects table, otherwise the frontend filters the event out of its list. -func TestPipeline_AutoCreatesSentryProject(t *testing.T) { +// newSentryPipeline builds a full ingestion pipeline backed by an in-memory DB +// with the Sentry module registered, for end-to-end project-routing tests. +func newSentryPipeline(t *testing.T) (*serverhttp.IngestionPipeline, *sql.DB) { + t.Helper() db, err := storage.Open(":memory:") if err != nil { t.Fatal(err) @@ -643,12 +644,21 @@ func TestPipeline_AutoCreatesSentryProject(t *testing.T) { hub := ws.NewHub() es := serverhttp.NewEventService(store, hub, registry, nil, db) pipeline := serverhttp.NewIngestionPipeline(registry.Handlers(), es, fstest.MapFS{"index.html": {Data: []byte("")}}) + return pipeline, db +} + +// TestPipeline_AutoCreatesSentryProject ensures a NAMED project key embedded in a +// Sentry DSN path (e.g. /api/myteam/envelope/ → "myteam") becomes a real row in +// the projects table, otherwise the frontend filters the event out of its list. +// (Numeric ids route to the default project instead — see the test below.) +func TestPipeline_AutoCreatesSentryProject(t *testing.T) { + pipeline, db := newSentryPipeline(t) body := `{"event_id":"sx-1","sent_at":"2026-01-01T00:00:00Z"} {"type":"event"} {"event_id":"sx-1","level":"error","message":"x","exception":{"values":[{"type":"E","value":"v"}]}}` - r := httptest.NewRequest("POST", "http://localhost/api/12345/envelope/", strings.NewReader(body)) + r := httptest.NewRequest("POST", "http://localhost/api/myteam/envelope/", strings.NewReader(body)) r.Header.Set("X-Sentry-Auth", "Sentry sentry_key=abc") w := httptest.NewRecorder() pipeline.ServeHTTP(w, r) @@ -658,8 +668,136 @@ func TestPipeline_AutoCreatesSentryProject(t *testing.T) { } var name string - if err := db.QueryRow(`SELECT name FROM projects WHERE key = ?`, "12345").Scan(&name); err != nil { - t.Fatalf("project 12345 was not auto-registered: %v", err) + if err := db.QueryRow(`SELECT name FROM projects WHERE key = ?`, "myteam").Scan(&name); err != nil { + t.Fatalf("project myteam was not auto-registered: %v", err) + } +} + +// TestPipeline_SentryNumericProjectRoutesToDefault is the regression test for +// issue #338: the Sentry JS SDK forces a numeric DSN project id, which must land +// in the default project rather than creating an orphan project named after the id. +func TestPipeline_SentryNumericProjectRoutesToDefault(t *testing.T) { + pipeline, db := newSentryPipeline(t) + + payload := `{"event_id":"8f78970685be481994063baacda75197","timestamp":1774960590.323397,"level":"error","exception":{"values":[{"type":"E","value":"v"}]}}` + r := httptest.NewRequest("POST", "http://localhost/api/1/store", strings.NewReader(payload)) + r.Header.Set("X-Sentry-Auth", "Sentry sentry_key=abc") + w := httptest.NewRecorder() + pipeline.ServeHTTP(w, r) + + if w.Code != 200 { + t.Fatalf("status = %d, body = %s", w.Code, w.Body.String()) + } + + var orphan int + if err := db.QueryRow(`SELECT COUNT(*) FROM projects WHERE key = ?`, "1").Scan(&orphan); err != nil { + t.Fatal(err) + } + if orphan != 0 { + t.Errorf("orphan project %q was created, want none", "1") + } + + var project string + if err := db.QueryRow(`SELECT project FROM events WHERE uuid = ?`, "8f78970685be481994063baacda75197").Scan(&project); err != nil { + t.Fatalf("event was not stored: %v", err) + } + if project != "default" { + t.Errorf("event project = %q, want %q", project, "default") + } +} + +// TestPipeline_SentryNumericProjectEnvelopeRoutesToDefault is the envelope-path +// counterpart of the regression above — the path the Sentry JS SDK actually uses. +func TestPipeline_SentryNumericProjectEnvelopeRoutesToDefault(t *testing.T) { + pipeline, db := newSentryPipeline(t) + + body := `{"event_id":"env-err-1","sent_at":"2026-01-01T00:00:00Z"} +{"type":"event"} +{"event_id":"env-err-1","level":"error","message":"x","exception":{"values":[{"type":"E","value":"v"}]}}` + r := httptest.NewRequest("POST", "http://localhost/api/1/envelope/", strings.NewReader(body)) + r.Header.Set("X-Sentry-Auth", "Sentry sentry_key=abc") + w := httptest.NewRecorder() + pipeline.ServeHTTP(w, r) + + if w.Code != 200 { + t.Fatalf("status = %d, body = %s", w.Code, w.Body.String()) + } + + var orphan int + if err := db.QueryRow(`SELECT COUNT(*) FROM projects WHERE key = ?`, "1").Scan(&orphan); err != nil { + t.Fatal(err) + } + if orphan != 0 { + t.Errorf("orphan project %q was created, want none", "1") + } + + var project string + if err := db.QueryRow(`SELECT project FROM events WHERE uuid = ?`, "env-err-1").Scan(&project); err != nil { + t.Fatalf("event was not stored: %v", err) + } + if project != "default" { + t.Errorf("event project = %q, want %q", project, "default") + } +} + +// TestPipeline_SentryProjectHeaderOverridesNumeric confirms an explicit +// X-Buggregator-Project still wins over numeric-resolution: the escape hatch for +// directing a numeric DSN to a named project rather than to default. +func TestPipeline_SentryProjectHeaderOverridesNumeric(t *testing.T) { + pipeline, db := newSentryPipeline(t) + + payload := `{"event_id":"ovr-1","timestamp":1774960590.32,"level":"error","exception":{"values":[{"type":"E","value":"v"}]}}` + r := httptest.NewRequest("POST", "http://localhost/api/1/store", strings.NewReader(payload)) + r.Header.Set("X-Buggregator-Event", "sentry") + r.Header.Set("X-Buggregator-Project", "web") + w := httptest.NewRecorder() + pipeline.ServeHTTP(w, r) + + if w.Code != 200 { + t.Fatalf("status = %d, body = %s", w.Code, w.Body.String()) + } + + var project string + if err := db.QueryRow(`SELECT project FROM events WHERE uuid = ?`, "ovr-1").Scan(&project); err != nil { + t.Fatalf("event was not stored: %v", err) + } + if project != "web" { + t.Errorf("event project = %q, want %q", project, "web") + } + + var orphan int + if err := db.QueryRow(`SELECT COUNT(*) FROM projects WHERE key = ?`, "1").Scan(&orphan); err != nil { + t.Fatal(err) + } + if orphan != 0 { + t.Errorf("orphan project %q was created, want none", "1") + } +} + +// TestPipeline_SentryTransactionNumericProject_NoOrphanProject confirms a +// transaction-only envelope on a numeric DSN creates no orphan project (it +// produces no canonical event, so the auto-register path is never reached). +func TestPipeline_SentryTransactionNumericProject_NoOrphanProject(t *testing.T) { + pipeline, db := newSentryPipeline(t) + + body := `{"event_id":"tx-1","sent_at":"2026-01-01T00:00:00Z"} +{"type":"transaction"} +{"event_id":"tx-1","type":"transaction","transaction":"GET /","start_timestamp":1,"timestamp":2,"spans":[]}` + r := httptest.NewRequest("POST", "http://localhost/api/1/envelope/", strings.NewReader(body)) + r.Header.Set("X-Sentry-Auth", "Sentry sentry_key=abc") + w := httptest.NewRecorder() + pipeline.ServeHTTP(w, r) + + if w.Code != 200 { + t.Fatalf("status = %d, body = %s", w.Code, w.Body.String()) + } + + var orphan int + if err := db.QueryRow(`SELECT COUNT(*) FROM projects WHERE key = ?`, "1").Scan(&orphan); err != nil { + t.Fatal(err) + } + if orphan != 0 { + t.Errorf("orphan project %q was created by a transaction envelope, want none", "1") } } diff --git a/modules/sentry/api_exceptions.go b/modules/sentry/api_exceptions.go index b9ba33e..ff1d915 100644 --- a/modules/sentry/api_exceptions.go +++ b/modules/sentry/api_exceptions.go @@ -211,19 +211,21 @@ func handleExceptionDetail(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { id := r.PathValue("id") - // Load error event. + // Load error event. The URL segment may be an internal id or a Sentry + // event_id (the exceptions list and the frontend route both key on + // event_id), so resolve by either and prefer the internal-id match. var ( - eventID, fingerprint, level, receivedAt, payloadStr string - handled sql.NullBool - platform, environment, serverName, txn, release sql.NullString - traceID, spanID sql.NullString - eventTS sql.NullString + internalID, eventID, fingerprint, level, receivedAt, payloadStr string + handled sql.NullBool + platform, environment, serverName, txn, release sql.NullString + traceID, spanID sql.NullString + eventTS sql.NullString ) err := db.QueryRow( - `SELECT event_id, fingerprint, level, handled, platform, environment, server_name, + `SELECT id, event_id, fingerprint, level, handled, platform, environment, server_name, "transaction", release, trace_id, span_id, received_at, event_ts, payload - FROM sentry_error_events WHERE id = ?`, id, - ).Scan(&eventID, &fingerprint, &level, &handled, &platform, &environment, + FROM sentry_error_events WHERE id = ? OR event_id = ? ORDER BY (id = ?) DESC LIMIT 1`, id, id, id, + ).Scan(&internalID, &eventID, &fingerprint, &level, &handled, &platform, &environment, &serverName, &txn, &release, &traceID, &spanID, &receivedAt, &eventTS, &payloadStr) if err == sql.ErrNoRows { apiError(w, "not found", http.StatusNotFound) @@ -234,11 +236,9 @@ func handleExceptionDetail(db *sql.DB) http.HandlerFunc { return } - // Load exceptions. - exceptions := loadExceptions(db, id) - - // Load breadcrumbs. - breadcrumbs := loadBreadcrumbs(db, id) + // Child tables key on the internal id, not the URL segment. + exceptions := loadExceptions(db, internalID) + breadcrumbs := loadBreadcrumbs(db, internalID) // Load trace summary if trace_id exists. var traceSummary any @@ -253,7 +253,7 @@ func handleExceptionDetail(db *sql.DB) http.HandlerFunc { } result := map[string]any{ - "id": id, + "id": internalID, "event_id": eventID, "fingerprint": fingerprint, "level": level, diff --git a/modules/sentry/api_test.go b/modules/sentry/api_test.go index 7295aa9..bec6cd7 100644 --- a/modules/sentry/api_test.go +++ b/modules/sentry/api_test.go @@ -208,6 +208,43 @@ func TestAPIExceptionDetail(t *testing.T) { } } +// TestAPIExceptionDetailByEventID covers Fix B: the detail endpoint must resolve +// by event_id (the key the list and the frontend route use) and still return the +// populated exceptions/breadcrumbs keyed off the internal id. +func TestAPIExceptionDetailByEventID(t *testing.T) { + mux := http.NewServeMux() + seedTestData(t, mux) + + req := httptest.NewRequest("GET", "/api/sentry/exceptions", nil) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + var listResp map[string]any + json.Unmarshal(w.Body.Bytes(), &listResp) + first := listResp["data"].([]any)[0].(map[string]any) + eventID := first["event_id"].(string) + internalID := first["id"].(string) + + req = httptest.NewRequest("GET", "/api/sentry/exceptions/"+eventID, nil) + w = httptest.NewRecorder() + mux.ServeHTTP(w, req) + + if w.Code != 200 { + t.Fatalf("status = %d, want 200 for event_id lookup", w.Code) + } + + var detail map[string]any + json.Unmarshal(w.Body.Bytes(), &detail) + + if detail["id"] != internalID { + t.Errorf("detail id = %v, want internal id %q", detail["id"], internalID) + } + excs, ok := detail["exceptions"].([]any) + if !ok || len(excs) == 0 { + t.Errorf("exceptions empty for event_id lookup, want populated (got %v)", detail["exceptions"]) + } +} + func TestAPIExceptionDetailNotFound(t *testing.T) { mux := http.NewServeMux() seedTestData(t, mux) diff --git a/modules/sentry/handler.go b/modules/sentry/handler.go index 71dc668..4c2e636 100644 --- a/modules/sentry/handler.go +++ b/modules/sentry/handler.go @@ -46,7 +46,7 @@ func (h *handler) Handle(r *http.Request) (*event.Incoming, error) { body = decompress(body, r.Header.Get("Content-Encoding")) // Extract project from path: /api/{project}/store - project := extractProject(r.URL.Path) + project := resolveProject(extractProject(r.URL.Path)) // Try parsing as plain JSON first (legacy /store format). var parsed map[string]any @@ -56,6 +56,11 @@ func (h *handler) Handle(r *http.Request) (*event.Incoming, error) { uuid = id } + // Resolve source code for browser/JS frames that only carry file+line. + if enriched, changed := enrichSourceContext(body, nil); changed { + body = enriched + } + // Store structured data if DB is available. h.storeJSONEvent(body, project) @@ -198,6 +203,12 @@ func (h *handler) handleEventItem(item EnvelopeItem, envelopeEventID string, pro payload = injectEventID(payload, envelopeEventID) } + // Resolve source code for browser/JS frames that only carry file+line. + if enriched, changed := enrichSourceContext(payload, nil); changed { + payload = enriched + _ = json.Unmarshal(payload, &ev) // re-parse so structured storage carries the code too + } + if h.db != nil { if _, err := storeErrorEvent(h.db, &ev, payload, project); err != nil { slog.Warn("sentry: failed to store structured error event", "err", err) @@ -371,3 +382,26 @@ func extractProject(path string) string { } return "" } + +// resolveProject maps a Sentry DSN project segment to a Buggregator project key. +// The Sentry JavaScript SDK rejects non-numeric DSN project ids, so a purely +// numeric segment is the SDK-internal project id, not a Buggregator project name — +// it resolves to empty so the event lands in the default project. Issue #338. +func resolveProject(segment string) string { + if isAllDigits(segment) { + return "" + } + return segment +} + +func isAllDigits(s string) bool { + if s == "" { + return false + } + for _, r := range s { + if r < '0' || r > '9' { + return false + } + } + return true +} diff --git a/modules/sentry/handler_test.go b/modules/sentry/handler_test.go index d06cf3a..05c3fb8 100644 --- a/modules/sentry/handler_test.go +++ b/modules/sentry/handler_test.go @@ -180,6 +180,53 @@ func TestExtractProject(t *testing.T) { } } +func TestResolveProject(t *testing.T) { + tests := []struct { + segment string + want string + }{ + {"1", ""}, // numeric DSN id (Sentry SDK-forced) → default + {"123", ""}, // multi-digit numeric → default + {"my-app", "my-app"}, + {"", ""}, + {"app2", "app2"}, // trailing digit is a named project + {"2team", "2team"}, + {"١٢٣", "١٢٣"}, // Arabic-Indic digits are named, not numeric (ASCII only) + {"123", "123"}, // fullwidth digits are named, not numeric + } + + for _, tt := range tests { + t.Run(tt.segment, func(t *testing.T) { + if got := resolveProject(tt.segment); got != tt.want { + t.Errorf("resolveProject(%q) = %q, want %q", tt.segment, got, tt.want) + } + }) + } +} + +func TestIsAllDigits(t *testing.T) { + tests := []struct { + s string + want bool + }{ + {"007", true}, + {"0", true}, + {"", false}, + {"+1", false}, + {" 1", false}, + {"1.0", false}, + {"abc", false}, + } + + for _, tt := range tests { + t.Run(tt.s, func(t *testing.T) { + if got := isAllDigits(tt.s); got != tt.want { + t.Errorf("isAllDigits(%q) = %v, want %v", tt.s, got, tt.want) + } + }) + } +} + func TestHandleEnvelope(t *testing.T) { h := &handler{} // no db — structured storage silently skipped @@ -215,6 +262,29 @@ not valid json`) }) } +// TestHandler_NumericProject_Envelope routes a numeric DSN envelope to the +// default project (empty project) — the JS SDK path of issue #338. +func TestHandler_NumericProject_Envelope(t *testing.T) { + h := &handler{} + + body := `{"event_id":"env-1"} +{"type":"event"} +{"event_id":"env-1","level":"error","exception":{"values":[{"type":"E","value":"v"}]}}` + r := httptest.NewRequest("POST", "/api/1/envelope/", strings.NewReader(body)) + r.Header.Set("X-Sentry-Auth", "Sentry sentry_key=test") + + inc, err := h.Handle(r) + if err != nil { + t.Fatalf("Handle() error: %v", err) + } + if inc == nil { + t.Fatal("Handle() returned nil, want non-nil incoming event") + } + if inc.Project != "" { + t.Errorf("Project = %q, want %q (routes to default)", inc.Project, "") + } +} + // TestHandler_SDKv3_PlainJSON tests Sentry PHP SDK v3 without tracing. // v3 sends plain JSON to /api/{project}/store with event_id in payload. func TestHandler_SDKv3_PlainJSON(t *testing.T) { @@ -239,8 +309,10 @@ func TestHandler_SDKv3_PlainJSON(t *testing.T) { if inc.UUID != "8f78970685be481994063baacda75197" { t.Errorf("UUID = %q, want %q", inc.UUID, "8f78970685be481994063baacda75197") } - if inc.Project != "1" { - t.Errorf("Project = %q, want %q", inc.Project, "1") + // A numeric DSN project id (forced by the Sentry SDKs) resolves to empty so + // the event lands in the default project downstream — see issue #338. + if inc.Project != "" { + t.Errorf("Project = %q, want %q", inc.Project, "") } // Payload must contain event_id so the frontend can identify the event. diff --git a/modules/sentry/source_context.go b/modules/sentry/source_context.go new file mode 100644 index 0000000..032c10d --- /dev/null +++ b/modules/sentry/source_context.go @@ -0,0 +1,240 @@ +package sentry + +import ( + "crypto/tls" + "encoding/json" + "io" + "net/http" + "strings" + "time" +) + +// Source context resolution for browser/JS stack frames. +// +// JavaScript SDKs (e.g. sentry.javascript.sveltekit) send error frames that +// carry only `filename` (a URL), `lineno` and `colno` — no source code. The +// frontend cannot fetch that source itself because the dev server is a +// different origin and CORS blocks it. So we resolve the code here, server-side, +// where there is no CORS restriction: fetch the file the frame points at and +// attach pre_context / context_line / post_context. This is what makes +// SvelteKit (and any Vite/webpack dev-server) JS stack traces actually show +// code in Buggregator. +// +// It is strictly best-effort and self-limiting: +// - only http(s) `filename`s are touched, which naturally scopes this to +// browser/JS frames (PHP/Python frames carry filesystem paths, not URLs); +// - frames that already include source are left alone; +// - any fetch error leaves the frame untouched. + +const ( + sourceContextLines = 5 // lines of context kept before and after + sourceFetchTimeout = 3 * time.Second + sourceMaxBytes = 2 << 20 // 2 MiB cap per fetched file +) + +// sourceFetcher returns the text of a remote source file and whether it was +// retrieved. It is an injection point for tests. +type sourceFetcher func(url string) (string, bool) + +var defaultSourceClient = &http.Client{ + Timeout: sourceFetchTimeout, + Transport: &http.Transport{ + // Local dev servers (Vite behind ddev/https) use self-signed certs. + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + }, +} + +func httpSourceFetcher(url string) (string, bool) { + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return "", false + } + resp, err := defaultSourceClient.Do(req) + if err != nil { + return "", false + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return "", false + } + data, err := io.ReadAll(io.LimitReader(resp.Body, sourceMaxBytes)) + if err != nil { + return "", false + } + return string(data), true +} + +// enrichSourceContext walks every stacktrace in a Sentry error payload and +// attaches source code to frames that reference a fetchable URL but carry none. +// It returns the (possibly rewritten) payload and whether anything changed. +func enrichSourceContext(payload json.RawMessage, fetch sourceFetcher) (json.RawMessage, bool) { + if fetch == nil { + fetch = httpSourceFetcher + } + + var root map[string]any + if err := json.Unmarshal(payload, &root); err != nil { + return payload, false + } + + cache := map[string]*fetchedSource{} // url -> parsed file (per-event, dedupes refetches) + missing := map[string]bool{} // url -> fetch already failed + changed := false + + visit := func(st any) { + if enrichStacktrace(st, fetch, cache, missing) { + changed = true + } + } + + if exc, ok := root["exception"].(map[string]any); ok { + for _, v := range asAnySlice(exc["values"]) { + if vm, ok := v.(map[string]any); ok { + visit(vm["stacktrace"]) + } + } + } + if th, ok := root["threads"].(map[string]any); ok { + for _, v := range asAnySlice(th["values"]) { + if vm, ok := v.(map[string]any); ok { + visit(vm["stacktrace"]) + } + } + } + visit(root["stacktrace"]) + + if !changed { + return payload, false + } + + out, err := json.Marshal(root) + if err != nil { + return payload, false + } + return out, true +} + +func asAnySlice(v any) []any { + s, _ := v.([]any) + return s +} + +// fetchedSource is a transformed file plus its parsed source map (if any), +// cached per event so multiple frames from the same file fetch once. +type fetchedSource struct { + transformed []string + sm *sourceMap +} + +func enrichStacktrace(st any, fetch sourceFetcher, cache map[string]*fetchedSource, missing map[string]bool) bool { + stm, ok := st.(map[string]any) + if !ok { + return false + } + changed := false + for _, f := range asAnySlice(stm["frames"]) { + if fm, ok := f.(map[string]any); ok { + if enrichFrame(fm, fetch, cache, missing) { + changed = true + } + } + } + return changed +} + +func enrichFrame(fm map[string]any, fetch sourceFetcher, cache map[string]*fetchedSource, missing map[string]bool) bool { + // Skip frames that already carry source. + if s, ok := fm["context_line"].(string); ok && s != "" { + return false + } + + filename, _ := fm["filename"].(string) + if !strings.HasPrefix(filename, "http://") && !strings.HasPrefix(filename, "https://") { + return false + } + + lineno, ok := toLineNumber(fm["lineno"]) + if !ok || lineno < 1 { + return false + } + colno, _ := toLineNumber(fm["colno"]) // 1-based, 0 if absent + + src, ok := cache[filename] + if !ok { + if missing[filename] { + return false + } + text, fetched := fetch(filename) + if !fetched { + missing[filename] = true + return false + } + src = &fetchedSource{transformed: strings.Split(text, "\n")} + if sm, ok := extractSourceMap(text, filename, fetch); ok { + src.sm = sm + } + cache[filename] = src + } + + // Prefer the original source via the source map; fall back to the + // transformed lines the dev server served. + if src.sm != nil { + genCol := colno - 1 + if genCol < 0 { + genCol = 0 + } + if srcIdx, origLine, origCol, ok := src.sm.originalPosition(lineno, genCol); ok { + if lines, ok := src.sm.originalSourceLines(srcIdx); ok && origLine < len(lines) { + applyContext(fm, lines, origLine) + // Re-point the frame at the original location. Keep `filename` + // (the dev-server URL is the precise path, e.g. .../+page.svelte) + // but expose the original name as `abs_path` for reference. + fm["lineno"] = origLine + 1 + fm["colno"] = origCol + 1 + if name := src.sm.sourceName(srcIdx); name != "" { + fm["abs_path"] = name + } + return true + } + } + } + + if lineno > len(src.transformed) { + return false + } + applyContext(fm, src.transformed, lineno-1) + return true +} + +// applyContext sets context_line / pre_context / post_context around a 0-based +// line index, using sourceContextLines of surrounding context. +func applyContext(fm map[string]any, lines []string, idx int) { + fm["context_line"] = lines[idx] + + pre := make([]any, 0, sourceContextLines) + for i := max(0, idx-sourceContextLines); i < idx; i++ { + pre = append(pre, lines[i]) + } + post := make([]any, 0, sourceContextLines) + for i := idx + 1; i < min(len(lines), idx+1+sourceContextLines); i++ { + post = append(post, lines[i]) + } + fm["pre_context"] = pre + fm["post_context"] = post +} + +func toLineNumber(v any) (int, bool) { + switch n := v.(type) { + case float64: + return int(n), true + case json.Number: + i, err := n.Int64() + if err != nil { + return 0, false + } + return int(i), true + case int: + return n, true + } + return 0, false +} diff --git a/modules/sentry/source_context_test.go b/modules/sentry/source_context_test.go new file mode 100644 index 0000000..432a142 --- /dev/null +++ b/modules/sentry/source_context_test.go @@ -0,0 +1,177 @@ +package sentry + +import ( + "encoding/base64" + "encoding/json" + "testing" +) + +func TestEnrichSourceContext_BrowserFrame(t *testing.T) { + // A SvelteKit/browser frame: only filename(URL) + lineno, no source. + payload := []byte(`{ + "event_id":"abc", + "platform":"javascript", + "exception":{"values":[{ + "type":"Error", + "value":"boom", + "stacktrace":{"frames":[ + {"filename":"https://app.test/src/routes/+page.svelte","function":"?","in_app":true,"lineno":3,"colno":10} + ]} + }]} + }`) + + src := "line1\nline2\nline3-THROW\nline4\nline5" + fetch := func(url string) (string, bool) { + if url == "https://app.test/src/routes/+page.svelte" { + return src, true + } + return "", false + } + + out, changed := enrichSourceContext(payload, fetch) + if !changed { + t.Fatal("expected payload to change") + } + + frame := firstFrame(t, out) + if got := frame["context_line"]; got != "line3-THROW" { + t.Fatalf("context_line = %v, want line3-THROW", got) + } + pre := toStrings(frame["pre_context"]) + if len(pre) != 2 || pre[0] != "line1" || pre[1] != "line2" { + t.Fatalf("pre_context = %v, want [line1 line2]", pre) + } + post := toStrings(frame["post_context"]) + if len(post) != 2 || post[0] != "line4" || post[1] != "line5" { + t.Fatalf("post_context = %v, want [line4 line5]", post) + } +} + +func TestEnrichSourceContext_SkipsNonURLAndExistingSource(t *testing.T) { + // PHP-style frame (filesystem path) and a frame that already has source. + payload := []byte(`{ + "exception":{"values":[{"stacktrace":{"frames":[ + {"filename":"/var/www/app/Foo.php","lineno":7}, + {"filename":"https://app.test/a.js","lineno":1,"context_line":"already here"} + ]}}]} + }`) + + called := false + fetch := func(string) (string, bool) { called = true; return "x", true } + + _, changed := enrichSourceContext(payload, fetch) + if changed { + t.Fatal("should not change: no eligible frames") + } + if called { + t.Fatal("fetch must not be called for filesystem paths or framed-with-source") + } +} + +func TestEnrichSourceContext_FetchFailureIsBestEffort(t *testing.T) { + payload := []byte(`{"exception":{"values":[{"stacktrace":{"frames":[ + {"filename":"https://app.test/a.js","lineno":1} + ]}}]}}`) + + fetch := func(string) (string, bool) { return "", false } + + out, changed := enrichSourceContext(payload, fetch) + if changed { + t.Fatal("fetch failed, payload must be unchanged") + } + frame := firstFrame(t, out) + if _, ok := frame["context_line"]; ok { + t.Fatal("no context_line should be added on fetch failure") + } +} + +func TestEnrichSourceContext_TopLevelAndThreads(t *testing.T) { + src := "a\nb\nc" + fetch := func(string) (string, bool) { return src, true } + + for name, payload := range map[string]string{ + "top-level": `{"stacktrace":{"frames":[{"filename":"https://x/y.js","lineno":2}]}}`, + "threads": `{"threads":{"values":[{"stacktrace":{"frames":[{"filename":"https://x/y.js","lineno":2}]}}]}}`, + } { + t.Run(name, func(t *testing.T) { + _, changed := enrichSourceContext([]byte(payload), fetch) + if !changed { + t.Fatalf("%s: expected enrichment", name) + } + }) + } +} + +func TestSourceMap_OriginalPosition(t *testing.T) { + // generated line 1 -> original line 1; generated line 2 -> original line 3. + sm := &sourceMap{ + Mappings: "AAAA;AAEA", + Sources: []string{"original.js"}, + SourcesContent: []string{"// header\n// header2\nthrow new Error(\"boom\");\n"}, + } + srcIdx, origLine, _, ok := sm.originalPosition(2, 0) + if !ok { + t.Fatal("expected a mapping") + } + if srcIdx != 0 || origLine != 2 { + t.Fatalf("got srcIdx=%d origLine=%d, want 0 and 2", srcIdx, origLine) + } + lines, _ := sm.originalSourceLines(srcIdx) + if lines[origLine] != `throw new Error("boom");` { + t.Fatalf("original line = %q", lines[origLine]) + } +} + +func TestEnrichFrame_UsesOriginalSourceViaSourceMap(t *testing.T) { + smJSON := `{"version":3,"sources":["original.js"],` + + `"sourcesContent":["// header\n// header2\nthrow new Error(\"boom\");\n"],` + + `"mappings":"AAAA;AAEA","names":[]}` + b64 := base64.StdEncoding.EncodeToString([]byte(smJSON)) + transformed := "var x = 1;\nthrow new Error(\"boom\");\n" + + "//# sourceMappingURL=data:application/json;base64," + b64 + + fetch := func(string) (string, bool) { return transformed, true } + + // frame points at the GENERATED line 2; we expect the ORIGINAL (line 3) back. + payload := []byte(`{"exception":{"values":[{"stacktrace":{"frames":[ + {"filename":"https://app/x.js","lineno":2,"colno":1} + ]}}]}}`) + + out, changed := enrichSourceContext(payload, fetch) + if !changed { + t.Fatal("expected enrichment") + } + frame := firstFrame(t, out) + if got := frame["context_line"]; got != `throw new Error("boom");` { + t.Fatalf("context_line = %v, want original throw line", got) + } + if got, _ := frame["lineno"].(float64); got != 3 { + t.Fatalf("lineno = %v, want 3 (original)", frame["lineno"]) + } + pre := toStrings(frame["pre_context"]) + if len(pre) != 2 || pre[0] != "// header" || pre[1] != "// header2" { + t.Fatalf("pre_context = %v, want original header lines", pre) + } +} + +func firstFrame(t *testing.T, payload json.RawMessage) map[string]any { + t.Helper() + var root map[string]any + if err := json.Unmarshal(payload, &root); err != nil { + t.Fatalf("unmarshal: %v", err) + } + exc := root["exception"].(map[string]any) + vals := exc["values"].([]any) + st := vals[0].(map[string]any)["stacktrace"].(map[string]any) + frames := st["frames"].([]any) + return frames[0].(map[string]any) +} + +func toStrings(v any) []string { + s, _ := v.([]any) + out := make([]string, len(s)) + for i, e := range s { + out[i], _ = e.(string) + } + return out +} diff --git a/modules/sentry/sourcemap.go b/modules/sentry/sourcemap.go new file mode 100644 index 0000000..cd5e96a --- /dev/null +++ b/modules/sentry/sourcemap.go @@ -0,0 +1,215 @@ +package sentry + +import ( + "encoding/base64" + "encoding/json" + "net/url" + "strings" +) + +// Minimal source-map (v3) support, enough to turn a transformed/bundled dev +// frame back into the developer's original source. Dev servers (Vite, webpack) +// serve compiled modules with an inline `//# sourceMappingURL=data:...` map whose +// `sourcesContent` carries the original file; we decode the VLQ `mappings` to +// translate the frame's generated (line, col) into the original position. + +type sourceMap struct { + Version int `json:"version"` + Sources []string `json:"sources"` + SourcesContent []string `json:"sourcesContent"` + Mappings string `json:"mappings"` + Names []string `json:"names"` + + origLineCache map[int][]string // sourceIndex -> split sourcesContent +} + +const b64alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/" + +var b64lookup = func() [256]int8 { + var t [256]int8 + for i := range t { + t[i] = -1 + } + for i := 0; i < len(b64alphabet); i++ { + t[b64alphabet[i]] = int8(i) + } + return t +}() + +// decodeVLQ decodes a base64 VLQ-encoded segment into its integer fields. +func decodeVLQ(seg string) []int { + var out []int + shift, value := 0, 0 + for i := 0; i < len(seg); i++ { + digit := b64lookup[seg[i]] + if digit < 0 { + return out + } + d := int(digit) + cont := d&32 != 0 + d &= 31 + value += d << shift + shift += 5 + if !cont { + neg := value&1 != 0 + value >>= 1 + if neg { + value = -value + } + out = append(out, value) + shift, value = 0, 0 + } + } + return out +} + +// extractSourceMap finds and parses the source map referenced by a transformed +// source file. fileURL is the URL the text was fetched from (used to resolve a +// relative external map). fetch retrieves an external .map when needed. +func extractSourceMap(text, fileURL string, fetch sourceFetcher) (*sourceMap, bool) { + marker := "sourceMappingURL=" + idx := strings.LastIndex(text, marker) + if idx < 0 { + return nil, false + } + ref := text[idx+len(marker):] + if nl := strings.IndexAny(ref, "\r\n"); nl >= 0 { + ref = ref[:nl] + } + ref = strings.TrimSpace(ref) + if ref == "" { + return nil, false + } + + var raw []byte + if strings.HasPrefix(ref, "data:") { + comma := strings.IndexByte(ref, ',') + if comma < 0 { + return nil, false + } + meta, payload := ref[len("data:"):comma], ref[comma+1:] + if strings.Contains(meta, "base64") { + dec, err := base64.StdEncoding.DecodeString(payload) + if err != nil { + return nil, false + } + raw = dec + } else { + dec, err := url.QueryUnescape(payload) + if err != nil { + return nil, false + } + raw = []byte(dec) + } + } else { + mapURL := resolveURL(fileURL, ref) + body, ok := fetch(mapURL) + if !ok { + return nil, false + } + raw = []byte(body) + } + + var sm sourceMap + if err := json.Unmarshal(raw, &sm); err != nil { + return nil, false + } + if sm.Mappings == "" || len(sm.SourcesContent) == 0 { + return nil, false + } + return &sm, true +} + +func resolveURL(base, ref string) string { + b, err := url.Parse(base) + if err != nil { + return ref + } + r, err := url.Parse(ref) + if err != nil { + return ref + } + return b.ResolveReference(r).String() +} + +// originalPosition translates a generated 1-based line and 0-based column into +// the original source: index into Sources/SourcesContent and 0-based line. +func (sm *sourceMap) originalPosition(genLine, genCol int) (srcIndex, origLine, origCol int, ok bool) { + lines := strings.Split(sm.Mappings, ";") + if genLine < 1 || genLine > len(lines) { + return 0, 0, 0, false + } + + // Source index / original line / original column are delta-encoded across the + // whole mappings string; the generated column resets at each generated line. + curSrc, curLine, curCol := 0, 0, 0 + found := false + var fSrc, fLine, fCol int + var firstSrc, firstLine, firstCol int + haveFirst := false + + for li := 0; li < genLine; li++ { + genColAbs := 0 + for _, seg := range strings.Split(lines[li], ",") { + if seg == "" { + continue + } + f := decodeVLQ(seg) + if len(f) == 0 { + continue + } + genColAbs += f[0] + if len(f) >= 4 { + curSrc += f[1] + curLine += f[2] + curCol += f[3] + } + if li == genLine-1 && len(f) >= 4 { + if !haveFirst { + firstSrc, firstLine, firstCol = curSrc, curLine, curCol + haveFirst = true + } + if genColAbs <= genCol { + fSrc, fLine, fCol = curSrc, curLine, curCol + found = true + } + } + } + } + + if found { + return fSrc, fLine, fCol, true + } + if haveFirst { + return firstSrc, firstLine, firstCol, true + } + return 0, 0, 0, false +} + +// originalSourceLines returns the split content of the given source index. +func (sm *sourceMap) originalSourceLines(srcIndex int) ([]string, bool) { + if srcIndex < 0 || srcIndex >= len(sm.SourcesContent) { + return nil, false + } + content := sm.SourcesContent[srcIndex] + if content == "" { + return nil, false + } + if sm.origLineCache == nil { + sm.origLineCache = map[int][]string{} + } + if cached, ok := sm.origLineCache[srcIndex]; ok { + return cached, true + } + lines := strings.Split(content, "\n") + sm.origLineCache[srcIndex] = lines + return lines, true +} + +// sourceName returns a display name for a resolved source index. +func (sm *sourceMap) sourceName(srcIndex int) string { + if srcIndex < 0 || srcIndex >= len(sm.Sources) { + return "" + } + return sm.Sources[srcIndex] +} diff --git a/modules/sentry/types.go b/modules/sentry/types.go index b0a28da..309a93a 100644 --- a/modules/sentry/types.go +++ b/modules/sentry/types.go @@ -1,6 +1,7 @@ package sentry import ( + "bytes" "encoding/json" "fmt" "strconv" @@ -147,6 +148,29 @@ type BreadcrumbList struct { Values []Breadcrumb `json:"values"` } +// UnmarshalJSON accepts both shapes Sentry SDKs emit for breadcrumbs: the +// "interface" object form {"values": [...]} (Python/PHP) and the bare-array +// form [...] (sentry.javascript / SvelteKit). Without this, a JS error event +// fails to parse and never lands in the sentry tables. +func (b *BreadcrumbList) UnmarshalJSON(data []byte) error { + trimmed := bytes.TrimSpace(data) + if len(trimmed) == 0 || bytes.Equal(trimmed, []byte("null")) { + return nil + } + + if trimmed[0] == '[' { + return json.Unmarshal(trimmed, &b.Values) + } + + type alias BreadcrumbList + var a alias + if err := json.Unmarshal(trimmed, &a); err != nil { + return err + } + b.Values = a.Values + return nil +} + type Breadcrumb struct { Type string `json:"type"` Category string `json:"category"` diff --git a/modules/sentry/types_test.go b/modules/sentry/types_test.go new file mode 100644 index 0000000..6be2066 --- /dev/null +++ b/modules/sentry/types_test.go @@ -0,0 +1,61 @@ +package sentry + +import ( + "encoding/json" + "testing" +) + +// JS SDKs (sentry.javascript.*) send breadcrumbs as a bare array, while +// Python/PHP send the interface object {"values": [...]}. Both must parse, or +// the whole error event fails to unmarshal and never reaches the sentry tables. +func TestBreadcrumbList_AcceptsBothShapes(t *testing.T) { + cases := map[string]struct { + json string + want int + }{ + "object form": {`{"values":[{"category":"console"},{"category":"ui.click"}]}`, 2}, + "array form": {`[{"category":"console"},{"category":"ui.click"},{"category":"navigation"}]`, 3}, + "null": {`null`, 0}, + "empty array": {`[]`, 0}, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + var bl BreadcrumbList + if err := json.Unmarshal([]byte(tc.json), &bl); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(bl.Values) != tc.want { + t.Fatalf("got %d breadcrumbs, want %d", len(bl.Values), tc.want) + } + }) + } +} + +// Regression: a full SvelteKit browser error event (breadcrumbs as array) must +// unmarshal into ErrorEvent without error. +func TestErrorEvent_SvelteKitBrowserPayload(t *testing.T) { + payload := []byte(`{ + "event_id":"a8d5cb1ab96b4736918953b6468a0ee9", + "platform":"javascript", + "level":"error", + "exception":{"values":[{"type":"Error","value":"sentry-test client error (browser)", + "stacktrace":{"frames":[{"filename":"https://app/src/+page.svelte","lineno":16,"colno":10,"in_app":true}]}, + "mechanism":{"type":"auto.browser.browserapierrors.setTimeout","handled":false}}]}, + "breadcrumbs":[ + {"category":"console","level":"warning","message":"hi"}, + {"category":"ui.click","message":"button"} + ] + }`) + + var ev ErrorEvent + if err := json.Unmarshal(payload, &ev); err != nil { + t.Fatalf("SvelteKit payload failed to parse: %v", err) + } + if ev.Breadcrumbs == nil || len(ev.Breadcrumbs.Values) != 2 { + t.Fatalf("breadcrumbs not parsed: %+v", ev.Breadcrumbs) + } + if ev.Exception == nil || len(ev.Exception.Values) != 1 { + t.Fatalf("exception not parsed: %+v", ev.Exception) + } +}