From 4b6ae894fe9056bdd3b831c1d603164b6d2fc0d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Sun, 25 Oct 2020 21:54:44 +0100 Subject: [PATCH 1/3] Expose more data to error plugin hooks --- .../webpack/loaders/next-serverless-loader.ts | 4 +-- packages/next/client/index.tsx | 27 +++++++++++++------ .../next/next-server/lib/router/router.ts | 5 ++++ .../next/next-server/server/next-server.ts | 19 ++++++++++--- 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 739a40d3ce747..67640a06589dd 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -428,7 +428,7 @@ const nextServerlessLoader: loader.Loader = function () { ) } catch (err) { console.error(err) - await onError(err) + await onError({ err, req, res }) // TODO: better error for DECODE_FAILED? if (err.code === 'DECODE_FAILED') { @@ -846,7 +846,7 @@ const nextServerlessLoader: loader.Loader = function () { } } catch(err) { console.error(err) - await onError(err) + await onError({ err, req, res }) // Throw the error to crash the serverless function throw err } diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index 5cc0056da5c24..1554bac2ffb8e 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -9,6 +9,7 @@ import type Router from '../next-server/lib/router/router' import type { AppComponent, AppProps, + ErrorInfo, PrivateRouteInfo, } from '../next-server/lib/router/router' import { delBasePath, hasBasePath } from '../next-server/lib/router/router' @@ -142,10 +143,10 @@ let cachedStyleSheets: StyleSheetTuple[] let CachedApp: AppComponent, onPerfEntry: (metric: any) => void class Container extends React.Component<{ - fn: (err: Error, info?: any) => void + fn: (err: Error, errorInfo?: any) => void }> { - componentDidCatch(componentErr: Error, info: any) { - this.props.fn(componentErr, info) + componentDidCatch(componentErr: Error, errorInfo: ErrorInfo) { + this.props.fn(componentErr, errorInfo) } componentDidMount() { @@ -391,7 +392,7 @@ export async function render(renderingProps: RenderRouteInfo) { // 404 and 500 errors are special kind of errors // and they are still handle via the main render method. export function renderError(renderErrorProps: RenderErrorProps) { - const { App, err } = renderErrorProps + const { App, err, errorInfo } = renderErrorProps // In development runtime errors are caught by our overlay // In production we catch runtime errors using componentDidCatch which will trigger renderError @@ -409,12 +410,19 @@ export function renderError(renderErrorProps: RenderErrorProps) { styleSheets: [], }) } + if (process.env.__NEXT_PLUGINS) { // @ts-ignore // eslint-disable-next-line import('next-plugin-loader?middleware=on-error-client!') .then((onClientErrorModule) => { - return onClientErrorModule.default({ err }) + return onClientErrorModule.default({ + err, + errorInfo, + renderErrorProps, + data, + version, + }) }) .catch((onClientErrorErr) => { console.error( @@ -437,7 +445,7 @@ export function renderError(renderErrorProps: RenderErrorProps) { Component: ErrorComponent, AppTree, router, - ctx: { err, pathname: page, query, asPath, AppTree }, + ctx: { err, errorInfo, pathname: page, query, asPath, AppTree }, } return Promise.resolve( renderErrorProps.props @@ -447,6 +455,7 @@ export function renderError(renderErrorProps: RenderErrorProps) { doRender({ ...renderErrorProps, err, + errorInfo, Component: ErrorComponent, styleSheets, props: initProps, @@ -544,8 +553,8 @@ function AppContainer({ }: React.PropsWithChildren<{}>): React.ReactElement { return ( - renderError({ App: CachedApp, err: error }).catch((err) => + fn={(error, errorInfo) => + renderError({ App: CachedApp, err: error, errorInfo }).catch((err) => console.error('Error rendering page: ', err) ) } @@ -580,6 +589,7 @@ function doRender({ Component, props, err, + errorInfo, styleSheets, }: RenderRouteInfo): Promise { Component = Component || lastAppProps.Component @@ -589,6 +599,7 @@ function doRender({ ...props, Component, err, + errorInfo, router, } // lastAppProps has to be set before ReactDom.render to account for ReactDom throwing an error. diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index e027649823d03..a61b137878ff4 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -271,6 +271,10 @@ export type PrefetchOptions = { priority?: boolean } +export type ErrorInfo = { + componentStack?: string +} + export type PrivateRouteInfo = { Component: ComponentType styleSheets: StyleSheetTuple[] @@ -279,6 +283,7 @@ export type PrivateRouteInfo = { props?: Record err?: Error error?: any + errorInfo?: ErrorInfo } export type AppProps = Pick & { diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 4102a0f48c2e2..966f688b6cec2 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -147,7 +147,15 @@ export default class Server { defaultLocale?: string } private compression?: Middleware - private onErrorMiddleware?: ({ err }: { err: Error }) => Promise + private onErrorMiddleware?: ({ + err, + req, + res, + }: { + err: Error + req: IncomingMessage + res: ServerResponse + }) => Promise private incrementalCache: IncrementalCache router: Router protected dynamicRoutes?: DynamicRoutes @@ -273,9 +281,6 @@ export default class Server { } public logError(err: Error): void { - if (this.onErrorMiddleware) { - this.onErrorMiddleware({ err }) - } if (this.quiet) return console.error(err) } @@ -437,6 +442,9 @@ export default class Server { try { return await this.run(req, res, parsedUrl) } catch (err) { + if (this.onErrorMiddleware) { + this.onErrorMiddleware({ err, req, res }) + } this.logError(err) res.statusCode = 500 res.end('Internal Server Error') @@ -1585,6 +1593,9 @@ export default class Server { } } } catch (err) { + if (this.onErrorMiddleware) { + this.onErrorMiddleware({ err, req, res }) + } this.logError(err) if (err && err.code === 'DECODE_FAILED') { From c158301a2583b366c1c5e07b814e13d712e30e8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Mon, 26 Oct 2020 21:39:44 +0100 Subject: [PATCH 2/3] Extend next-plugin-sentry functionality to handle source-maps and extract some metadata --- packages/next-plugin-sentry/config.js | 6 ++ packages/next-plugin-sentry/env.js | 9 ++ packages/next-plugin-sentry/index.js | 11 +++ packages/next-plugin-sentry/package.json | 11 +-- packages/next-plugin-sentry/readme.md | 99 ++++++++++++++++++- .../next-plugin-sentry/src/on-error-client.js | 13 ++- .../next-plugin-sentry/src/on-error-server.js | 28 +++++- .../next-plugin-sentry/src/on-init-client.js | 23 +++-- .../next-plugin-sentry/src/on-init-server.js | 42 ++++++-- 9 files changed, 215 insertions(+), 27 deletions(-) create mode 100644 packages/next-plugin-sentry/config.js create mode 100644 packages/next-plugin-sentry/env.js create mode 100644 packages/next-plugin-sentry/index.js diff --git a/packages/next-plugin-sentry/config.js b/packages/next-plugin-sentry/config.js new file mode 100644 index 0000000000000..e6c6b7732757e --- /dev/null +++ b/packages/next-plugin-sentry/config.js @@ -0,0 +1,6 @@ +/** + * TODO(kamil): Unify SDK configuration options. + * Workaround for callbacks, integrations and any unserializable config options. + **/ +exports.serverConfig = {} +exports.clientConfig = {} diff --git a/packages/next-plugin-sentry/env.js b/packages/next-plugin-sentry/env.js new file mode 100644 index 0000000000000..3aa805e43e6bb --- /dev/null +++ b/packages/next-plugin-sentry/env.js @@ -0,0 +1,9 @@ +export const getDsn = () => + process.env.SENTRY_DSN || process.env.NEXT_PUBLIC_SENTRY_DSN + +export const getRelease = () => + process.env.SENTRY_RELEASE || + process.env.NEXT_PUBLIC_SENTRY_RELEASE || + process.env.VERCEL_GITHUB_COMMIT_SHA || + process.env.VERCEL_GITLAB_COMMIT_SHA || + process.env.VERCEL_BITBUCKET_COMMIT_SHA diff --git a/packages/next-plugin-sentry/index.js b/packages/next-plugin-sentry/index.js new file mode 100644 index 0000000000000..7bd20707328a3 --- /dev/null +++ b/packages/next-plugin-sentry/index.js @@ -0,0 +1,11 @@ +const { serverConfig, clientConfig } = require('./config.js') + +const Sentry = require('@sentry/minimal') +// NOTE(kamiL): @sentry/minimal doesn't expose this method, as it's not env-agnostic, but we still want to test it here +Sentry.showReportDialog = (...args) => { + Sentry._callOnClient('showReportDialog', ...args) +} + +exports.Sentry = Sentry +exports.serverConfig = serverConfig +exports.clientConfig = clientConfig diff --git a/packages/next-plugin-sentry/package.json b/packages/next-plugin-sentry/package.json index 8cd297cc8c69e..a5d8f7926ba20 100644 --- a/packages/next-plugin-sentry/package.json +++ b/packages/next-plugin-sentry/package.json @@ -7,16 +7,15 @@ }, "nextjs": { "name": "Sentry", - "required-env": [ - "SENTRY_DSN", - "SENTRY_RELEASE" - ] + "required-env": [] }, "peerDependencies": { "next": "*" }, "dependencies": { - "@sentry/browser": "5.7.1", - "@sentry/node": "5.7.1" + "@sentry/integrations": "5.27.0", + "@sentry/node": "5.27.0", + "@sentry/browser": "5.27.0", + "@sentry/minimal": "5.27.0" } } diff --git a/packages/next-plugin-sentry/readme.md b/packages/next-plugin-sentry/readme.md index b1ab001e9a491..0744a51da4deb 100644 --- a/packages/next-plugin-sentry/readme.md +++ b/packages/next-plugin-sentry/readme.md @@ -1,3 +1,98 @@ -# Unstable @next/plugin-sentry +# Next.js + Sentry -This package is still very experimental and should not be used at this point +Capture unhandled exceptions with Sentry in your Next.js project. + +## Installation + +``` +npm install @next/plugin-sentry +``` + +or + +``` +yarn add @next/plugin-sentry +``` + +Provide necessary env variables through `.env` or available way + +``` +NEXT_PUBLIC_SENTRY_DSN + +// variables below are required only when using with @next/sentry-source-maps +NEXT_PUBLIC_SENTRY_RELEASE +SENTRY_PROJECT +SENTRY_ORG +SENTRY_AUTH_TOKEN +``` + +### Usage + +Create a next.config.js + +```js +// next.config.js +module.exports = { + experimental: { plugins: true }, +} +``` + +With only that, you'll get a complete error coverage for your application. +If you want to use Sentry SDK APIs, you can do so in both, server-side and client-side code with the same namespace from the plugin. + +```js +import { Sentry } from '@next/plugin-sentry' + +const MyComponent = () =>

Server Test 1

+ +export function getServerSideProps() { + if (!this.veryImportantValue) { + Sentry.withScope((scope) => { + scope.setTag('method', 'getServerSideProps') + Sentry.captureMessage('veryImportantValue is missing') + }) + } + + return {} +} + +export default MyComponent +``` + +### Configuration + +There are two ways to configure Sentry SDK. One through `next.config.js` which allows for the full configuration of the server-side code, and partial configuration of client-side code. And additional method for client-side code. + +```js +// next.config.js +module.exports = { + experimental: { plugins: true }, + // Sentry.init config for server-side code. Can accept any available config option. + serverRuntimeConfig: { + sentry: { + type: 'server', + }, + }, + // Sentry.init config for client-side code (and fallback for server-side) + // can accept only serializeable values. For more granular control see below. + publicRuntimeConfig: { + sentry: { + type: 'client', + }, + }, +} +``` + +If you need to pass config options for the client-side, that are non-serializable, for example `beforeSend` or `beforeBreadcrumb`: + +```js +// _app.js +import { clientConfig } from '@next/plugin-sentry' + +clientConfig.beforeSend = () => { + /* ... */ +} +clientConfig.beforeBreadcrumb = () => { + /* ... */ +} +``` diff --git a/packages/next-plugin-sentry/src/on-error-client.js b/packages/next-plugin-sentry/src/on-error-client.js index bd1dd69f2695a..6f58be8a75580 100644 --- a/packages/next-plugin-sentry/src/on-error-client.js +++ b/packages/next-plugin-sentry/src/on-error-client.js @@ -1,5 +1,12 @@ -import * as Sentry from '@sentry/browser' +import { withScope, captureException } from '@sentry/browser' -export default async function onErrorClient({ err }) { - Sentry.captureException(err) +export default async function onErrorClient({ err, errorInfo }) { + withScope((scope) => { + if (typeof errorInfo?.componentStack === 'string') { + scope.setContext('react', { + componentStack: errorInfo.componentStack.trim(), + }) + } + captureException(err) + }) } diff --git a/packages/next-plugin-sentry/src/on-error-server.js b/packages/next-plugin-sentry/src/on-error-server.js index 5ccc7e73dc58b..856372a76e335 100644 --- a/packages/next-plugin-sentry/src/on-error-server.js +++ b/packages/next-plugin-sentry/src/on-error-server.js @@ -1,5 +1,27 @@ -import * as Sentry from '@sentry/node' +import { captureException, flush, Handlers, withScope } from '@sentry/node' +import getConfig from 'next/config' -export default async function onErrorServer({ err }) { - Sentry.captureException(err) +const { parseRequest } = Handlers + +export default async function onErrorServer({ err, req }) { + const { serverRuntimeConfig = {}, publicRuntimeConfig = {} } = + getConfig() || {} + const sentryTimeout = + serverRuntimeConfig.sentryTimeout || + publicRuntimeConfig.sentryTimeout || + 2000 + + withScope((scope) => { + if (req) { + scope.addEventProcessor((event) => + parseRequest(event, req, { + // TODO(kamil): 'cookies' and 'query_string' use `dynamicRequire` which has a bug in SSR envs right now. + request: ['data', 'headers', 'method', 'url'], + }) + ) + } + captureException(err) + }) + + await flush(sentryTimeout) } diff --git a/packages/next-plugin-sentry/src/on-init-client.js b/packages/next-plugin-sentry/src/on-init-client.js index 59c4120210339..5cafe3ad725c9 100644 --- a/packages/next-plugin-sentry/src/on-init-client.js +++ b/packages/next-plugin-sentry/src/on-init-client.js @@ -1,10 +1,21 @@ -import * as Sentry from '@sentry/browser' +import { init } from '@sentry/browser' +import getConfig from 'next/config' + +import { getDsn, getRelease } from '../env' +import { clientConfig } from '../config' export default async function initClient() { - // by default `@sentry/browser` is configured with defaultIntegrations - // which capture uncaughtExceptions and unhandledRejections - Sentry.init({ - dsn: process.env.SENTRY_DSN, - release: process.env.SENTRY_RELEASE, + /** + * TODO(kamil): Unify SDK configuration options. + * RuntimeConfig cannot be used for callbacks and integrations as it supports only serializable data. + **/ + const { publicRuntimeConfig = {} } = getConfig() || {} + const runtimeConfig = publicRuntimeConfig.sentry || {} + + init({ + dsn: getDsn(), + ...(getRelease() && { release: getRelease() }), + ...runtimeConfig, + ...clientConfig, }) } diff --git a/packages/next-plugin-sentry/src/on-init-server.js b/packages/next-plugin-sentry/src/on-init-server.js index 4d6a5b72b8d8c..1de9fd5ebcb64 100644 --- a/packages/next-plugin-sentry/src/on-init-server.js +++ b/packages/next-plugin-sentry/src/on-init-server.js @@ -1,11 +1,39 @@ -import * as Sentry from '@sentry/node' +import { init } from '@sentry/node' +import { RewriteFrames } from '@sentry/integrations' +import getConfig from 'next/config' + +import { getDsn, getRelease } from '../env' +import { serverConfig } from '../config' export default async function initServer() { - // by default `@sentry/node` is configured with defaultIntegrations - // which capture uncaughtExceptions and unhandledRejections - // see here for more https://github.com/getsentry/sentry-javascript/blob/46a02209bafcbc1603c769476ba0a1eaa450759d/packages/node/src/sdk.ts#L22 - Sentry.init({ - dsn: process.env.SENTRY_DSN, - release: process.env.SENTRY_RELEASE, + /** + * TODO(kamil): Unify SDK configuration options. + * RuntimeConfig cannot be used for callbacks and integrations as it supports only serializable data. + **/ + const { serverRuntimeConfig = {}, publicRuntimeConfig = {} } = + getConfig() || {} + const runtimeConfig = + serverRuntimeConfig.sentry || publicRuntimeConfig.sentry || {} + + init({ + dsn: getDsn(), + ...(getRelease() && { release: getRelease() }), + ...runtimeConfig, + ...serverConfig, + integrations: [ + new RewriteFrames({ + iteratee: (frame) => { + try { + const [, path] = frame.filename.split('.next/') + if (path) { + frame.filename = `app:///_next/${path}` + } + } catch {} + return frame + }, + }), + ...(runtimeConfig.integrations || []), + ...(serverConfig.integrations || []), + ], }) } From 6fb4e276b8f59326927fef2d7b8b15fa5b6e2e61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Tue, 3 Nov 2020 11:13:04 +0100 Subject: [PATCH 3/3] Move onErrorMiddleware inside the logError method --- packages/next/next-server/server/next-server.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 966f688b6cec2..b0d14b0ca2973 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -280,7 +280,10 @@ export default class Server { return PHASE_PRODUCTION_SERVER } - public logError(err: Error): void { + public logError(err: Error, req: IncomingMessage, res: ServerResponse): void { + if (this.onErrorMiddleware) { + this.onErrorMiddleware({ err, req, res }) + } if (this.quiet) return console.error(err) } @@ -442,10 +445,7 @@ export default class Server { try { return await this.run(req, res, parsedUrl) } catch (err) { - if (this.onErrorMiddleware) { - this.onErrorMiddleware({ err, req, res }) - } - this.logError(err) + this.logError(err, req, res) res.statusCode = 500 res.end('Internal Server Error') } @@ -1593,10 +1593,7 @@ export default class Server { } } } catch (err) { - if (this.onErrorMiddleware) { - this.onErrorMiddleware({ err, req, res }) - } - this.logError(err) + this.logError(err, req, res) if (err && err.code === 'DECODE_FAILED') { res.statusCode = 400