fix(nextjs): Add missing changes from #3462 (#3476)

Author: lobsterkatie

package.json

@@ -30,7 +30,6 @@
     "packages/integrations",
     "packages/minimal",
     "packages/nextjs",
-    "packages/next-plugin-sentry",
     "packages/node",
     "packages/react",
     "packages/serverless",

packages/nextjs/src/index.server.ts

@@ -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';
@@ -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();

packages/nextjs/src/utils/config.ts

@@ -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
@@ -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
@@ -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'];
   }
@@ -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
@@ -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(

packages/nextjs/src/utils/instrumentServer.ts

@@ -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);
+  };
+}