Skip to content

feat(nextjs): Add browser SDK to app directory browser bundle #6812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
23 changes: 9 additions & 14 deletions packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -500,12 +500,12 @@ function shouldAddSentryToEntryPoint(
excludeServerRoutes: Array<string | RegExp> = [],
isDev: boolean,
): boolean {
if (entryPointName === 'middleware') {
return true;
}

// On the server side, by default we inject the `Sentry.init()` code into every page (with a few exceptions).
if (isServer) {
if (entryPointName === 'middleware') {
return true;
}

const entryPointRoute = entryPointName.replace(/^pages/, '');

// User-specified pages to skip. (Note: For ease of use, `excludeServerRoutes` is specified in terms of routes,
Expand All @@ -527,23 +527,18 @@ function shouldAddSentryToEntryPoint(
// versions.)
entryPointRoute === '/_app' ||
entryPointRoute === '/_document' ||
// Newer versions of nextjs are starting to introduce things outside the `pages/` folder (middleware, an `app/`
// directory, etc), but until those features are stable and we know how we want to support them, the safest bet is
// not to inject anywhere but inside `pages/`.
!entryPointName.startsWith('pages/')
) {
return false;
}

// We want to inject Sentry into all other pages
return true;
}

// On the client side, we only want to inject into `_app`, because that guarantees there'll be only one copy of the
// SDK in the eventual bundle. Since `_app` is the (effectively) the root component for every nextjs app, inclusing
// Sentry there means it will be available for every front end page.
else {
return entryPointName === 'pages/_app';
} else {
return (
entryPointName === 'pages/_app' || // entrypoint for `/pages` pages
entryPointName === 'main-app' // entrypoint for `/app` pages
);
}
}

Expand Down
3 changes: 3 additions & 0 deletions packages/nextjs/test/integration/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,6 @@ yarn-error.log*

# vercel
.vercel

# Generated by Next.js 13
.vscode
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function ({ children }: { children: React.ReactNode }) {
return <>{children}</>;
}
5 changes: 5 additions & 0 deletions packages/nextjs/test/integration/app/clientcomponent/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use client';
Copy link
Member

Choose a reason for hiding this comment

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

lol @ the fact that the nextjs api uses directives like this


export default function () {
return <p>I am a client component!</p>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function ({ children }: { children: React.ReactNode }) {
return <>{children}</>;
}
3 changes: 3 additions & 0 deletions packages/nextjs/test/integration/app/servercomponent/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default async function () {
return <p>I am a server component!</p>;
}
26 changes: 26 additions & 0 deletions packages/nextjs/test/integration/next12.config.template
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
const { withSentryConfig } = require('@sentry/nextjs');

// NOTE: This will be used by integration tests to distinguish between Webpack 4 and Webpack 5
const moduleExports = {
webpack5: %RUN_WEBPACK_5%,
eslint: {
ignoreDuringBuilds: true,
},
pageExtensions: ['jsx', 'js', 'tsx', 'ts', 'page.tsx'],
sentry: {
// Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts`
// TODO (v8): This can come out in v8, because this option will get a default value
hideSourceMaps: false,
excludeServerRoutes: [
'/api/excludedEndpoints/excludedWithString',
/\/api\/excludedEndpoints\/excludedWithRegExp/,
],
},
};

const SentryWebpackPluginOptions = {
dryRun: true,
silent: true,
};

module.exports = withSentryConfig(moduleExports, SentryWebpackPluginOptions);
29 changes: 29 additions & 0 deletions packages/nextjs/test/integration/next13.config.template
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const { withSentryConfig } = require('@sentry/nextjs');

// NOTE: This will be used by integration tests to distinguish between Webpack 4 and Webpack 5
const moduleExports = {
webpack5: %RUN_WEBPACK_5%,
eslint: {
ignoreDuringBuilds: true,
},
experimental: {
appDir: Number(process.env.NODE_MAJOR) >= 16, // experimental.appDir requires Node v16.8.0 or later.
},
pageExtensions: ['jsx', 'js', 'tsx', 'ts', 'page.tsx'],
sentry: {
// Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts`
// TODO (v8): This can come out in v8, because this option will get a default value
hideSourceMaps: false,
excludeServerRoutes: [
'/api/excludedEndpoints/excludedWithString',
/\/api\/excludedEndpoints\/excludedWithRegExp/,
],
},
};

const SentryWebpackPluginOptions = {
dryRun: true,
silent: true,
};

module.exports = withSentryConfig(moduleExports, SentryWebpackPluginOptions);
3 changes: 1 addition & 2 deletions packages/nextjs/test/integration/pages/api/broken/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { withSentry } from '@sentry/nextjs';
import { NextApiRequest, NextApiResponse } from 'next';

const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void> => {
res.status(500).json({ statusCode: 500, message: 'Something went wrong' });
};

export default withSentry(handler);
export default handler;
3 changes: 1 addition & 2 deletions packages/nextjs/test/integration/pages/api/error/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { withSentry } from '@sentry/nextjs';
import { NextApiRequest, NextApiResponse } from 'next';

const handler = async (_req: NextApiRequest, _res: NextApiResponse): Promise<void> => {
throw new Error('API Error');
};

export default withSentry(handler);
export default handler;
3 changes: 1 addition & 2 deletions packages/nextjs/test/integration/pages/api/http/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { withSentry } from '@sentry/nextjs';
import { get } from 'http';
import { NextApiRequest, NextApiResponse } from 'next';

Expand All @@ -9,4 +8,4 @@ const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void
res.status(200).json({});
};

export default withSentry(handler);
export default handler;
3 changes: 1 addition & 2 deletions packages/nextjs/test/integration/pages/api/users/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { withSentry } from '@sentry/nextjs';
import { NextApiRequest, NextApiResponse } from 'next';

import { sampleUserData } from '../../../utils/sample-data';
Expand All @@ -15,4 +14,4 @@ const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void
}
};

export default withSentry(handler);
export default handler;
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { withSentry } from '@sentry/nextjs';
import { NextApiRequest, NextApiResponse } from 'next';

const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void> => {
res.status(200).json({});
};

export default withSentry(handler);
export default handler;
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { withSentry } from '@sentry/nextjs';
import { NextApiRequest, NextApiResponse } from 'next';

const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void> => {
res.status(200).json({});
};

export default withSentry(handler);
export default handler;
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { withSentry } from '@sentry/nextjs';
import { NextApiRequest, NextApiResponse } from 'next';

const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void> => {
res.status(200).json({});
};

export default withSentry(handler);
export default handler;
7 changes: 7 additions & 0 deletions packages/nextjs/test/integration/sentry.edge.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import * as Sentry from '@sentry/nextjs';

Sentry.init({
dsn: 'https://[email protected]/1337',
tracesSampleRate: 1,
debug: process.env.SDK_DEBUG,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const { expectRequestCount, isTransactionRequest, expectTransaction } = require('../utils/client');

module.exports = async ({ page, url, requests }) => {
if (Number(process.env.NEXTJS_VERSION) < 13 || Number(process.env.NODE_MAJOR) < 16) {
// Next.js versions < 13 don't support the app directory and the app dir requires Node v16.8.0 or later.
return;
}

const requestPromise = page.waitForRequest(isTransactionRequest);
await page.goto(`${url}/clientcomponent`);
await requestPromise;

expectTransaction(requests.transactions[0], {
contexts: {
trace: {
op: 'pageload',
},
},
transaction: '/clientcomponent',
});

await expectRequestCount(requests, { transactions: 1 });
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const { expectRequestCount, isTransactionRequest, expectTransaction } = require('../utils/client');

module.exports = async ({ page, url, requests }) => {
if (Number(process.env.NEXTJS_VERSION) < 13 || Number(process.env.NODE_MAJOR) < 16) {
// Next.js versions < 13 don't support the app directory and the app dir requires Node v16.8.0 or later.
return;
}

const requestPromise = page.waitForRequest(isTransactionRequest);
await page.goto(`${url}/servercomponent`);
await requestPromise;

expectTransaction(requests.transactions[0], {
contexts: {
trace: {
op: 'pageload',
},
},
transaction: '/servercomponent',
});

await expectRequestCount(requests, { transactions: 1 });
};
35 changes: 19 additions & 16 deletions packages/nextjs/test/integration/test/client/sessionCrashed.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
const { waitForAll } = require('../utils/common');
const { expectRequestCount, isSessionRequest, expectSession } = require('../utils/client');
const { expectRequestCount, isSessionRequest, expectSession, extractEnvelopeFromRequest } = require('../utils/client');

module.exports = async ({ page, url, requests }) => {
await waitForAll([
page.goto(`${url}/crashed`),
page.waitForRequest(isSessionRequest),
page.waitForRequest(isSessionRequest),
]);

expectSession(requests.sessions[0], {
init: true,
status: 'ok',
errors: 0,
const sessionRequestPromise1 = page.waitForRequest(request => {
if (isSessionRequest(request)) {
const { item } = extractEnvelopeFromRequest(request);
return item.init === true && item.status === 'ok' && item.errors === 0;
} else {
return false;
}
});

expectSession(requests.sessions[1], {
init: false,
status: 'crashed',
errors: 1,
const sessionRequestPromise2 = page.waitForRequest(request => {
if (isSessionRequest(request)) {
const { item } = extractEnvelopeFromRequest(request);
return item.init === false && item.status === 'crashed' && item.errors === 1;
} else {
return false;
}
});

await page.goto(`${url}/crashed`);
await sessionRequestPromise1;
await sessionRequestPromise2;

await expectRequestCount(requests, { sessions: 2 });
};
21 changes: 9 additions & 12 deletions packages/nextjs/test/integration/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@
"forceConsistentCasingInFileNames": true,
"isolatedModules": true,
"jsx": "preserve",
"lib": [
"dom",
"es2017"
],
"lib": ["dom", "es2017"],
"module": "esnext",
"moduleResolution": "node",
"noEmit": true,
Expand All @@ -20,13 +17,13 @@
"skipLibCheck": true,
"strict": true,
"target": "esnext",
"incremental": true
"incremental": true,
"plugins": [
{
"name": "next"
}
]
},
"exclude": [
"node_modules"
],
"include": [
"**/*.ts",
"**/*.tsx"
]
"exclude": ["node_modules"],
"include": ["**/*.ts", "**/*.tsx", ".next/types/**/*.ts"]
}
6 changes: 5 additions & 1 deletion packages/nextjs/test/run-integration-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,12 @@ for NEXTJS_VERSION in 10 11 12 13; do
# next 10 defaults to webpack 4 and next 11 defaults to webpack 5, but each can use either based on settings
if [ "$NEXTJS_VERSION" -eq "10" ]; then
sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next10.config.template >next.config.js
else
elif [ "$NEXTJS_VERSION" -eq "11" ]; then
sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next11.config.template >next.config.js
elif [ "$NEXTJS_VERSION" -eq "12" ]; then
sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next12.config.template >next.config.js
elif [ "$NEXTJS_VERSION" -eq "13" ]; then
sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next13.config.template >next.config.js
fi

echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Building..."
Expand Down