fix: next.js few fixes (#3479)

* fix: Use buildId as fallback release

* fix: Make getRemainingTimeInMillis optional

* fix: add serverless package as a dependecy

* ref: Remove serverless package again

* fix: Also apply to api pages

* fix: Bind this for origErrorLogger

* ref: Changelog

Author: HazAT

CHANGELOG.md

@@ -4,6 +4,10 @@
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
+## 6.3.4
+
+- [nextjs] fix: API routes logging (#3479)
+
 ## 6.3.3
 
 - [nextjs] fix: User server types (#3471)

packages/nextjs/src/utils/config.ts

@@ -15,8 +15,7 @@ type WebpackExport = (config: WebpackConfig, options: WebpackOptions) => Webpack
 
 // 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 };
+type WebpackOptions = { dev: boolean; isServer: boolean; buildId: string };
 
 // For our purposes, the value for `entry` is either an object, or a function which returns such an object
 type EntryProperty = (() => Promise<EntryPropertyObject>) | EntryPropertyObject;
@@ -27,33 +26,10 @@ 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<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
-  // changed that, we need to check a few things along the way.
-
-  // The `entry` entry in a webpack config can be a string, array of strings, object, or function. By default, nextjs
-  // sets it to an async function which returns the promise of an object of string arrays. Because we don't know whether
-  // someone else has come along before us and changed that, we need to check a few things along the way. The one thing
-  // we know is that it won't have gotten *simpler* in form, so we only need to worry about the object and function
-  // options. See https://webpack.js.org/configuration/entry-context/#entry.
-
-  let newEntryProperty = origEntryProperty;
-
-  if (typeof origEntryProperty === 'function') {
-    newEntryProperty = await origEntryProperty();
-  }
-
-  newEntryProperty = newEntryProperty as EntryPropertyObject;
-
-  // according to vercel, we only need to inject Sentry in one spot for server and one spot for client, and because
-  // those are used as bases, it will apply everywhere
-  const injectionPoint = isServer ? 'pages/_document' : 'main';
-  const injectee = isServer ? './sentry.server.config.js' : './sentry.client.config.js';
-
+/** Add a file (`injectee`) to a given element (`injectionPoint`) of the `entry` property */
+const _injectFile = (entryProperty: EntryPropertyObject, injectionPoint: string, injectee: string): void => {
   // can be a string, array of strings, or object whose `import` property is one of those two
-  let injectedInto = newEntryProperty[injectionPoint];
-
+  let injectedInto = entryProperty[injectionPoint];
   // whatever the format, add in the sentry file
   injectedInto =
     typeof injectedInto === 'string'
@@ -73,15 +49,42 @@ const injectSentry = async (origEntryProperty: EntryProperty, isServer: boolean)
               : // array case for inner property
                 [injectee, ...injectedInto.import],
         };
+  entryProperty[injectionPoint] = injectedInto;
+};
 
-  newEntryProperty[injectionPoint] = injectedInto;
-
+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
+  // changed that, we need to check a few things along the way.
+  // The `entry` entry in a webpack config can be a string, array of strings, object, or function. By default, nextjs
+  // sets it to an async function which returns the promise of an object of string arrays. Because we don't know whether
+  // someone else has come along before us and changed that, we need to check a few things along the way. The one thing
+  // we know is that it won't have gotten *simpler* in form, so we only need to worry about the object and function
+  // options. See https://webpack.js.org/configuration/entry-context/#entry.
+  let newEntryProperty = origEntryProperty;
+  if (typeof origEntryProperty === 'function') {
+    newEntryProperty = await origEntryProperty();
+  }
+  newEntryProperty = newEntryProperty as EntryPropertyObject;
+  // On the server, we need to inject the SDK into both into the base page (`_document`) and into individual API routes
+  // (which have no common base).
+  if (isServer) {
+    Object.keys(newEntryProperty).forEach(key => {
+      if (key === 'pages/_document' || key.includes('pages/api')) {
+        // for some reason, because we're now in a function, we have to cast again
+        _injectFile(newEntryProperty as EntryPropertyObject, key, './sentry.server.config.js');
+      }
+    });
+  }
+  // On the client, it's sufficient to inject it into the `main` JS code, which is included in every browser page.
+  else {
+    _injectFile(newEntryProperty, 'main', './sentry.client.config.js');
+  }
   // 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'];
   }
-
   return newEntryProperty;
 };
 
@@ -97,7 +100,6 @@ export function withSentryConfig(
   providedWebpackPluginOptions: Partial<SentryCliPluginOptions> = {},
 ): NextConfigExports {
   const defaultWebpackPluginOptions = {
-    release: getSentryRelease(),
     url: process.env.SENTRY_URL,
     org: process.env.SENTRY_ORG,
     project: process.env.SENTRY_PROJECT,
@@ -144,6 +146,7 @@ export function withSentryConfig(
       // TODO it's not clear how to do this better, but there *must* be a better way
       new ((SentryWebpackPlugin as unknown) as typeof defaultWebpackPlugin)({
         dryRun: options.dev,
+        release: getSentryRelease(options.buildId),
         ...defaultWebpackPluginOptions,
         ...providedWebpackPluginOptions,
       }),

packages/nextjs/src/utils/instrumentServer.ts

@@ -86,6 +86,8 @@ function makeWrappedErrorLogger(origErrorLogger: ErrorLogger): WrappedErrorLogge
   return (err: Error): void => {
     // TODO add context data here
     Sentry.captureException(err);
-    return origErrorLogger(err);
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
+    return origErrorLogger.bind(this, err);
   };
 }

packages/node/src/sdk.ts

@@ -151,7 +151,7 @@ export async function close(timeout?: number): Promise<boolean> {
 /**
  * Returns a release dynamically from environment variables.
  */
-export function getSentryRelease(): string | undefined {
+export function getSentryRelease(fallback?: string): string | undefined {
   // Always read first as Sentry takes this as precedence
   if (process.env.SENTRY_RELEASE) {
     return process.env.SENTRY_RELEASE;
@@ -176,6 +176,7 @@ export function getSentryRelease(): string | undefined {
     // Zeit (now known as Vercel)
     process.env.ZEIT_GITHUB_COMMIT_SHA ||
     process.env.ZEIT_GITLAB_COMMIT_SHA ||
-    process.env.ZEIT_BITBUCKET_COMMIT_SHA
+    process.env.ZEIT_BITBUCKET_COMMIT_SHA ||
+    fallback
   );
 }

packages/serverless/src/awslambda.ts

@@ -131,6 +131,15 @@ export function tryPatchHandler(taskRoot: string, handlerPath: string): void {
   (mod as HandlerModule)[functionName!] = wrapHandler(obj as Handler);
 }
 
+/**
+ * Tries to invoke context.getRemainingTimeInMillis if not available returns 0
+ * Some environments use AWS lambda but don't support this function
+ * @param context
+ */
+function tryGetRemainingTimeInMillis(context: Context): number {
+  return typeof context.getRemainingTimeInMillis === 'function' ? context.getRemainingTimeInMillis() : 0;
+}
+
 /**
  * Adds additional information from the environment and AWS Context to the Sentry Scope.
  *
@@ -155,7 +164,7 @@ function enhanceScopeWithEnvironmentData(scope: Scope, context: Context, startTi
     function_version: context.functionVersion,
     invoked_function_arn: context.invokedFunctionArn,
     execution_duration_in_millis: performance.now() - startTime,
-    remaining_time_in_millis: context.getRemainingTimeInMillis(),
+    remaining_time_in_millis: tryGetRemainingTimeInMillis(context),
     'sys.argv': process.argv,
   });
 
@@ -221,7 +230,7 @@ export function wrapHandler<TEvent, TResult>(
     context.callbackWaitsForEmptyEventLoop = options.callbackWaitsForEmptyEventLoop;
 
     // In seconds. You cannot go any more granular than this in AWS Lambda.
-    const configuredTimeout = Math.ceil(context.getRemainingTimeInMillis() / 1000);
+    const configuredTimeout = Math.ceil(tryGetRemainingTimeInMillis(context) / 1000);
     const configuredTimeoutMinutes = Math.floor(configuredTimeout / 60);
     const configuredTimeoutSeconds = configuredTimeout % 60;
 
@@ -233,7 +242,7 @@ export function wrapHandler<TEvent, TResult>(
     // When `callbackWaitsForEmptyEventLoop` is set to false, which it should when using `captureTimeoutWarning`,
     // we don't have a guarantee that this message will be delivered. Because of that, we don't flush it.
     if (options.captureTimeoutWarning) {
-      const timeoutWarningDelay = context.getRemainingTimeInMillis() - options.timeoutWarningLimit;
+      const timeoutWarningDelay = tryGetRemainingTimeInMillis(context) - options.timeoutWarningLimit;
 
       timeoutWarningTimer = setTimeout(() => {
         withScope(scope => {