Skip to content
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

feat(nextjs): Instrument SSR page components #9346

Merged
merged 5 commits into from
Oct 25, 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import React from 'react';

export default class Page extends React.Component {
render() {
throw new Error('Pages SSR Error Class');
return <div>Hello world!</div>;
}
}

export function getServerSideProps() {
return {
props: {
foo: 'bar',
},
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export default function Page() {
throw new Error('Pages SSR Error FC');
return <div>Hello world!</div>;
}

export function getServerSideProps() {
return {
props: {
foo: 'bar',
},
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { test, expect } from '@playwright/test';
import { waitForError } from '../event-proxy-server';

test('Will capture error for SSR rendering error (Class Component)', async ({ page }) => {
const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => {
return errorEvent?.exception?.values?.[0]?.value === 'Pages SSR Error Class';
});

await page.goto('/pages-router/ssr-error-class');

const errorEvent = await errorEventPromise;
expect(errorEvent).toBeDefined();
});

test('Will capture error for SSR rendering error (Functional Component)', async ({ page }) => {
const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => {
return errorEvent?.exception?.values?.[0]?.value === 'Pages SSR Error FC';
});

await page.goto('/pages-router/ssr-error-fc');

const errorEvent = await errorEventPromise;
expect(errorEvent).toBeDefined();
});
2 changes: 2 additions & 0 deletions packages/nextjs/src/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,5 @@ export { wrapRouteHandlerWithSentry } from './wrapRouteHandlerWithSentry';
export { wrapApiHandlerWithSentryVercelCrons } from './wrapApiHandlerWithSentryVercelCrons';

export { wrapMiddlewareWithSentry } from './wrapMiddlewareWithSentry';

export { wrapPageComponentWithSentry } from './wrapPageComponentWithSentry';
66 changes: 66 additions & 0 deletions packages/nextjs/src/common/wrapPageComponentWithSentry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { captureException } from '@sentry/core';
import { addExceptionMechanism } from '@sentry/utils';

interface FunctionComponent {
(...args: unknown[]): unknown;
}

interface ClassComponent {
new (...args: unknown[]): {
render(...args: unknown[]): unknown;
};
}

function isReactClassComponent(target: unknown): target is ClassComponent {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
return typeof target === 'function' && target?.prototype?.isReactComponent;
Copy link
Member

Choose a reason for hiding this comment

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

LETS GOO

}

/**
* Wraps a page component with Sentry error instrumentation.
*/
export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | ClassComponent): unknown {
if (isReactClassComponent(pageComponent)) {
return class SentryWrappedPageComponent extends pageComponent {
public render(...args: unknown[]): unknown {
Copy link
Member

Choose a reason for hiding this comment

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

l: I wonder if we can use suspense + error boundary to catch this - it's what is recommended with reactjs/rfcs#215

Copy link
Member Author

Choose a reason for hiding this comment

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

SSR error boundaries are effectively only needed when you want to display a fallback in case of an error. I don't think we need this and I don't think this is possible in non React Server Component worlds but there's an excellent article on this: https://gal.hagever.com/posts/react-error-boundaries-and-ssr

try {
return super.render(...args);
} catch (e) {
captureException(e, scope => {
scope.addEventProcessor(event => {
addExceptionMechanism(event, {
handled: false,
});
return event;
});

return scope;
});
throw e;
}
}
};
} else if (typeof pageComponent === 'function') {
return new Proxy(pageComponent, {
apply(target, thisArg, argArray) {
try {
return target.apply(thisArg, argArray);
} catch (e) {
captureException(e, scope => {
scope.addEventProcessor(event => {
addExceptionMechanism(event, {
handled: false,
});
return event;
});

return scope;
});
throw e;
}
},
});
} else {
return pageComponent;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const getServerSideProps =
? Sentry.wrapGetServerSidePropsWithSentry(origGetServerSideProps, '__ROUTE__')
: undefined;

export default pageComponent;
export default pageComponent ? Sentry.wrapPageComponentWithSentry(pageComponent as unknown) : pageComponent;

// Re-export anything exported by the page module we're wrapping. When processing this code, Rollup is smart enough to
// not include anything whose name matchs something we've explicitly exported above.
Expand Down
5 changes: 5 additions & 0 deletions packages/nextjs/src/index.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,8 @@ export declare function wrapApiHandlerWithSentryVercelCrons<F extends (...args:
WrappingTarget: F,
vercelCronsConfig: VercelCronsConfig,
): F;

/**
* Wraps a page component with Sentry error instrumentation.
*/
export declare function wrapPageComponentWithSentry<C>(WrappingTarget: C): C;
Loading