Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ test('links a cached layout hit to the trace that filled the layout entry', asyn
const putSpans = (missTx.spans ?? []).filter(span => span.op === 'cache.put');
expect(new Set(putSpans.map(span => span.description)).size).toBe(1);

expect(putSpans[0]!.data?.['code.file.path']).toBe('app/(cached-nesting)/cached-mid-layout/[id]/layout.tsx');

const hitGetSpan = hitTx.spans?.find(span => span.op === 'cache.get' && span.data?.['cache.hit'] === true);
expect(hitGetSpan).toBeDefined();
expect(hitGetSpan?.description).toBe(putSpans[0]!.description);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ test('Should create cache spans around `use cache` functions', async ({ request
}),
});

// Route handler cache functions have no server-reference manifest entry, so no source file.
expect(putSpan!.data).not.toHaveProperty('code.file.path');

const hitGetSpan = hitTx.spans?.find(span => span.op === 'cache.get');
expect(hitGetSpan).toBeDefined();
expect(hitGetSpan).toMatchObject({
Expand Down Expand Up @@ -97,7 +100,12 @@ test('Should create cache spans for `use cache` inside a rendered page', async (
await request.get(`/use-cache-page?id=${id}`);
const hitTx = await hitTxPromise;

expect(missTx.spans?.some(span => span.op === 'cache.put')).toBe(true);
// The source file on a fill span marks which cached function produced the entry.
const missPutSpans = missTx.spans?.filter(span => span.op === 'cache.put') ?? [];
expect(missPutSpans.length).toBeGreaterThan(0);
for (const putSpan of missPutSpans) {
expect(putSpan.data?.['code.file.path']).toBe('app/use-cache-page/page.tsx');
}

// A render can read more than one cache entry, so look at every hit instead of the first `cache.get`.
const hitGetSpans = hitTx.spans?.filter(span => span.op === 'cache.get' && span.data?.['cache.hit'] === true) ?? [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ test('links a cached layout hit to the trace that filled the layout entry', asyn
const putSpan = findCacheSpan(missSpans, 'cache.put');
expect(putSpan).toBeDefined();

expect(putSpan!.attributes['code.file.path']?.value).toBe('app/(cached-nesting)/cached-mid-layout/[id]/layout.tsx');
Comment thread
s1gr1d marked this conversation as resolved.
Resolved

const hitGetSpan = findCacheSpan(hitSpans, 'cache.get', true);
expect(hitGetSpan).toBeDefined();
expect(hitGetSpan!.attributes['cache.key']).toEqual(putSpan!.attributes['cache.key']);
Expand Down Expand Up @@ -114,6 +116,14 @@ test('links two cached levels to different origin traces after the layout expire
const fillPutSpans = fillSpans.filter(span => getSpanOp(span) === 'cache.put');
expect(new Set(fillPutSpans.map(span => JSON.stringify(span.attributes['cache.key']?.value))).size).toBe(2);

// One fill per file: the layout (multipart key) and the cached component (JSON key).
expect(new Set(fillPutSpans.map(span => span.attributes['code.file.path']?.value))).toEqual(
new Set([
'app/(cached-nesting)/mixed-lifetimes/[id]/layout.tsx',
'app/(cached-nesting)/mixed-lifetimes/[id]/page.tsx',
]),
);

// Sleep past the layout's `expire` (2s); the component entry stays valid for hours.
await new Promise(resolve => setTimeout(resolve, 3_000));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,33 @@ test('uses low-cardinality names for `use cache` spans', async ({ request }) =>
expect(putSpan).toBeDefined();
expect(putSpan!.name).toBe('cache.put');
expect(putSpan!.attributes['cache.key']?.value).toEqual(cacheKeyDigest);

// Route handler cache functions have no server-reference manifest entry, so no source file.
expect(putSpan!.attributes['code.file.path']).toBeUndefined();
});

test('sets the source file of the cached component on `cache.put` spans', async ({ request }) => {
const id = crypto.randomUUID();

const spansPromise = collectStreamedSpans('nextjs-16-streaming-cacheComponents', spansOfTrace => {
return (
spansOfTrace.some(span => span.name === 'GET /cached-sibling-components' && span.is_segment) &&
spansOfTrace.filter(span => getSpanOp(span) === 'cache.put').length >= 2
);
});

await request.get(`/cached-sibling-components?id=${id}`);
const spans = await spansPromise;

// Both sibling entries come from the same page file.
const putSpans = spans.filter(span => getSpanOp(span) === 'cache.put');
expect(putSpans.length).toBeGreaterThanOrEqual(2);
for (const putSpan of putSpans) {
expect(putSpan.attributes['code.file.path']?.value).toBe('app/cached-sibling-components/page.tsx');
}

// The source file marks the producer of an entry. Reads do not carry it.
const getSpan = findCacheSpan(spans, 'cache.get');
expect(getSpan).toBeDefined();
expect(getSpan!.attributes['code.file.path']).toBeUndefined();
});
25 changes: 19 additions & 6 deletions packages/nextjs/src/server/useCacheInstrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
CACHE_OPERATION,
CACHE_TAGS,
CACHE_TTL,
CODE_FILE_PATH,
SENTRY_LINK_TYPE,
SENTRY_ORIGIN,
} from '@sentry/conventions/attributes';
Expand All @@ -26,6 +27,7 @@ import {
timestampInSeconds,
} from '@sentry/core';
import { DEBUG_BUILD } from '../common/debug-build';
import { getCacheFunctionSourceFile } from './useCacheSourceFile';

// Next.js shares its `use cache` handlers across bundles via `globalThis`
// (`next/src/server/use-cache/handlers.ts`). This module can load once per bundle, so all
Expand Down Expand Up @@ -268,24 +270,35 @@ function instrumentHandler(handler: unknown): void {

fill(handler, 'set', (originalSet: UseCacheHandler['set']) => {
return function (this: UseCacheHandler, cacheKey: string, pendingEntry: Promise<unknown>): Promise<void> {
const digest = keyDigest(cacheKey);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: Can we gate this with shouldRecordCacheSpan again?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sure! seems like the conflicts merge got it wrong


// A successful write replaces the entry, so a remembered origin from a previous fill is now wrong.
// An unsampled fill has no span to link to -> remember nothing instead.
if (!shouldRecordCacheSpan()) {
return originalSet.call(this, cacheKey, pendingEntry);
return Promise.resolve(originalSet.call(this, cacheKey, pendingEntry)).then(result => {
getCacheOrigins().remove(originKeyPrefix + digest);
return result;
});
}

const digest = keyDigest(cacheKey);
const sourceFile = getCacheFunctionSourceFile(cacheKey);

// The handler drains `pendingEntry` (the still-streaming entry) before storing, so this
// span covers producing and storing the entry, not just the write.
return startCacheSpan(CACHE_PUT, digest, span =>
return startCacheSpan(CACHE_PUT, digest, span => {
if (sourceFile) {
span.setAttribute(CODE_FILE_PATH, sourceFile);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Span attribute set after start

Medium Severity

sourceFile is already resolved before startCacheSpan runs, but code.file.path is attached afterward with span.setAttribute. tracesSampler and ignoreSpans only see attributes passed into startSpan, so they cannot filter these cache fill spans by source file.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit b2703e7. Configure here.

// Only a successful write becomes a fill origin: a failed write leaves no entry or the
// previous one (whose origin still stands). A dropped span (`ignoreSpans`) never
// reaches Sentry, so a link to it would be broken.
Promise.resolve(originalSet.call(this, cacheKey, pendingEntry)).then(result => {
return Promise.resolve(originalSet.call(this, cacheKey, pendingEntry)).then(result => {
if (span.isRecording()) {
rememberCacheOrigin(originKeyPrefix + digest, span, pendingEntry);
}
return result;
}),
);
});
});
};
});
} catch (error) {
Expand Down
134 changes: 134 additions & 0 deletions packages/nextjs/src/server/useCacheSourceFile.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Next.js' runtime manifest registry
// https://github.com/vercel/next.js/blob/8e0700c74474498a07b33f58da1c1316f740eb19/packages/next/src/server/app-render/manifests-singleton.ts#L56-L59
const NEXT_MANIFESTS_SINGLETON = Symbol.for('next.server.manifests');

type ManifestEntries = Record<string, { filename?: unknown } | undefined>;

type GlobalWithManifests = typeof globalThis & {
[NEXT_MANIFESTS_SINGLETON]?: {
serverActionsManifest?: {
node?: ManifestEntries;
edge?: ManifestEntries;
};
};
};

interface MultipartField {
content: string;
/** Index of the first character after this field. */
end: number;
}

/**
* Reads one field of a multipart cache key, starting at `start`.
* A field is `<length>:<content>`, with the length in lowercase hex counting UTF-16 code units.
* https://github.com/vercel/next.js/blob/8e0700c74474498a07b33f58da1c1316f740eb19/packages/next/src/server/use-cache/use-cache-wrapper.ts#L1815-L1847
*
* Returns `undefined` when the framing is broken.
*/
function readMultipartField(cacheKey: string, start: number): MultipartField | undefined {
// The characters before the next `:` must be a non-empty hex length.
const colon = cacheKey.indexOf(':', start);
if (colon <= start || !/^[0-9a-f]+$/.test(cacheKey.slice(start, colon))) {
return undefined;
}

// A length that points past the end of the key means the key is truncated.
const contentStart = colon + 1;
const contentEnd = contentStart + parseInt(cacheKey.slice(start, colon), 16);
if (contentEnd > cacheKey.length) {
return undefined;
}

return { content: cacheKey.slice(contentStart, contentEnd), end: contentEnd };
}

/**
* Cache keys with arguments that do not serialize to JSON (a page's `params` promise, a layout's
* `children`) are serialized FormData: pairs of length-prefixed fields, a field name followed by
* its content. React stores the key parts JSON in the field named `"0"`, which is not necessarily
* the first field. Returns that JSON text, or `undefined` for a malformed key.
*/
function readKeyPartsFromMultipartKey(cacheKey: string): string | undefined {
let position = 0;
while (position < cacheKey.length) {
// Each pair is the field name, then the field content.
const name = readMultipartField(cacheKey, position);
const content = name && readMultipartField(cacheKey, name.end);
if (name === undefined || content === undefined) {
return undefined;
}

if (name.content === '0') {
return content.content;
}
position = content.end;
}
return undefined;
}

/**
* Picks the function id out of the decoded key parts. Next.js 16.3 puts the id at index 1
* (`[buildId, id, args]` in prod; dev appends a fourth part). 16.4 canary moves it to index 0
* (`[id, args, …]`). Returns `undefined` for any other shape.
*/
function readFunctionIdFromKeyParts(keyParts: unknown): string | undefined {
if (!Array.isArray(keyParts)) {
return undefined;
}
// An args array at index 1 marks the canary shape, where the id sits at index 0.
const functionId = Array.isArray(keyParts[1]) ? keyParts[0] : keyParts[1];
return typeof functionId === 'string' ? functionId : undefined;
}

/**
* Manifest filenames start at the repo root, but only the path inside the project is useful.
* The repo-root-to-project part equals the tail of `process.cwd()`, because `next dev`,
* `next start`, and the standalone server all run in the project directory (the standalone
* `server.js` chdirs into its mirrored copy). Unknown layouts keep the full path.
*/
function toProjectRelativePath(filename: string): string {
// Split the cwd on both separators so Windows paths work. The manifest always uses `/`.
const cwdSegments = process.cwd().split(/[\\/]/).filter(Boolean);
const fileSegments = filename.split('/');

// Drop the longest filename prefix that matches the cwd tail. Longest first, so the whole
// repo prefix goes, not just a part of it. At least one segment always remains.
const maxOverlap = Math.min(fileSegments.length - 1, cwdSegments.length);
for (let overlap = maxOverlap; overlap > 0; overlap--) {
const cwdTail = cwdSegments.slice(-overlap);
if (cwdTail.every((segment, index) => segment === fileSegments[index])) {
return fileSegments.slice(overlap).join('/');
}
}
return filename;
}

/**
* Resolves the source file of the `use cache` function behind a cache key, through the server-reference manifest.
* On Next.js 16.3 the manifest only covers component-tree functions, so route handlers resolve to `undefined`.
* 16.4 canary includes route handlers.
* Any unexpected key or manifest shape returns `undefined`, never a wrong file.
*/
export function getCacheFunctionSourceFile(cacheKey: string): string | undefined {
try {
// The key is `encodeReply(keyParts)`: a plain JSON array when all function arguments
// serialize to JSON, the multipart form otherwise.
const keyPartsJson = cacheKey.startsWith('[') ? cacheKey : readKeyPartsFromMultipartKey(cacheKey);
if (!keyPartsJson) {
return undefined;
}

const functionId = readFunctionIdFromKeyParts(JSON.parse(keyPartsJson));
if (functionId === undefined) {
return undefined;
}

// The manifest has one section per runtime. `NEXT_RUNTIME` is only `'edge'` on edge.
const manifest = (globalThis as GlobalWithManifests)[NEXT_MANIFESTS_SINGLETON]?.serverActionsManifest;
const filename = manifest?.[process.env.NEXT_RUNTIME === 'edge' ? 'edge' : 'node']?.[functionId]?.filename;
return typeof filename === 'string' && filename ? toProjectRelativePath(filename) : undefined;
} catch {
return undefined;
}
}
74 changes: 74 additions & 0 deletions packages/nextjs/test/server/useCacheInstrumentation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { _instrumentUseCacheHandlers } from '../../src/server/useCacheInstrument

const NEXT_CACHE_HANDLERS_MAP = Symbol.for('@next/cache-handlers-map');
const NEXT_PRIVATE_CACHE_HANDLER = Symbol.for('@next/cache-handlers-private');
const NEXT_MANIFESTS_SINGLETON = Symbol.for('next.server.manifests');
const SENTRY_CACHE_INSTRUMENTED = Symbol.for('sentry.nextjs.cacheHandlersInstrumented');
const SENTRY_WRAPPED_HANDLERS = Symbol.for('sentry.nextjs.wrappedCacheHandlers');
const SENTRY_CACHE_ORIGINS = Symbol.for('sentry.nextjs.cacheOrigins');
Expand Down Expand Up @@ -64,6 +65,7 @@ describe('instrumentUseCacheHandlers', () => {
beforeEach(() => {
mocks.activeSpan = {};
mocks.sampled = true;
mocks.state.spanCount = 0;
mocks.client = undefined;
mocks.state.spanCount = 0;
mocks.state.recording = true;
Expand All @@ -77,6 +79,7 @@ describe('instrumentUseCacheHandlers', () => {
for (const symbol of [
NEXT_CACHE_HANDLERS_MAP,
NEXT_PRIVATE_CACHE_HANDLER,
NEXT_MANIFESTS_SINGLETON,
SENTRY_CACHE_INSTRUMENTED,
SENTRY_WRAPPED_HANDLERS,
SENTRY_CACHE_ORIGINS,
Expand Down Expand Up @@ -468,6 +471,35 @@ describe('instrumentUseCacheHandlers', () => {
expect(mocks.addLink).not.toHaveBeenCalled();
});

it('forgets a remembered origin when the entry is refilled without a sampled parent span', async () => {
const handler = installWithDefaultHandler({ timestamp: nowMs() });

await handler.set('cache-key', Promise.resolve({}));

mocks.activeSpan = undefined;
await handler.set('cache-key', Promise.resolve({}));
mocks.activeSpan = {};

await handler.get('cache-key');

expect(mocks.addLink).not.toHaveBeenCalled();
});

it('forgets a remembered origin when the refill `cache.put` span is not recording', async () => {
const handler = installWithDefaultHandler({ timestamp: nowMs() });

await handler.set('cache-key', Promise.resolve({}));

// e.g. the `cache.put` op is filtered via `ignoreSpans`
mocks.state.recording = false;
await handler.set('cache-key', Promise.resolve({}));
mocks.state.recording = true;

await handler.get('cache-key');

expect(mocks.addLink).not.toHaveBeenCalled();
});

it('does not remember fills whose write failed', async () => {
const entry = { timestamp: nowMs() };
const handler = {
Expand All @@ -484,6 +516,48 @@ describe('instrumentUseCacheHandlers', () => {
});
});

describe('source file on `cache.put`', () => {
const functionId = 'c05120808bb68f6400d039e720226869fb1f079019';
const jsonCacheKey = JSON.stringify(['build-id', functionId, [['arg'], {}]]);

function setManifest(filename: unknown): void {
setGlobal(NEXT_MANIFESTS_SINGLETON, {
serverActionsManifest: { node: { [functionId]: { filename } } },
});
}

it('sets `code.file.path` when the key parses and the manifest knows the function', async () => {
setManifest('app/(cached-nesting)/mixed-lifetimes/[id]/layout.tsx');
const handler = installWithDefaultHandler();

await handler.set(jsonCacheKey, Promise.resolve({}));

expect(mocks.setAttribute).toHaveBeenCalledWith(
'code.file.path',
'app/(cached-nesting)/mixed-lifetimes/[id]/layout.tsx',
);
});

it('does not set `code.file.path` on `cache.get` spans', async () => {
setManifest('app/page.tsx');
const handler = installWithDefaultHandler({ timestamp: nowMs() });

await handler.get(jsonCacheKey);

expect(mocks.setAttribute).not.toHaveBeenCalledWith('code.file.path', expect.anything());
});

// Key parsing, manifest lookup, and path shortening are covered in `useCacheSourceFile.test.ts`.
it('still writes the entry and omits `code.file.path` when the key does not resolve', async () => {
setManifest('app/page.tsx');
const handler = installWithDefaultHandler();

await expect(handler.set('multipart-encoded-key', Promise.resolve({}))).resolves.toBeUndefined();

expect(mocks.setAttribute).not.toHaveBeenCalledWith('code.file.path', expect.anything());
});
});

it('creates a `cache.put` span around handler writes', async () => {
const handler = installWithDefaultHandler();

Expand Down
Loading