diff --git a/apps/web/src/components/StackTrace.tsx b/apps/web/src/components/StackTrace.tsx index 0a72c6e..cab7ed0 100644 --- a/apps/web/src/components/StackTrace.tsx +++ b/apps/web/src/components/StackTrace.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { useQuery } from '@tanstack/react-query'; import type { NormalizedFrame } from '@geniusdebug/shared'; -import { hasUsableFramePath } from '@geniusdebug/shared'; +import { hasUsableFramePath, normalizeFramePath, pickSuspectFrame } from '@geniusdebug/shared'; import { api } from '../lib/api'; import { ChevronDownIcon } from './icons'; @@ -26,11 +26,12 @@ export function StackTrace({ frames, shortId }: { frames: NormalizedFrame[]; sho return
No stack trace on this event.
; } const ordered = [...frames].reverse(); // crashing frame first - // The innermost frame is sometimes one the SDK couldn't resolve a real file - // for (e.g. sentry-php's "Unknown"/line-0 placeholder on a shutdown-captured - // fatal with no backtrace to the actual trigger) — featuring that as "Crashed - // in" is actively misleading. Prefer the nearest frame with a real path. - const crash = ordered.find(hasUsableFramePath) ?? ordered[0]; + // Prefer the nearest IN-APP frame with a real path over the literal + // innermost frame — the innermost frame is often framework/dispatch code + // (e.g. React/Next internals), or an SDK placeholder like sentry-php's + // "Unknown" on a shutdown-captured fatal — either is misleading as + // "Crashed in". Same selection used for the Suspect-frame card. + const crash = pickSuspectFrame(frames)!; // Group consecutive system frames so they collapse together (Sentry behavior). const groups: { system: boolean; frames: { f: NormalizedFrame; idx: number }[] }[] = []; @@ -231,8 +232,7 @@ function CodeLine({ n, text, crash }: { n: number | null; text: string; crash?: // ------------------------- helpers ------------------------- function shortFile(p?: string | null): string { - if (!p) return ''; - return p.replace(/^webpack-internal:\/\/\/(\(.*?\)\/)?/, '').replace(/^\.\//, ''); + return normalizeFramePath(p) ?? ''; } // Lightweight JS/TS syntax highlighter for source-context lines. diff --git a/apps/web/src/lib/agentMarkdown.ts b/apps/web/src/lib/agentMarkdown.ts index 228e813..baa647f 100644 --- a/apps/web/src/lib/agentMarkdown.ts +++ b/apps/web/src/lib/agentMarkdown.ts @@ -1,4 +1,5 @@ import type { IssueDto, EventDto, NormalizedFrame } from '@geniusdebug/shared'; +import { normalizeFramePath, pickSuspectFrame } from '@geniusdebug/shared'; /** * Serialize an issue + its latest event into a structured Markdown document @@ -35,7 +36,7 @@ export function buildAgentMarkdown(issue: IssueDto, event: EventDto | null): str // In-app frames first, most-relevant last-to-first (crash frame last in array → show innermost first). const ordered = [...frames].reverse(); for (const f of ordered) { - const loc = `${f.filename ?? f.module ?? ''}${f.lineno ? `:${f.lineno}` : ''}`; + const loc = `${normalizeFramePath(f.filename ?? f.module) ?? ''}${f.lineno ? `:${f.lineno}` : ''}`; L.push(`### ${loc} — \`${f.function ?? ''}\`${f.inApp ? ' _(in-app)_' : ''}`); if (f.githubUrl) L.push(`GitHub: ${f.githubUrl}`); if (f.contextLine || f.preContext?.length || f.postContext?.length) { @@ -75,8 +76,8 @@ export function buildAgentMarkdown(issue: IssueDto, event: EventDto | null): str L.push('## Tags', ...tagKeys.map((k) => `- ${k}: ${tags[k]}`), ''); } - const crashFrame = frames.find((f) => f.inApp) ?? frames[frames.length - 1]; - const crashLoc = crashFrame ? `${crashFrame.filename ?? ''}${crashFrame.lineno ? `:${crashFrame.lineno}` : ''}` : 'the top in-app frame'; + const crashFrame = pickSuspectFrame(frames); + const crashLoc = crashFrame ? `${normalizeFramePath(crashFrame.filename) ?? ''}${crashFrame.lineno ? `:${crashFrame.lineno}` : ''}` : 'the top in-app frame'; L.push('## Task for the AI agent'); L.push( `You are an expert debugger. Identify the **root cause** of the error above and propose a **minimal fix** as a unified diff.`, diff --git a/apps/web/src/pages/IssueDetail.tsx b/apps/web/src/pages/IssueDetail.tsx index 4250c48..79f6061 100644 --- a/apps/web/src/pages/IssueDetail.tsx +++ b/apps/web/src/pages/IssueDetail.tsx @@ -2,7 +2,7 @@ import * as React from "react"; import { Link, useParams } from "react-router-dom"; import { useQuery, useQueryClient, useMutation } from "@tanstack/react-query"; import type { IssueDto, EventDto, NormalizedFrame } from "@geniusdebug/shared"; -import { hasUsableFramePath } from "@geniusdebug/shared"; +import { normalizeFramePath, pickSuspectFrame } from "@geniusdebug/shared"; import { api, errMsg } from "../lib/api"; import { useUi } from "../store/ui"; import { toast, ACTION_PAST } from "../store/toast"; @@ -1528,20 +1528,13 @@ function SuspectFrame({ }) { void githubDefault; if (!frames || frames.length === 0) return null; - const ordered = [...frames].reverse(); // crashing frame first - // Fallback chain: in-app + has source context → any frame with context → - // in-app with at least a real (non-SDK-placeholder) path → any real path → - // give up and show whatever the innermost frame is. - const suspect = - ordered.find((f) => f.inApp && f.contextLine != null) ?? - ordered.find((f) => f.contextLine != null) ?? - ordered.find((f) => f.inApp && hasUsableFramePath(f)) ?? - ordered.find(hasUsableFramePath) ?? - ordered[0]; + // In-app always wins over framework/system, even when the in-app frame + // lacks resolved source context (a different chunk's map may have resolved + // a framework frame WITH context — showing that instead would be wrong). + // Same selection used by the "Crashed in" summary (StackTrace.tsx). + const suspect = pickSuspectFrame(frames)!; const path = suspect.absPath ?? suspect.filename ?? ""; - const base = path - .replace(/^webpack-internal:\/\/\/(\(.*?\)\/)?/, "") - .replace(/^\.\//, ""); + const base = normalizeFramePath(path) ?? path; const hasCode = suspect.contextLine != null || (suspect.preContext?.length ?? 0) > 0; const mappable = /\.(mjs|cjs|jsx?|tsx?|vue|svelte)$/.test(base); diff --git a/apps/workers/src/apply-map.test.ts b/apps/workers/src/apply-map.test.ts index 2d9c996..7ed0b1d 100644 --- a/apps/workers/src/apply-map.test.ts +++ b/apps/workers/src/apply-map.test.ts @@ -94,3 +94,24 @@ test('Next.js internal source (webpack://_N_E/src/client/...) is not flagged in- assert.equal(f.filename, 'src/client/app-next.ts'); assert.equal(f.inApp, false, 'Next.js framework internals must not read as the app\'s own code'); }); + +test('Turbopack-resolved Next.js internal source (turbopack:///[project]/src/client/...) is not flagged in-app', async () => { + // Regression: FRAMEWORK_INTERNAL_RE's `^src/...` alternative is anchored to + // string-start; the unstripped turbopack:///[project]/ prefix broke that + // anchor and left framework-internal frames misclassified as in-app. + const g = new SourceMapGenerator({ file: 'bundle.js' }); + g.addMapping({ generated: { line: 1, column: 0 }, original: { line: 7, column: 10 }, source: 'turbopack:///[project]/src/client/app-next.ts' }); + const minified: NormalizedFrame = { filename: 'bundle.js', lineno: 1, colno: 0, inApp: true }; + const [f] = await symbolicateWithMap([minified], g.toString()); + assert.equal(f.filename, 'src/client/app-next.ts'); + assert.equal(f.inApp, false, 'Next.js framework internals resolved via Turbopack must not read as the app\'s own code'); +}); + +test('Turbopack-resolved app source (turbopack:///[project]/app/...) IS flagged in-app', async () => { + const g = new SourceMapGenerator({ file: 'bundle.js' }); + g.addMapping({ generated: { line: 1, column: 0 }, original: { line: 71, column: 21 }, source: 'turbopack:///[project]/app/sentry-replay-test/page.tsx' }); + const minified: NormalizedFrame = { filename: 'bundle.js', lineno: 1, colno: 0, inApp: false }; + const [f] = await symbolicateWithMap([minified], g.toString()); + assert.equal(f.filename, 'app/sentry-replay-test/page.tsx'); + assert.equal(f.inApp, true); +}); diff --git a/apps/workers/src/apply-map.ts b/apps/workers/src/apply-map.ts index 4e77870..951882d 100644 --- a/apps/workers/src/apply-map.ts +++ b/apps/workers/src/apply-map.ts @@ -1,5 +1,6 @@ import { SourceMapConsumer } from 'source-map'; import type { NormalizedFrame } from '@geniusdebug/shared'; +import { normalizeFramePath } from '@geniusdebug/shared'; /** * Paths that are framework internals, not the app's own code, even though they @@ -57,22 +58,19 @@ export async function symbolicateWithMaps( } } -/** - * Our own uploader reads .map files straight off disk — Sentry's own - * `rewriteSources` normalization (which strips this) never runs, since that's - * part of the SaaS-upload pipeline we don't use (no auth token). So every - * resolved `sources` entry still carries webpack's raw `webpack://_N_E/...` - * (or bare `webpack://...`) scheme prefix unless we strip it ourselves. - */ -function cleanSourcePath(source: string): string { - return source.replace(/^webpack:\/\/(?:_N_E\/)?/, ''); -} - export function resolveFrame(f: NormalizedFrame, consumer: SourceMapConsumer): NormalizedFrame { if (f.lineno == null) return f; const pos = consumer.originalPositionFor({ line: f.lineno, column: f.colno ?? 0 }); if (!pos.source || pos.line == null) return f; // no mapping → keep raw frame (FR-MAP-8) - const source = cleanSourcePath(pos.source); + // Our own uploader reads .map files straight off disk — Sentry's own + // `rewriteSources` normalization (which strips this) never runs, since + // that's part of the SaaS-upload pipeline we don't use (no auth token). So + // every resolved `sources` entry still carries the bundler's raw scheme + // prefix (webpack://_N_E/..., turbopack:///[project]/...) unless we strip + // it ourselves — and FRAMEWORK_INTERNAL_RE's anchored `^src/...` + // alternative needs that stripped, normalized path to match consistently + // across bundlers. + const source = normalizeFramePath(pos.source) ?? pos.source; const resolved: NormalizedFrame = { ...f, diff --git a/apps/workers/src/frames.test.ts b/apps/workers/src/frames.test.ts new file mode 100644 index 0000000..a1fda99 --- /dev/null +++ b/apps/workers/src/frames.test.ts @@ -0,0 +1,83 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { normalizeFramePath, pickSuspectFrame } from '@geniusdebug/shared'; +import type { NormalizedFrame } from '@geniusdebug/shared'; + +const f = (over: Partial): NormalizedFrame => ({ inApp: false, ...over }); + +test('normalizeFramePath strips webpack-internal (Next.js dev) prefix', () => { + assert.equal(normalizeFramePath('webpack-internal:///(app-pages-browser)/./app/page.tsx'), 'app/page.tsx'); +}); + +test('normalizeFramePath strips webpack://_N_E/ prefix', () => { + assert.equal(normalizeFramePath('webpack://_N_E/app/page.tsx'), 'app/page.tsx'); +}); + +test('normalizeFramePath strips bare webpack:// prefix', () => { + assert.equal(normalizeFramePath('webpack://app/page.tsx'), 'app/page.tsx'); +}); + +test('normalizeFramePath strips turbopack:///[project]/ prefix', () => { + assert.equal(normalizeFramePath('turbopack:///[project]/app/sentry-replay-test/page.tsx'), 'app/sentry-replay-test/page.tsx'); +}); + +test('normalizeFramePath strips a built _next/(app|src)/ asset prefix, with or without an origin', () => { + assert.equal(normalizeFramePath('_next/app/page.js'), 'app/page.js'); + assert.equal(normalizeFramePath('https://example.com/_next/src/client.js'), 'src/client.js'); +}); + +test('normalizeFramePath strips a leading ./', () => { + assert.equal(normalizeFramePath('./app/page.tsx'), 'app/page.tsx'); +}); + +test('normalizeFramePath returns undefined for empty/undefined/null input', () => { + assert.equal(normalizeFramePath(undefined), undefined); + assert.equal(normalizeFramePath(null), undefined); + assert.equal(normalizeFramePath(''), undefined); +}); + +test('pickSuspectFrame prefers an in-app frame with source context over anything else', () => { + const frames = [ + f({ filename: 'app/page.tsx', inApp: true, contextLine: 'throw new Error()' }), + f({ filename: 'node_modules/next/dist/client/app-bootstrap.js', inApp: false, contextLine: 'dispatch()' }), + ]; + assert.equal(pickSuspectFrame(frames)?.filename, 'app/page.tsx'); +}); + +test('regression: in-app frame without context must NOT lose to a framework frame that resolved WITH context', () => { + // A frame can be inApp:true with contextLine:undefined (its own chunk's map + // lacked sourcesContent) while an unrelated framework frame from a + // different chunk resolved with full context — the in-app frame must still + // win, since showing the wrong (framework) frame is worse than showing the + // right frame with no code snippet. + const frames = [ + f({ filename: 'app/sentry-replay-test/page.tsx', inApp: true, contextLine: undefined }), + f({ filename: 'node_modules/next/src/client/app-bootstrap.ts', inApp: false, contextLine: 'const root = document;' }), + ]; + assert.equal(pickSuspectFrame(frames)?.filename, 'app/sentry-replay-test/page.tsx'); +}); + +test('pickSuspectFrame falls back to any frame with context when no in-app frame exists', () => { + const frames = [ + f({ filename: 'node_modules/react-dom/index.js', inApp: false, contextLine: 'render()' }), + f({ filename: 'node_modules/next/dist/index.js', inApp: false }), + ]; + assert.equal(pickSuspectFrame(frames)?.filename, 'node_modules/react-dom/index.js'); +}); + +test('pickSuspectFrame falls back to any usable-path frame when nothing has context', () => { + const frames = [ + f({ filename: 'Unknown', inApp: true }), + f({ filename: 'vendor/framework/src/Handler.php', inApp: false }), + ]; + assert.equal(pickSuspectFrame(frames)?.filename, 'vendor/framework/src/Handler.php'); +}); + +test('pickSuspectFrame falls back to the innermost frame when nothing has a usable path', () => { + const frames = [f({ filename: 'Unknown', inApp: true }), f({ filename: '[internal]', inApp: false })]; + assert.equal(pickSuspectFrame(frames)?.filename, '[internal]', 'innermost frame (last in array, first after reverse)'); +}); + +test('pickSuspectFrame returns undefined for an empty frame list', () => { + assert.equal(pickSuspectFrame([]), undefined); +}); diff --git a/apps/workers/src/symbolicate.ts b/apps/workers/src/symbolicate.ts index 95b6f4c..e1e3207 100644 --- a/apps/workers/src/symbolicate.ts +++ b/apps/workers/src/symbolicate.ts @@ -4,7 +4,7 @@ import { gunzipSync } from 'node:zlib'; import type { NormalizedEvent, NormalizedFrame } from '@geniusdebug/shared'; import { getObject, r2Configured } from './r2'; import { symbolicateWithMaps, FRAMEWORK_INTERNAL_RE } from './apply-map'; -import { computeCulprit } from '@geniusdebug/shared'; +import { computeCulprit, normalizeFramePath } from '@geniusdebug/shared'; /** Uploader gzips maps before PUT (build-time cost); gunzip on read here, * detected by magic bytes so pre-existing plain-JSON maps in R2 still work. */ @@ -115,12 +115,10 @@ async function resolveGithub(projectId: string, release?: string): Promise f.inApp && f.contextLine != null) ?? + ordered.find((f) => f.inApp && hasUsableFramePath(f)) ?? + ordered.find((f) => f.contextLine != null) ?? + ordered.find(hasUsableFramePath) ?? + ordered[0] + ); +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index a6c8ebe..a343960 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -5,3 +5,4 @@ export * from './crypto'; export * from './redis'; export * from './webpages'; export * from './culprit'; +export * from './frames';