Fix Sentry config on the server side (#377)

* Re-export Sentry from init (DX) and call init from error endpoint to see if it fixes the issue with flush() timeout * Improve sentry init documentation + rollback to timeout 2s + auto-init Sentry when using configureReq * Implement better error handling on _error page (enrich Sentry metadata from req object) * Fix 404 error handling and reporting to Sentry * Improve documentation about error handling * Upgrade Sentry to latest (minor)
UnlyEd · Jun 25, 2021 · 5ea63ac · 5ea63ac
1 parent 9a4e137
commit 5ea63ac
Show file tree

Hide file tree

Showing 10 changed files with 131 additions and 90 deletions.
diff --git a/package.json b/package.json
@@ -117,8 +117,8 @@
     "@fortawesome/free-regular-svg-icons": "5.15.3",
     "@fortawesome/free-solid-svg-icons": "5.15.3",
     "@fortawesome/react-fontawesome": "0.1.14",
-    "@sentry/browser": "6.3.6",
-    "@sentry/node": "6.3.6",
+    "@sentry/browser": "6.7.2",
+    "@sentry/node": "6.7.2",
     "@types/lodash.isequal": "4.5.5",
     "@unly/simple-logger": "1.0.0",
     "@unly/universal-language-detector": "2.0.3",

diff --git a/src/app/components/MultiversalAppBootstrap.tsx b/src/app/components/MultiversalAppBootstrap.tsx
@@ -301,6 +301,12 @@ const MultiversalAppBootstrap: React.FunctionComponent<Props> = (props): JSX.Ele
           Sentry.captureException(props.err);
         });
       }
+    } else {
+      // XXX Opinionated: Record an exception in Sentry for 404, if you don't want this then uncomment the below code
+      const err = new Error(`Page not found (404) for "${router?.asPath}"`);
+
+      logger.warn(err);
+      Sentry.captureException(err);
     }
 
     const i18nextInstance: i18n = i18nextLocize(lang, i18nTranslations); // Apply i18next configuration with Locize backend

diff --git a/src/app/isNextApiRequest.ts b/src/app/isNextApiRequest.ts
@@ -0,0 +1,14 @@
+import { IncomingMessage } from 'http';
+import { NextApiRequest } from 'next';
+
+/**
+ * TS type guard resolving whether "req" matches a "NextApiRequest" object.
+ *
+ * @param req
+ *
+ * @see https://www.typescripttutorial.net/typescript-tutorial/typescript-type-guards/
+ * @see https://www.logicbig.com/tutorials/misc/typescript/type-guards.html
+ */
+export const isNextApiRequest = (req: NextApiRequest | IncomingMessage): req is NextApiRequest => {
+  return (req as NextApiRequest).body !== undefined;
+};
diff --git a/src/modules/core/sentry/config.ts b/src/modules/core/sentry/config.ts
@@ -20,4 +20,4 @@ export const ALERT_TYPES = {
  * @see https://github.com/vercel/next.js/blob/canary/examples/with-sentry/pages/_error.js#L45
  * @see https://vercel.com/docs/platform/limits#streaming-responses
  */
-export const FLUSH_TIMEOUT = 5000;
+export const FLUSH_TIMEOUT = 2000;
diff --git a/src/modules/core/sentry/init.ts b/src/modules/core/sentry/init.ts
@@ -2,12 +2,23 @@ import * as Sentry from '@sentry/node';
 import { isBrowser } from '@unly/utils';
 
 /**
- * Initialize Sentry and export it.
+ * Initializes Sentry and exports it.
  *
- * Helper to avoid duplicating the init() call in every /pages/api file.
- * Also used in pages/_app for the client side, which automatically applies it for all frontend pages.
+ * Helper to avoid duplicating the Sentry initialization in:
+ * - The "/pages/api" files, for the server side.
+ * - The "pages/_app" file, for the client side, which in turns automatically applies it for all frontend pages.
  *
- * Doesn't initialise Sentry if SENTRY_DSN isn't defined
+ * Also configures the default scope, subsequent calls to "configureScope" will enrich the scope.
+ * Must only contain tags/contexts/extras that are universal (not server or browser specific).
+ *
+ * The Sentry scope will be enriched by:
+ * - BrowserPageBootstrap, for browser-specific metadata.
+ * - ServerPageBootstrap, for server-specific metadata.
+ * - API endpoints, for per-API additional metadata.
+ * - React components, for per-component additional metadata.
+ *
+ * Doesn't initialize Sentry if SENTRY_DSN isn't defined.
+ * Re-exports the Sentry object to make it simpler to consume by developers (DX).
  *
  * @see https://www.npmjs.com/package/@sentry/node
  */
@@ -19,13 +30,7 @@ if (process.env.SENTRY_DSN) {
     release: process.env.NEXT_PUBLIC_APP_VERSION_RELEASE,
   });
 
-  if (!process.env.SENTRY_DSN && process.env.NODE_ENV !== 'test') {
-    // eslint-disable-next-line no-console
-    console.error('Sentry DSN not defined');
-  }
-
-  // Scope configured by default, subsequent calls to "configureScope" will add additional data
-  Sentry.configureScope((scope) => { // See https://www.npmjs.com/package/@sentry/node
+  Sentry.configureScope((scope) => {
     scope.setTag('customerRef', process.env.NEXT_PUBLIC_CUSTOMER_REF);
     scope.setTag('appStage', process.env.NEXT_PUBLIC_APP_STAGE);
     scope.setTag('appName', process.env.NEXT_PUBLIC_APP_NAME);
@@ -40,4 +45,11 @@ if (process.env.SENTRY_DSN) {
     scope.setTag('memory', process.env.AWS_LAMBDA_FUNCTION_MEMORY_SIZE || null); // Optional - Available on production environment only
     scope.setTag('runtimeEngine', isBrowser() ? 'browser' : 'server');
   });
+} else {
+  if (process.env.NODE_ENV !== 'test') {
+    // eslint-disable-next-line no-console
+    console.error(`Sentry DSN not defined, events (exceptions, messages, etc.) won't be sent to Sentry.`);
+  }
 }
+
+export default Sentry;
diff --git a/src/modules/core/sentry/server.ts b/src/modules/core/sentry/server.ts
@@ -1,21 +1,27 @@
+import { isNextApiRequest } from '@/app/isNextApiRequest';
 import { convertRequestBodyToJSObject } from '@/modules/core/api/convertRequestBodyToJSObject';
 import { GenericObject } from '@/modules/core/data/types/GenericObject';
-import * as Sentry from '@sentry/node';
+import Sentry from '@/modules/core/sentry/init';
+import { IncomingMessage } from 'http'; // Automatically inits Sentry during import
 import map from 'lodash.map';
 import { NextApiRequest } from 'next';
 
 /**
- * Configure the Sentry scope by extracting useful tags and context from the given request.
+ * Configures the Sentry scope by extracting useful tags and context from the given request.
+ *
+ * XXX Because it imports Sentry from "@/modules/core/sentry/init", it automatically initializes Sentry as well
  *
  * @param req
  * @param tags
  * @param contexts
  * @see https://www.npmjs.com/package/@sentry/node
  */
-export const configureReq = (req: NextApiRequest, tags?: { [key: string]: string }, contexts?: { [key: string]: any }): void => {
+export const configureReq = (req: NextApiRequest | IncomingMessage, tags?: { [key: string]: string }, contexts?: { [key: string]: any }): void => {
   let parsedBody: GenericObject = {};
   try {
-    parsedBody = convertRequestBodyToJSObject(req);
+    if (isNextApiRequest(req)) {
+      parsedBody = convertRequestBodyToJSObject(req);
+    }
   } catch (e) {
     // eslint-disable-next-line no-console
     // console.error(e);
@@ -25,12 +31,15 @@ export const configureReq = (req: NextApiRequest, tags?: { [key: string]: string
     scope.setTag('host', req?.headers?.host);
     scope.setTag('url', req?.url);
     scope.setTag('method', req?.method);
-    scope.setExtra('query', req?.query);
-    scope.setExtra('body', req?.body);
-    scope.setExtra('cookies', req?.cookies);
     scope.setContext('headers', req?.headers);
     scope.setContext('parsedBody', parsedBody);
 
+    if (isNextApiRequest(req)) {
+      scope.setExtra('query', req?.query);
+      scope.setExtra('body', req?.body);
+      scope.setExtra('cookies', req?.cookies);
+    }
+
     map(tags, (value: string, tag: string) => {
       scope.setTag(tag, value);
     });

diff --git a/src/pages/[locale]/demo/built-in-utilities/errors-handling.tsx b/src/pages/[locale]/demo/built-in-utilities/errors-handling.tsx
@@ -77,7 +77,7 @@ const ErrorsHandlingPage: NextPage<Props> = (props): JSX.Element => {
         <h2>404 - Using CSR</h2>
 
         <Alert color={'info'}>
-          This page doesn't exist and should display a 404 page.
+          This page doesn't exist and should display a 404 page. The error will be reported to Sentry.
         </Alert>
 
         <p>
@@ -98,7 +98,7 @@ const ErrorsHandlingPage: NextPage<Props> = (props): JSX.Element => {
         <h2>404 - Using full page reload</h2>
 
         <Alert color={'info'}>
-          This page doesn't exist and should display a 404 page.
+          This page doesn't exist and should display a 404 page. The error will be reported to Sentry.
         </Alert>
 
         <p>
@@ -123,6 +123,7 @@ const ErrorsHandlingPage: NextPage<Props> = (props): JSX.Element => {
 
         <Alert color={'info'}>
           This page throws an error right from the Page component and should display a 500 page error without anything else (no footer/header).
+          The error will be reported to Sentry.
         </Alert>
 
         <Code
@@ -162,7 +163,7 @@ const ErrorsHandlingPage: NextPage<Props> = (props): JSX.Element => {
 
         <hr />
 
-        <h2>Interactive error (simulating User interaction)</h2>
+        <h2>Interactive errors (simulating User interaction)</h2>
 
         <Btn mode={'primary-outline'}>
           <I18nLink href={'/demo/built-in-utilities/interactive-error'}>Go to interactive error page</I18nLink><br />

diff --git a/src/pages/[locale]/demo/built-in-utilities/interactive-error.tsx b/src/pages/[locale]/demo/built-in-utilities/interactive-error.tsx
@@ -76,7 +76,7 @@ const InteractiveErrorPage: NextPage<Props> = (props): JSX.Element => {
         <Button
           onClick={(): void => {
             setIsClicked(true);
-            throw new Error('Page 500 error example');
+            throw new Error('Page 500 error example (handled)');
           }}
         >
           Will it crash the whole app?
@@ -99,7 +99,7 @@ const InteractiveErrorPage: NextPage<Props> = (props): JSX.Element => {
           <Button
             onClick={(): void => {
               setIsClicked(true);
-              throw new Error('Page 500 error example');
+              throw new Error('Page 500 error example (handled)');
             }}
           >
             Will it crash the whole app?

diff --git a/src/pages/_error.tsx b/src/pages/_error.tsx
@@ -86,15 +86,19 @@ const ErrorPage = (props: ErrorPageProps): JSX.Element => {
 };
 
 /**
- * TODO Doc - Is this only called when the error happens server-side?
- *  What's the point of getInitialProps when using SSG or hybrid apps?
+ * Might be called from the server and the client side.
+ *
+ * Won't be called for 404 errors (those are caught in MultiversalPageBootstrap).
+ *
+ * XXX Question: What's the point of getInitialProps when using SSG or hybrid apps? Is it being used? In what cases?
  *
  * @param props
  *
  * @see https://github.com/vercel/next.js/blob/canary/examples/with-sentry/pages/_error.js
  */
 ErrorPage.getInitialProps = async (props: NextPageContext): Promise<ErrorProps> => {
   const {
+    req,
     res,
     err,
     asPath,
@@ -115,13 +119,9 @@ ErrorPage.getInitialProps = async (props: NextPageContext): Promise<ErrorProps>
   if (res) {
     // Running on the server, the response object is available.
     //
-    // Next.js will pass an err on the server if a page's `getInitialProps`
-    // threw or returned a Promise that rejected
-
-    // XXX Opinionated: Record an exception in Sentry for 404, if you don't want this then uncomment the below code
-    // if (res.statusCode === 404) {
-    //   return { statusCode: 404, isReadyToRender: true };
-    // }
+    // Next.js will pass an err on the server if a page's `getInitialProps` threw or returned a Promise that rejected
+    const configureReq = (await import('@/modules/core/sentry/server')).configureReq;
+    configureReq(req);
 
     if (err) {
       Sentry.captureException(err);
@@ -134,7 +134,6 @@ ErrorPage.getInitialProps = async (props: NextPageContext): Promise<ErrorProps>
     // Running on the client (browser).
     //
     // Next.js will provide an err if:
-    //
     //  - a page's `getInitialProps` threw or returned a Promise that rejected
     //  - an exception was thrown somewhere in the React lifecycle (render,
     //    componentDidMount, etc) that was caught by Next.js's React Error

diff --git a/yarn.lock b/yarn.lock
@@ -2206,82 +2206,82 @@
   dependencies:
     any-observable "^0.3.0"
 
-"@sentry/browser@6.3.6":
-  version "6.3.6"
-  resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-6.3.6.tgz#bba07033efded6c844de88dcc47f99548a29afed"
-  integrity sha512-l4323jxuBOArki6Wf+EHes39IEyJ2Zj/CIUaTY7GWh7CntpfHQAfFmZWQw3Ozq+ka1u8lVp25RPhb4Wng3azNA==
-  dependencies:
-    "@sentry/core" "6.3.6"
-    "@sentry/types" "6.3.6"
-    "@sentry/utils" "6.3.6"
+"@sentry/browser@6.7.2":
+  version "6.7.2"
+  resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-6.7.2.tgz#cfbe060de5a9694617f175a6bde469e5e266792e"
+  integrity sha512-Lv0Ne1QcesyGAhVcQDfQa3hDPR/MhPSDTMg3xFi+LxqztchVc4w/ynzR0wCZFb8KIHpTj5SpJHfxpDhXYMtS9g==
+  dependencies:
+    "@sentry/core" "6.7.2"
+    "@sentry/types" "6.7.2"
+    "@sentry/utils" "6.7.2"
     tslib "^1.9.3"
 
-"@sentry/core@6.3.6":
-  version "6.3.6"
-  resolved "https://registry.yarnpkg.com/@sentry/core/-/core-6.3.6.tgz#e2ec6ae7e456e61f28000bab2d8ce85f58c59c66"
-  integrity sha512-w6BRizAqh7BaiM9oeKzO6aACXwRijUPacYaVLX/OfhqCSueF9uDxpMRT7+4D/eCeDVqgJYhBJ4Vsu2NSstkk4A==
+"@sentry/core@6.7.2":
+  version "6.7.2"
+  resolved "https://registry.yarnpkg.com/@sentry/core/-/core-6.7.2.tgz#1d294fac6e62744bce3b9dfbcd90b14e93620480"
+  integrity sha512-NTZqwN5nR94yrXmSfekoPs1mIFuKvf8esdIW/DadwSKWAdLJwQTJY9xK/8PQv+SEzd7wiitPAx+mCw2By1xiNQ==
   dependencies:
-    "@sentry/hub" "6.3.6"
-    "@sentry/minimal" "6.3.6"
-    "@sentry/types" "6.3.6"
-    "@sentry/utils" "6.3.6"
+    "@sentry/hub" "6.7.2"
+    "@sentry/minimal" "6.7.2"
+    "@sentry/types" "6.7.2"
+    "@sentry/utils" "6.7.2"
     tslib "^1.9.3"
 
-"@sentry/hub@6.3.6":
-  version "6.3.6"
-  resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-6.3.6.tgz#e7bc6960e30d8731e23c6e77f31af0bfb1d5af3c"
-  integrity sha512-foBZ3ilMnm9Gf9OolrAxYHK8jrA6IF72faDdJ3Al+1H27qcpnBaMdrdEp2/jzwu/dgmwuLmbBaMjEPXaGH/0JQ==
+"@sentry/hub@6.7.2":
+  version "6.7.2"
+  resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-6.7.2.tgz#31b250e74aa303877620dfa500aa89e4411e2dec"
+  integrity sha512-05qVW6ymChJsXag4+fYCQokW3AcABIgcqrVYZUBf6GMU/Gbz5SJqpV7y1+njwWvnPZydMncP9LaDVpMKbE7UYQ==
   dependencies:
-    "@sentry/types" "6.3.6"
-    "@sentry/utils" "6.3.6"
+    "@sentry/types" "6.7.2"
+    "@sentry/utils" "6.7.2"
     tslib "^1.9.3"
 
-"@sentry/minimal@6.3.6":
-  version "6.3.6"
-  resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-6.3.6.tgz#aebcebd2ee9007b0ec505b9fcefd10f10fc5d43d"
-  integrity sha512-uM2/dH0a6zfvI5f+vg+/mST+uTBdN6Jgpm585ipH84ckCYQwIIDRg6daqsen4S1sy/xgg1P1YyC3zdEC4G6b1Q==
+"@sentry/minimal@6.7.2":
+  version "6.7.2"
+  resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-6.7.2.tgz#9e6c0c587daea64a9042041694a4ad5d559d16cd"
+  integrity sha512-jkpwFv2GFHoVl5vnK+9/Q+Ea8eVdbJ3hn3/Dqq9MOLFnVK7ED6MhdHKLT79puGSFj+85OuhM5m2Q44mIhyS5mw==
   dependencies:
-    "@sentry/hub" "6.3.6"
-    "@sentry/types" "6.3.6"
+    "@sentry/hub" "6.7.2"
+    "@sentry/types" "6.7.2"
     tslib "^1.9.3"
 
-"@sentry/node@6.3.6":
-  version "6.3.6"
-  resolved "https://registry.yarnpkg.com/@sentry/node/-/node-6.3.6.tgz#e584500a10a9162d47fc8b55c9bf2ac3fec8d9f9"
-  integrity sha512-QVWakREgVUV/rocm4uMq+RkC0/g9d/z2BYic+2b0ZZMZD2aXF5RulrUQlAO2MzoXcO+bqpkXQsqdhMccqB4Zeg==
+"@sentry/node@6.7.2":
+  version "6.7.2"
+  resolved "https://registry.yarnpkg.com/@sentry/node/-/node-6.7.2.tgz#ef2b865af2c37d83966db7fbd031179aa8c82cc0"
+  integrity sha512-vfNTmxBbHthAKPDBo0gVk/aNHdgUfXLzmaK7FgWO7cISiI2soCfvKEIP61XqIFZru06teqcRuDsYlR4wSeyWpg==
   dependencies:
-    "@sentry/core" "6.3.6"
-    "@sentry/hub" "6.3.6"
-    "@sentry/tracing" "6.3.6"
-    "@sentry/types" "6.3.6"
-    "@sentry/utils" "6.3.6"
+    "@sentry/core" "6.7.2"
+    "@sentry/hub" "6.7.2"
+    "@sentry/tracing" "6.7.2"
+    "@sentry/types" "6.7.2"
+    "@sentry/utils" "6.7.2"
     cookie "^0.4.1"
     https-proxy-agent "^5.0.0"
     lru_map "^0.3.3"
     tslib "^1.9.3"
 
-"@sentry/tracing@6.3.6":
-  version "6.3.6"
-  resolved "https://registry.yarnpkg.com/@sentry/tracing/-/tracing-6.3.6.tgz#dc2aced01cdc401f97d6027113f6313503ee9c91"
-  integrity sha512-dfyYY2eESJGt5Qbigmfmb2U9ntqbwPhLNAOcjKaVg9WQRV5q2RkHCVctPoYk7TEAvfNeNRXCD8SnuFOZhttt8g==
+"@sentry/tracing@6.7.2":
+  version "6.7.2"
+  resolved "https://registry.yarnpkg.com/@sentry/tracing/-/tracing-6.7.2.tgz#78a6934837143ae5e200b49bd256bc8a917477bc"
+  integrity sha512-juKlI7FICKONWJFJxDxerj0A+8mNRhmtrdR+OXFqOkqSAy/QXlSFZcA/j//O19k2CfwK1BrvoMcQ/4gnffUOVg==
   dependencies:
-    "@sentry/hub" "6.3.6"
-    "@sentry/minimal" "6.3.6"
-    "@sentry/types" "6.3.6"
-    "@sentry/utils" "6.3.6"
+    "@sentry/hub" "6.7.2"
+    "@sentry/minimal" "6.7.2"
+    "@sentry/types" "6.7.2"
+    "@sentry/utils" "6.7.2"
     tslib "^1.9.3"
 
-"@sentry/types@6.3.6":
-  version "6.3.6"
-  resolved "https://registry.yarnpkg.com/@sentry/types/-/types-6.3.6.tgz#aa3687051af1dc04ebc4eaf7f9562872da67aa5c"
-  integrity sha512-93cFJdJkWyCfyZeWFARSU11qnoHVOS/R2h5WIsEf+jbQmkqG2C+TXVz/19s6nHVsfDrwpvYpwALPv4/nrxfU7g==
+"@sentry/types@6.7.2":
+  version "6.7.2"
+  resolved "https://registry.yarnpkg.com/@sentry/types/-/types-6.7.2.tgz#8108272c98ad7784ddf9ddda0b7bdc6880ed6e50"
+  integrity sha512-h21Go/PfstUN+ZV6SbwRSZVg9GXRJWdLfHoO5PSVb3TVEMckuxk8tAE57/u+UZDwX8wu+Xyon2TgsKpiWKxqUg==
 
-"@sentry/utils@6.3.6":
-  version "6.3.6"
-  resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-6.3.6.tgz#6f619a525f2a94fa6b160500f63f4bd5bd171055"
-  integrity sha512-HnYlDBf8Dq8MEv7AulH7B6R1D/2LAooVclGdjg48tSrr9g+31kmtj+SAj2WWVHP9+bp29BWaC7i5nkfKrOibWw==
+"@sentry/utils@6.7.2":
+  version "6.7.2"
+  resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-6.7.2.tgz#c7f957ebe16de3e701a0c5477ac2dba04e7b4b68"
+  integrity sha512-9COL7aaBbe61Hp5BlArtXZ1o/cxli1NGONLPrVT4fMyeQFmLonhUiy77NdsW19XnvhvaA+2IoV5dg3dnFiF/og==
   dependencies:
-    "@sentry/types" "6.3.6"
+    "@sentry/types" "6.7.2"
     tslib "^1.9.3"
 
 "@sindresorhus/is@^0.14.0":