Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(remix): Add instrumentation step for Express server adapters. #504

Merged
merged 2 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

- feat(remix): Add instrumentation step for Express server adapters (#504)s

## 3.19.0

- feat(nextjs): Add instructions on how to set auth token in CI (#511)
Expand Down
197 changes: 197 additions & 0 deletions src/remix/codemods/express-server.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
// @ts-expect-error - clack is ESM and TS complains about that. It works though
import clack from '@clack/prompts';
import chalk from 'chalk';
import * as recast from 'recast';
import { visit } from 'ast-types';
import {
ASTNode,
ProxifiedImportItem,
generateCode,
loadFile,
writeFile,
// @ts-expect-error - magicast is ESM and TS complains about that. It works though
} from 'magicast';
import type { Program } from '@babel/types';
import * as fs from 'fs';

import { getInitCallInsertionIndex, hasSentryContent } from '../utils';
import { findFile } from '../../utils/ast-utils';

// Find `loadViteServerBuild` or `unstable_loadViteServerBuild` call inside an arrow function
// and replace it with await loadViteServerBuild.
// For context, see: https://github.com/getsentry/sentry-javascript/issues/9500
export function updateViteBuildParameter(node: ASTNode) {
const hasViteConfig = findFile('vite.config');

if (!hasViteConfig) {
return;
}

visit(node, {
visitArrowFunctionExpression(path) {
if (
path.value.body.type === 'CallExpression' &&
path.value.body.callee.type === 'Identifier' &&
(path.value.body.callee.name === 'unstable_loadViteServerBuild' ||
path.value.body.callee.name === 'loadViteServerBuild')
) {
// Replace the arrow function with a call to await loadViteServerBuild
path.replace(recast.types.builders.awaitExpression(path.value.body));
}

this.traverse(path);
},
});
}

// Try to find the Express server implementation that contains `createRequestHandler` from `@remix-run/express`
export async function findCustomExpressServerImplementation() {
const possiblePaths = [
'server',
'server/index',
'app/server',
'app/server/index',
];

for (const filePath of possiblePaths) {
const filename = findFile(filePath);

if (!filename) {
continue;
}

const fileStat = fs.statSync(filename);

if (!fileStat.isFile()) {
continue;
}

const fileMod = await loadFile(filename);
const createRequestHandlerImport = fileMod.imports.$items.find(
(imp) =>
imp.from === '@remix-run/express' &&
imp.imported === 'createRequestHandler',
);

if (createRequestHandlerImport) {
return filename;
}
}

return null;
}

// Wrap createRequestHandler with `wrapExpressCreateRequestHandler` from `@sentry/remix`
export async function instrumentExpressCreateRequestHandler(
expressServerPath: string,
): Promise<boolean> {
const originalExpressServerMod = await loadFile(expressServerPath);

if (
hasSentryContent(
generateCode(originalExpressServerMod.$ast).code,
originalExpressServerMod.$code,
)
) {
clack.log.warn(
`Express server in ${chalk.cyan(
expressServerPath,
)} already has Sentry instrumentation. Skipping.`,
);

return false;
}

originalExpressServerMod.imports.$add({
from: '@sentry/remix',
imported: 'wrapExpressCreateRequestHandler',
local: 'wrapExpressCreateRequestHandler',
});

const createRequestHandlerImport =
originalExpressServerMod.imports.$items.find(
(imp) =>
imp.from === '@remix-run/express' &&
imp.imported === 'createRequestHandler',
);

visit(originalExpressServerMod.$ast, {
visitIdentifier(path) {
if (
path.value.name === 'createRequestHandler' &&
path.parentPath.value.type === 'CallExpression'
) {
path.value.name = 'sentryCreateRequestHandler';
}

this.traverse(path);
},
});

// Insert the const declaration right after the imports
// Where we want to insert the const declaration is the same as where we would want to insert the init call.
const insertionIndex = getInitCallInsertionIndex(
originalExpressServerMod.$ast as Program,
);

const createRequestHandlerConst = wrapCreateRequestHandlerWithSentry(
createRequestHandlerImport,
);

if (!createRequestHandlerConst) {
// Todo: throw error
}

(originalExpressServerMod.$ast as Program).body.splice(
insertionIndex,
0,
// @ts-expect-error - string works here because the AST is proxified by magicast
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
createRequestHandlerConst,
);

// Update the Vite build parameter to await loadViteServerBuild if everything goes well.
// This should be the last thing we do.
updateViteBuildParameter(originalExpressServerMod.$ast);

try {
await writeFile(originalExpressServerMod.$ast, expressServerPath);

clack.log.info(
`Successfully instrumented Express server in ${chalk.cyan(
expressServerPath,
)}.`,
);
} catch (e) {
clack.log.warn(
`Could not write to Express server in ${chalk.cyan(expressServerPath)}.`,
);

throw e;
}

return true;
}

// Wrap `createRequestHandler` with `wrapExpressCreateRequestHandler` and set const name to `sentryCreateRequestHandler`
export function wrapCreateRequestHandlerWithSentry(
createRequestHandlerImport: ProxifiedImportItem | undefined,
) {
if (!createRequestHandlerImport) {
return;
}

const createRequestHandler = createRequestHandlerImport.local;

const wrapCreateRequestHandler = recast.types.builders.callExpression(
recast.types.builders.identifier('wrapExpressCreateRequestHandler'),
[recast.types.builders.identifier(createRequestHandler)],
);

return recast.types.builders.variableDeclaration('const', [
recast.types.builders.variableDeclarator(
recast.types.builders.identifier('sentryCreateRequestHandler'),
wrapCreateRequestHandler,
),
]);
}
20 changes: 20 additions & 0 deletions src/remix/remix-wizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
isRemixV2,
loadRemixConfig,
runRemixReveal,
instrumentExpressServer,
} from './sdk-setup';
import { debug } from '../utils/debug';
import { traceStep, withTelemetry } from '../telemetry';
Expand Down Expand Up @@ -152,6 +153,25 @@ async function runRemixWizardWithTelemetry(
}
});

await traceStep('Instrument custom Express server', async () => {
try {
const hasExpressAdapter = hasPackageInstalled(
'@remix-run/express',
packageJson,
);

if (!hasExpressAdapter) {
return;
}

await instrumentExpressServer();
} catch (e) {
clack.log.warn(`Could not instrument custom Express server.
Please do it manually using instructions from https://docs.sentry.io/platforms/javascript/guides/remix/manual-setup/#custom-express-server`);
debug(e);
}
});

clack.outro(`
${chalk.green(
'Sentry has been successfully configured for your Remix project.',
Expand Down
17 changes: 17 additions & 0 deletions src/remix/sdk-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ import { getInitCallInsertionIndex, hasSentryContent } from './utils';
import { instrumentRootRouteV1 } from './codemods/root-v1';
import { instrumentRootRouteV2 } from './codemods/root-v2';
import { instrumentHandleError } from './codemods/handle-error';
import {
findCustomExpressServerImplementation,
instrumentExpressCreateRequestHandler,
} from './codemods/express-server';
import { getPackageDotJson } from '../utils/clack-utils';

export type PartialRemixConfig = {
Expand Down Expand Up @@ -347,3 +351,16 @@ export async function initializeSentryOnEntryServer(
)}.`,
);
}

export async function instrumentExpressServer() {
const expressServerPath = await findCustomExpressServerImplementation();

if (!expressServerPath) {
clack.log.warn(
`Could not find custom Express server implementation. Please instrument it manually.`,
);
return;
}

await instrumentExpressCreateRequestHandler(expressServerPath);
}
Loading