Skip to content

Commit

Permalink
fix(nextjs): Add missing changes from #3462 (#3476)
Browse files Browse the repository at this point in the history
  • Loading branch information
lobsterkatie authored Apr 29, 2021
1 parent b7382a2 commit 3ab107b
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 10 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
"packages/integrations",
"packages/minimal",
"packages/nextjs",
"packages/next-plugin-sentry",
"packages/node",
"packages/react",
"packages/serverless",
Expand Down
4 changes: 4 additions & 0 deletions packages/nextjs/src/index.server.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { configureScope, init as nodeInit } from '@sentry/node';

import { instrumentServer } from './utils/instrumentServer';
import { MetadataBuilder } from './utils/metadataBuilder';
import { NextjsOptions } from './utils/nextjsOptions';
import { defaultRewriteFrames, getFinalServerIntegrations } from './utils/serverIntegrations';
Expand Down Expand Up @@ -28,3 +29,6 @@ export function init(options: NextjsOptions): void {

export { withSentryConfig } from './utils/config';
export { withSentry } from './utils/handlers';

// TODO capture project root (which this returns) for RewriteFrames?
instrumentServer();
14 changes: 5 additions & 9 deletions packages/nextjs/src/utils/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ type PlainObject<T = any> = { [key: string]: T };

// Man are these types hard to name well. "Entry" = an item in some collection of items, but in our case, one of the
// things we're worried about here is property (entry) in an object called... entry. So henceforth, the specific
// proptery we're modifying is going to be known as an EntryProperty, or EP for short.
// property we're modifying is going to be known as an EntryProperty.

// The function which is ultimately going to be exported from `next.config.js` under the name `webpack`
type WebpackExport = (config: WebpackConfig, options: WebpackOptions) => WebpackConfig;
// type WebpackExport = (config: WebpackConfig, options: WebpackOptions) => Promise<WebpackConfig>;

// The two arguments passed to the exported `webpack` function, as well as the thing it returns
type WebpackConfig = { devtool: string; plugins: PlainObject[]; entry: EntryProperty };
// TODO use real webpack types
type WebpackOptions = { dev: boolean; isServer: boolean };

// For our purposes, the value for `entry` is either an object, or a function which returns such an object
Expand All @@ -27,7 +27,6 @@ type EntryProperty = (() => Promise<EntryPropertyObject>) | EntryPropertyObject;
type EntryPropertyObject = PlainObject<string | Array<string> | EntryPointObject>;
type EntryPointObject = { import: string | Array<string> };

// const injectSentry = async (origEntryProperty: EntryProperty, isServer: boolean): Promise<EntryPropertyObject> => {
const injectSentry = async (origEntryProperty: EntryProperty, isServer: boolean): Promise<EntryProperty> => {
// Out of the box, nextjs uses the `() => Promise<EntryPropertyObject>)` flavor of EntryProperty, where the returned
// object has string arrays for values. But because we don't know whether someone else has come along before us and
Expand Down Expand Up @@ -77,7 +76,8 @@ const injectSentry = async (origEntryProperty: EntryProperty, isServer: boolean)

newEntryProperty[injectionPoint] = injectedInto;

// TODO: hack made necessary because promises are currently kicking my butt
// TODO: hack made necessary because the async-ness of this function turns our object back into a promise, meaning the
// internal `next` code which should do this doesn't
if ('main.js' in newEntryProperty) {
delete newEntryProperty['main.js'];
}
Expand Down Expand Up @@ -115,19 +115,17 @@ export function withSentryConfig(
.map(key => key in Object.keys(providedWebpackPluginOptions));
if (webpackPluginOptionOverrides.length > 0) {
logger.warn(
'[next-plugin-sentry] You are overriding the following automatically-set SentryWebpackPlugin config options:\n' +
'[Sentry] You are overriding the following automatically-set SentryWebpackPlugin config options:\n' +
`\t${webpackPluginOptionOverrides.toString()},\n` +
"which has the possibility of breaking source map upload and application. This is only a good idea if you know what you're doing.",
);
}

// const newWebpackExport = async (config: WebpackConfig, options: WebpackOptions): Promise<WebpackConfig> => {
const newWebpackExport = (config: WebpackConfig, options: WebpackOptions): WebpackConfig => {
let newConfig = config;

if (typeof providedExports.webpack === 'function') {
newConfig = providedExports.webpack(config, options);
// newConfig = await providedExports.webpack(config, options);
}

// Ensure quality source maps in production. (Source maps aren't uploaded in dev, and besides, Next doesn't let you
Expand All @@ -140,8 +138,6 @@ export function withSentryConfig(
// Inject user config files (`sentry.client.confg.js` and `sentry.server.config.js`), which is where `Sentry.init()`
// is called. By adding them here, we ensure that they're bundled by webpack as part of both server code and client code.
newConfig.entry = (injectSentry(newConfig.entry, options.isServer) as unknown) as EntryProperty;
// newConfig.entry = await injectSentry(newConfig.entry, options.isServer);
// newConfig.entry = async () => injectSentry(newConfig.entry, options.isServer);

// Add the Sentry plugin, which uploads source maps to Sentry when not in dev
newConfig.plugins.push(
Expand Down
91 changes: 91 additions & 0 deletions packages/nextjs/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { fill } from '@sentry/utils';
import * as http from 'http';
import { default as createNextServer } from 'next';
import * as url from 'url';

import * as Sentry from '../index.server';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type PlainObject<T = any> = { [key: string]: T };

interface NextServer {
server: Server;
}

interface Server {
dir: string;
}

type HandlerGetter = () => Promise<ReqHandler>;
type ReqHandler = (
req: http.IncomingMessage,
res: http.ServerResponse,
parsedUrl?: url.UrlWithParsedQuery,
) => Promise<void>;
type ErrorLogger = (err: Error) => void;

// these aliases are purely to make the function signatures more easily understandable
type WrappedHandlerGetter = HandlerGetter;
type WrappedErrorLogger = ErrorLogger;

// TODO is it necessary for this to be an object?
const closure: PlainObject = {};

/**
* Do the monkeypatching and wrapping necessary to catch errors in page routes. Along the way, as a bonus, grab (and
* return) the path of the project root, for use in `RewriteFrames`.
*
* @returns The absolute path of the project root directory
*
*/
export function instrumentServer(): string {
const nextServerPrototype = Object.getPrototypeOf(createNextServer({}));

// wrap this getter because it runs before the request handler runs, which gives us a chance to wrap the logger before
// it's called for the first time
fill(nextServerPrototype, 'getServerRequestHandler', makeWrappedHandlerGetter);

return closure.projectRootDir;
}

/**
* Create a wrapped version of Nextjs's `NextServer.getServerRequestHandler` method, as a way to access the running
* `Server` instance and monkeypatch its prototype.
*
* @param origHandlerGetter Nextjs's `NextServer.getServerRequestHandler` method
* @returns A wrapped version of the same method, to monkeypatch in at server startup
*/
function makeWrappedHandlerGetter(origHandlerGetter: HandlerGetter): WrappedHandlerGetter {
// We wrap this purely in order to be able to grab data and do further monkeypatching the first time it runs.
// Otherwise, it's just a pass-through to the original method.
const wrappedHandlerGetter = async function(this: NextServer): Promise<ReqHandler> {
if (!closure.wrappingComplete) {
closure.projectRootDir = this.server.dir;

const serverPrototype = Object.getPrototypeOf(this.server);

// wrap the logger so we can capture errors in page-level functions like `getServerSideProps`
fill(serverPrototype, 'logError', makeWrappedErrorLogger);

closure.wrappingComplete = true;
}

return origHandlerGetter.call(this);
};

return wrappedHandlerGetter;
}

/**
* Wrap the error logger used by the server to capture exceptions which arise from functions like `getServerSideProps`.
*
* @param origErrorLogger The original logger from the `Server` class
* @returns A wrapped version of that logger
*/
function makeWrappedErrorLogger(origErrorLogger: ErrorLogger): WrappedErrorLogger {
return (err: Error): void => {
// TODO add context data here
Sentry.captureException(err);
return origErrorLogger(err);
};
}

0 comments on commit 3ab107b

Please sign in to comment.