From 56f5330b89f777a582468846c7fc115c7f1b0c69 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 29 Apr 2021 00:06:55 +0200 Subject: [PATCH 1/7] fix: Use buildId as fallback release --- packages/nextjs/src/utils/config.ts | 4 ++-- packages/node/src/sdk.ts | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/utils/config.ts b/packages/nextjs/src/utils/config.ts index 909c2c2e6ef1..962bdbf50104 100644 --- a/packages/nextjs/src/utils/config.ts +++ b/packages/nextjs/src/utils/config.ts @@ -16,7 +16,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 }; -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; @@ -97,7 +97,6 @@ export function withSentryConfig( providedWebpackPluginOptions: Partial = {}, ): NextConfigExports { const defaultWebpackPluginOptions = { - release: getSentryRelease(), url: process.env.SENTRY_URL, org: process.env.SENTRY_ORG, project: process.env.SENTRY_PROJECT, @@ -148,6 +147,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, }), diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 0a3138cacca8..315141922a5e 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -151,7 +151,7 @@ export async function close(timeout?: number): Promise { /** * 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 ); } From 7ffc22a991e19b9f88f02ddb55a1ee7b36c6bfde Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 29 Apr 2021 00:07:21 +0200 Subject: [PATCH 2/7] fix: Make getRemainingTimeInMillis optional --- packages/serverless/src/awslambda.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index dda99c704ae4..c7e8ad450808 100644 --- a/packages/serverless/src/awslambda.ts +++ b/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( 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( // 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 => { From e9687a67efdebb4dc392c6711169da37c7b028f2 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 29 Apr 2021 00:28:02 +0200 Subject: [PATCH 3/7] fix: add serverless package as a dependecy --- packages/nextjs/package.json | 1 + packages/nextjs/src/index.server.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index a2b48fd45f99..616fd46988b2 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -21,6 +21,7 @@ "@sentry/integrations": "6.3.3", "@sentry/node": "6.3.3", "@sentry/react": "6.3.3", + "@sentry/serverless": "6.3.3", "@sentry/utils": "6.3.3", "@sentry/webpack-plugin": "1.15.0" }, diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index b781dd8d8a93..ecf0979f9675 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -1,4 +1,5 @@ import { configureScope, init as nodeInit } from '@sentry/node'; +import { AWSLambda } from '@sentry/serverless'; import { MetadataBuilder } from './utils/metadataBuilder'; import { NextjsOptions } from './utils/nextjsOptions'; @@ -26,5 +27,5 @@ export function init(options: NextjsOptions): void { }); } +export const withSentry = AWSLambda.wrapHandler; export { withSentryConfig } from './utils/config'; -export { withSentry } from './utils/handlers'; From cb61a2cad55f51c09578dc8387201b6b30005f15 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 29 Apr 2021 02:00:32 +0200 Subject: [PATCH 4/7] ref: Remove serverless package again --- packages/nextjs/package.json | 1 - packages/nextjs/src/index.server.ts | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 616fd46988b2..a2b48fd45f99 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -21,7 +21,6 @@ "@sentry/integrations": "6.3.3", "@sentry/node": "6.3.3", "@sentry/react": "6.3.3", - "@sentry/serverless": "6.3.3", "@sentry/utils": "6.3.3", "@sentry/webpack-plugin": "1.15.0" }, diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index ecf0979f9675..b781dd8d8a93 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -1,5 +1,4 @@ import { configureScope, init as nodeInit } from '@sentry/node'; -import { AWSLambda } from '@sentry/serverless'; import { MetadataBuilder } from './utils/metadataBuilder'; import { NextjsOptions } from './utils/nextjsOptions'; @@ -27,5 +26,5 @@ export function init(options: NextjsOptions): void { }); } -export const withSentry = AWSLambda.wrapHandler; export { withSentryConfig } from './utils/config'; +export { withSentry } from './utils/handlers'; From 44c06e5d22fc74b5aa8dee67edeb1d7d5e743bdc Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 29 Apr 2021 02:35:00 +0200 Subject: [PATCH 5/7] fix: Also apply to api pages --- packages/nextjs/src/utils/config.ts | 62 +++++++++++++++-------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/packages/nextjs/src/utils/config.ts b/packages/nextjs/src/utils/config.ts index 0fd988fafe08..7446a4829c11 100644 --- a/packages/nextjs/src/utils/config.ts +++ b/packages/nextjs/src/utils/config.ts @@ -26,33 +26,10 @@ type EntryProperty = (() => Promise) | EntryPropertyObject; type EntryPropertyObject = PlainObject | EntryPointObject>; type EntryPointObject = { import: string | Array }; -const injectSentry = async (origEntryProperty: EntryProperty, isServer: boolean): Promise => { - // Out of the box, nextjs uses the `() => Promise)` 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' @@ -72,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 => { + // Out of the box, nextjs uses the `() => Promise)` 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; }; From f8902c1c924cc978c2e3922eeaf7cc726ab48b83 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 29 Apr 2021 10:03:03 +0200 Subject: [PATCH 6/7] fix: Bind this for origErrorLogger --- packages/nextjs/src/utils/instrumentServer.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index 230fcf0efb2c..dabc48f52ea6 100644 --- a/packages/nextjs/src/utils/instrumentServer.ts +++ b/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); }; } From d55462e29a3cc73e488f3f46c608adf02a0323f0 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 29 Apr 2021 10:14:57 +0200 Subject: [PATCH 7/7] ref: Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2307ca73634..893abce0f7e1 100644 --- a/CHANGELOG.md +++ b/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)