Skip to content

Commit fd42f3b

Browse files
authored
fix(next): Wrap all Random APIs with a safe runner (#18700)
Currently the cache components feature in Next.js prevents us from using any random value APIs like: - `Date.now` - `performance.now` - `Math.random` - `crypto.*` We tried resolving this by patching several span methods, but then we have plenty of other instances where we use those APIs, like in trace propagation, timestamp generation for logs, and more. Running around and patching them one by one in the Next.js SDK isn't a viable solution since most of those functionalities are strictly internal and cannot be patched from the outside, and adding escape hatches for each of them is not maintainable. So I'm testing out the other way around, by hunting those APIs down and wrapping them with a safe runner that acts as an escape hatch. Some of the Vercel engineers suggested doing that, but we need to do it for ~almost every call~ (see Josh [comment](#18700 (comment)) below). The idea is an SDK can "turn on" the safe runner by injecting a global function that executes a callback and returns its results. I ### How does this fix it for Next.js? The Next.js SDK case, a safe runner would be an `AsyncLocalStorage` snapshot which is captured at the server runtime init, way before any rendering is done. ```ts const sym = Symbol.for('__SENTRY_SAFE_RANDOM_ID_WRAPPER__'); const globalWithSymbol: typeof GLOBAL_OBJ & { [sym]?: SafeRandomContextRunner } = GLOBAL_OBJ; globalWithSymbol[sym] = AsyncLocalStorage.snapshot(); // core SDK then offers a fn to run any random gen function export function withRandomSafeContext<T>(cb: () => T): T { // Looks for the global symbol and if it is set it uses the runner // otherwise just runs the callback normally. } ``` I kept the API internal as much as possible to avoid users messing up with it, but the `@sentry/opentelemetry` SDK also needed this functionality so I exported the API with `_INTERNAL` prefix as we already do. --- I tested this in a simple Next.js app and it no longer errors out, and all current tests pass. I still need to take a look at the traces and see how would they look in cached component cases. Charly is already working on this and may have a proper solution, but I thought to just see if we can ship a stopgap until then. On the bright side, this seems to fix it as well for Webpack. closes #18392 closes #18340
1 parent 8eb1d44 commit fd42f3b

File tree

31 files changed

+598
-30
lines changed

31 files changed

+598
-30
lines changed

.size-limit.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ module.exports = [
8282
path: 'packages/browser/build/npm/esm/prod/index.js',
8383
import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
8484
gzip: true,
85-
limit: '85 KB',
85+
limit: '85.5 KB',
8686
},
8787
{
8888
name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
@@ -243,7 +243,7 @@ module.exports = [
243243
import: createImport('init'),
244244
ignore: ['$app/stores'],
245245
gzip: true,
246-
limit: '42 KB',
246+
limit: '42.5 KB',
247247
},
248248
// Node-Core SDK (ESM)
249249
{
@@ -261,7 +261,7 @@ module.exports = [
261261
import: createImport('init'),
262262
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
263263
gzip: true,
264-
limit: '162 KB',
264+
limit: '162.5 KB',
265265
},
266266
{
267267
name: '@sentry/node - without tracing',
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import * as Sentry from '@sentry/nextjs';
2+
3+
function fetchPost() {
4+
return Promise.resolve({ id: '1', title: 'Post 1' });
5+
}
6+
7+
export async function generateMetadata() {
8+
const { id } = await fetchPost();
9+
const product = `Product: ${id}`;
10+
11+
return {
12+
title: product,
13+
};
14+
}
15+
16+
export default function Page() {
17+
return (
18+
<>
19+
<h1>This will be pre-rendered</h1>
20+
<DynamicContent />
21+
</>
22+
);
23+
}
24+
25+
async function DynamicContent() {
26+
const getTodos = async () => {
27+
return Sentry.startSpan({ name: 'getTodos', op: 'get.todos' }, async () => {
28+
'use cache';
29+
await new Promise(resolve => setTimeout(resolve, 100));
30+
return [1, 2, 3, 4, 5];
31+
});
32+
};
33+
34+
const todos = await getTodos();
35+
36+
return <div id="todos-fetched">Todos fetched: {todos.length}</div>;
37+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import * as Sentry from '@sentry/nextjs';
2+
3+
/**
4+
* Tests generateMetadata function with cache components, this calls the propagation context to be set
5+
* Which will generate and set a trace id in the propagation context, which should trigger the random API error if unpatched
6+
* See: https://github.com/getsentry/sentry-javascript/issues/18392
7+
*/
8+
export function generateMetadata() {
9+
return {
10+
title: 'Cache Components Metadata Test',
11+
};
12+
}
13+
14+
export default function Page() {
15+
return (
16+
<>
17+
<h1>This will be pre-rendered</h1>
18+
<DynamicContent />
19+
</>
20+
);
21+
}
22+
23+
async function DynamicContent() {
24+
const getTodos = async () => {
25+
return Sentry.startSpan({ name: 'getTodos', op: 'get.todos' }, async () => {
26+
'use cache';
27+
await new Promise(resolve => setTimeout(resolve, 100));
28+
return [1, 2, 3, 4, 5];
29+
});
30+
};
31+
32+
const todos = await getTodos();
33+
34+
return <div id="todos-fetched">Todos fetched: {todos.length}</div>;
35+
}

dev-packages/e2e-tests/test-applications/nextjs-16-cacheComponents/tests/cacheComponents.spec.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,29 @@ test('Should render suspense component', async ({ page }) => {
2626
expect(serverTx.spans?.filter(span => span.op === 'get.todos').length).toBeGreaterThan(0);
2727
await expect(page.locator('#todos-fetched')).toHaveText('Todos fetched: 5');
2828
});
29+
30+
test('Should generate metadata', async ({ page }) => {
31+
const serverTxPromise = waitForTransaction('nextjs-16-cacheComponents', async transactionEvent => {
32+
return transactionEvent.contexts?.trace?.op === 'http.server';
33+
});
34+
35+
await page.goto('/metadata');
36+
const serverTx = await serverTxPromise;
37+
38+
expect(serverTx.spans?.filter(span => span.op === 'get.todos')).toHaveLength(0);
39+
await expect(page.locator('#todos-fetched')).toHaveText('Todos fetched: 5');
40+
await expect(page).toHaveTitle('Cache Components Metadata Test');
41+
});
42+
43+
test('Should generate metadata async', async ({ page }) => {
44+
const serverTxPromise = waitForTransaction('nextjs-16-cacheComponents', async transactionEvent => {
45+
return transactionEvent.contexts?.trace?.op === 'http.server';
46+
});
47+
48+
await page.goto('/metadata-async');
49+
const serverTx = await serverTxPromise;
50+
51+
expect(serverTx.spans?.filter(span => span.op === 'get.todos')).toHaveLength(0);
52+
await expect(page.locator('#todos-fetched')).toHaveText('Todos fetched: 5');
53+
await expect(page).toHaveTitle('Product: 1');
54+
});

packages/core/.eslintrc.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,15 @@
11
module.exports = {
22
extends: ['../../.eslintrc.js'],
33
ignorePatterns: ['rollup.npm.config.mjs'],
4+
rules: {
5+
'@sentry-internal/sdk/no-unsafe-random-apis': 'error',
6+
},
7+
overrides: [
8+
{
9+
files: ['test/**/*.ts', 'test/**/*.tsx'],
10+
rules: {
11+
'@sentry-internal/sdk/no-unsafe-random-apis': 'off',
12+
},
13+
},
14+
],
415
};

packages/core/src/client.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import { checkOrSetAlreadyCaught, uuid4 } from './utils/misc';
4545
import { parseSampleRate } from './utils/parseSampleRate';
4646
import { prepareEvent } from './utils/prepareEvent';
4747
import { makePromiseBuffer, type PromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from './utils/promisebuffer';
48+
import { safeMathRandom } from './utils/randomSafeContext';
4849
import { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span';
4950
import { showSpanDropWarning } from './utils/spanUtils';
5051
import { rejectedSyncPromise } from './utils/syncpromise';
@@ -1288,7 +1289,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
12881289
// 0.0 === 0% events are sent
12891290
// Sampling for transaction happens somewhere else
12901291
const parsedSampleRate = typeof sampleRate === 'undefined' ? undefined : parseSampleRate(sampleRate);
1291-
if (isError && typeof parsedSampleRate === 'number' && Math.random() > parsedSampleRate) {
1292+
if (isError && typeof parsedSampleRate === 'number' && safeMathRandom() > parsedSampleRate) {
12921293
this.recordDroppedEvent('sample_rate', 'error');
12931294
return rejectedSyncPromise(
12941295
_makeDoNotSendEventError(

packages/core/src/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,3 +515,9 @@ export type {
515515
UnstableRollupPluginOptions,
516516
UnstableWebpackPluginOptions,
517517
} from './build-time-plugins/buildTimeOptionsBase';
518+
export {
519+
withRandomSafeContext as _INTERNAL_withRandomSafeContext,
520+
type RandomSafeContextRunner as _INTERNAL_RandomSafeContextRunner,
521+
safeMathRandom as _INTERNAL_safeMathRandom,
522+
safeDateNow as _INTERNAL_safeDateNow,
523+
} from './utils/randomSafeContext';

packages/core/src/integrations/mcp-server/correlation.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export function storeSpanForRequest(transport: MCPTransport, requestId: RequestI
4646
spanMap.set(requestId, {
4747
span,
4848
method,
49+
// eslint-disable-next-line @sentry-internal/sdk/no-unsafe-random-apis
4950
startTime: Date.now(),
5051
});
5152
}

packages/core/src/scope.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { isPlainObject } from './utils/is';
2222
import { merge } from './utils/merge';
2323
import { uuid4 } from './utils/misc';
2424
import { generateTraceId } from './utils/propagationContext';
25+
import { safeMathRandom } from './utils/randomSafeContext';
2526
import { _getSpanForScope, _setSpanForScope } from './utils/spanOnScope';
2627
import { truncate } from './utils/string';
2728
import { dateTimestampInSeconds } from './utils/time';
@@ -168,7 +169,7 @@ export class Scope {
168169
this._sdkProcessingMetadata = {};
169170
this._propagationContext = {
170171
traceId: generateTraceId(),
171-
sampleRand: Math.random(),
172+
sampleRand: safeMathRandom(),
172173
};
173174
}
174175

@@ -550,7 +551,10 @@ export class Scope {
550551
this._session = undefined;
551552
_setSpanForScope(this, undefined);
552553
this._attachments = [];
553-
this.setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
554+
this.setPropagationContext({
555+
traceId: generateTraceId(),
556+
sampleRand: safeMathRandom(),
557+
});
554558

555559
this._notifyScopeListeners();
556560
return this;

packages/core/src/tracing/trace.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { handleCallbackErrors } from '../utils/handleCallbackErrors';
1717
import { hasSpansEnabled } from '../utils/hasSpansEnabled';
1818
import { parseSampleRate } from '../utils/parseSampleRate';
1919
import { generateTraceId } from '../utils/propagationContext';
20+
import { safeMathRandom } from '../utils/randomSafeContext';
2021
import { _getSpanForScope, _setSpanForScope } from '../utils/spanOnScope';
2122
import { addChildSpanToSpan, getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
2223
import { propagationContextFromHeaders, shouldContinueTrace } from '../utils/tracing';
@@ -293,7 +294,7 @@ export function startNewTrace<T>(callback: () => T): T {
293294
return withScope(scope => {
294295
scope.setPropagationContext({
295296
traceId: generateTraceId(),
296-
sampleRand: Math.random(),
297+
sampleRand: safeMathRandom(),
297298
});
298299
DEBUG_BUILD && debug.log(`Starting a new trace with id ${scope.getPropagationContext().traceId}`);
299300
return withActiveSpan(null, callback);

0 commit comments

Comments
 (0)