fix(remix): Instrument existing root ErrorBoundary (#472)

getsentry · Sep 29, 2023 · d38a344 · d38a344
1 parent 33e337a
commit d38a344
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 - fix(remix): Use captureRemixServerException inside handleError (#466)
 - fix(remix): Fix `request` arg in `handleError` template (#469)
 - fix(remix): Update documentation links to the new routes (#470)
+- fix(remix): Instrument existing root `ErrorBoundary` (#472)
 
 ## 3.14.1
 

diff --git a/src/remix/codemods/root-common.ts b/src/remix/codemods/root-common.ts
@@ -0,0 +1,63 @@
+/* eslint-disable @typescript-eslint/no-unsafe-assignment */
+/* eslint-disable @typescript-eslint/no-unsafe-member-access */
+/* eslint-disable @typescript-eslint/no-unsafe-argument */
+
+import * as recast from 'recast';
+// @ts-expect-error - clack is ESM and TS complains about that. It works though
+import clack from '@clack/prompts';
+import chalk from 'chalk';
+
+// @ts-expect-error - magicast is ESM and TS complains about that. It works though
+import { builders, ProxifiedModule, generateCode } from 'magicast';
+
+export function wrapAppWithSentry(
+  rootRouteAst: ProxifiedModule,
+  rootFileName: string,
+) {
+  rootRouteAst.imports.$add({
+    from: '@sentry/remix',
+    imported: 'withSentry',
+    local: 'withSentry',
+  });
+
+  recast.visit(rootRouteAst.$ast, {
+    visitExportDefaultDeclaration(path) {
+      if (path.value.declaration.type === 'FunctionDeclaration') {
+        // Move the function declaration just before the default export
+        path.insertBefore(path.value.declaration);
+
+        // Get the name of the function to be wrapped
+        const functionName: string = path.value.declaration.id.name as string;
+
+        // Create the wrapped function call
+        const functionCall = recast.types.builders.callExpression(
+          recast.types.builders.identifier('withSentry'),
+          [recast.types.builders.identifier(functionName)],
+        );
+
+        // Replace the default export with the wrapped function call
+        path.value.declaration = functionCall;
+      } else if (path.value.declaration.type === 'Identifier') {
+        const rootRouteExport = rootRouteAst.exports.default;
+
+        const expressionToWrap = generateCode(rootRouteExport.$ast).code;
+
+        rootRouteAst.exports.default = builders.raw(
+          `withSentry(${expressionToWrap})`,
+        );
+      } else {
+        clack.log.warn(
+          chalk.yellow(
+            `Couldn't instrument ${chalk.bold(
+              rootFileName,
+            )} automatically. Wrap your default export with: ${chalk.dim(
+              'withSentry()',
+            )}\n`,
+          ),
+        );
+      }
+
+      this.traverse(path);
+    },
+  });
+}
diff --git a/src/remix/codemods/root-v1.ts b/src/remix/codemods/root-v1.ts
@@ -1,14 +1,14 @@
 /* eslint-disable @typescript-eslint/no-unsafe-assignment */
 
-import * as recast from 'recast';
 import * as path from 'path';
 
 // @ts-expect-error - clack is ESM and TS complains about that. It works though
 import clack from '@clack/prompts';
 import chalk from 'chalk';
 
 // @ts-expect-error - magicast is ESM and TS complains about that. It works though
-import { builders, generateCode, loadFile, writeFile } from 'magicast';
+import { loadFile, writeFile } from 'magicast';
+import { wrapAppWithSentry } from './root-common';
 
 export async function instrumentRootRouteV1(
   rootFileName: string,
@@ -18,57 +18,7 @@ export async function instrumentRootRouteV1(
       path.join(process.cwd(), 'app', rootFileName),
     );
 
-    rootRouteAst.imports.$add({
-      from: '@sentry/remix',
-      imported: 'withSentry',
-      local: 'withSentry',
-    });
-
-    recast.visit(rootRouteAst.$ast, {
-      visitExportDefaultDeclaration(path) {
-        /* eslint-disable @typescript-eslint/no-unsafe-member-access */
-        if (path.value.declaration.type === 'FunctionDeclaration') {
-          // Move the function declaration just before the default export
-          path.insertBefore(path.value.declaration);
-
-          // Get the name of the function to be wrapped
-          const functionName: string = path.value.declaration.id.name as string;
-
-          // Create the wrapped function call
-          const functionCall = recast.types.builders.callExpression(
-            recast.types.builders.identifier('withSentry'),
-            [recast.types.builders.identifier(functionName)],
-          );
-
-          // Replace the default export with the wrapped function call
-          path.value.declaration = functionCall;
-        } else if (path.value.declaration.type === 'Identifier') {
-          const rootRouteExport = rootRouteAst.exports.default;
-
-          const expressionToWrap = generateCode(
-            // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
-            rootRouteExport.$ast,
-          ).code;
-
-          rootRouteAst.exports.default = builders.raw(
-            `withSentry(${expressionToWrap})`,
-          );
-        } else {
-          clack.log.warn(
-            chalk.yellow(
-              `Couldn't instrument ${chalk.bold(
-                rootFileName,
-              )} automatically. Wrap your default export with: ${chalk.dim(
-                'withSentry()',
-              )}\n`,
-            ),
-          );
-        }
-
-        this.traverse(path);
-        /* eslint-enable @typescript-eslint/no-unsafe-member-access */
-      },
-    });
+    wrapAppWithSentry(rootRouteAst, rootFileName);
 
     await writeFile(
       rootRouteAst.$ast,

diff --git a/src/remix/codemods/root-v2.ts b/src/remix/codemods/root-v2.ts
@@ -1,4 +1,7 @@
+/* eslint-disable @typescript-eslint/no-unsafe-member-access */
 /* eslint-disable @typescript-eslint/no-unsafe-assignment */
+/* eslint-disable @typescript-eslint/no-unsafe-call */
+/* eslint-disable @typescript-eslint/no-unsafe-argument */
 
 import * as recast from 'recast';
 import * as path from 'path';
@@ -9,6 +12,8 @@ import type { ExportNamedDeclaration, Program } from '@babel/types';
 import { loadFile, writeFile } from 'magicast';
 
 import { ERROR_BOUNDARY_TEMPLATE_V2 } from '../templates';
+import { hasSentryContent } from '../utils';
+import { wrapAppWithSentry } from './root-common';
 
 export async function instrumentRootRouteV2(
   rootFileName: string,
@@ -63,18 +68,82 @@ export async function instrumentRootRouteV2(
 
     recast.visit(rootRouteAst.$ast, {
       visitExportDefaultDeclaration(path) {
-        // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
         const implementation = recast.parse(ERROR_BOUNDARY_TEMPLATE_V2).program
           .body[0];
 
         path.insertBefore(
-          // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
           recast.types.builders.exportDeclaration(false, implementation),
         );
 
         this.traverse(path);
       },
     });
+    // If there is already a ErrorBoundary export, and it doesn't have Sentry content
+  } else if (!hasSentryContent(rootFileName, rootRouteAst.$code)) {
+    rootRouteAst.imports.$add({
+      from: '@sentry/remix',
+      imported: 'captureRemixErrorBoundaryError',
+      local: 'captureRemixErrorBoundaryError',
+    });
+
+    wrapAppWithSentry(rootRouteAst, rootFileName);
+
+    recast.visit(rootRouteAst.$ast, {
+      visitExportNamedDeclaration(path) {
+        // Find ErrorBoundary export
+        if (path.value.declaration?.id?.name === 'ErrorBoundary') {
+          const errorBoundaryExport = path.value.declaration;
+
+          let errorIdentifier;
+
+          // check if useRouteError is called
+          recast.visit(errorBoundaryExport, {
+            visitVariableDeclaration(path) {
+              const variableDeclaration = path.value.declarations[0];
+              const initializer = variableDeclaration.init;
+
+              if (
+                initializer.type === 'CallExpression' &&
+                initializer.callee.name === 'useRouteError'
+              ) {
+                errorIdentifier = variableDeclaration.id.name;
+              }
+
+              this.traverse(path);
+            },
+          });
+
+          // We don't have an errorIdentifier, which means useRouteError is not called / imported
+          // We need to add it and capture the error
+          if (!errorIdentifier) {
+            rootRouteAst.imports.$add({
+              from: '@remix-run/react',
+              imported: 'useRouteError',
+              local: 'useRouteError',
+            });
+
+            const useRouteErrorCall = recast.parse(
+              `const error = useRouteError();`,
+            ).program.body[0];
+
+            // Insert at the top of ErrorBoundary body
+            errorBoundaryExport.body.body.splice(0, 0, useRouteErrorCall);
+          }
+
+          const captureErrorCall = recast.parse(
+            `captureRemixErrorBoundaryError(error);`,
+          ).program.body[0];
+
+          // Insert just before the the fallback page is returned
+          errorBoundaryExport.body.body.splice(
+            errorBoundaryExport.body.body.length - 1,
+            0,
+            captureErrorCall,
+          );
+        }
+        this.traverse(path);
+      },
+    });
   }
 
   await writeFile(