feat(solidstart): Add withSentry config wrapper to enable building instrumentation files

andreiborza · andreiborza · commit 3aa394408be2 · 2024-09-27T11:22:34.000+09:00
diff --git a/packages/solidstart/.eslintrc.js b/packages/solidstart/.eslintrc.js
@@ -11,7 +11,7 @@ module.exports = {
       },
     },
     {
-      files: ['src/vite/**', 'src/server/**'],
+      files: ['src/vite/**', 'src/server/**', 'src/config/**'],
       rules: {
         '@sentry-internal/sdk/no-optional-chaining': 'off',
         '@sentry-internal/sdk/no-nullish-coalescing': 'off',
diff --git a/packages/solidstart/src/config/addInstrumentation.ts b/packages/solidstart/src/config/addInstrumentation.ts
@@ -0,0 +1,82 @@
+import * as fs from 'fs';
+import * as path from 'path';
+import { consoleSandbox } from '@sentry/utils';
+import type { Nitro } from './types';
+
+/**
+ * Adds the built `instrument.server.js` file to the output directory.
+ *
+ * This will no-op if no `instrument.server.js` file was found in the
+ * build directory. Make sure the `sentrySolidStartVite` plugin was
+ * added to `app.config.ts` to enable building the instrumentation file.
+ */
+export async function addInstrumentationFileToBuild(nitro: Nitro): Promise<void> {
+  const { buildDir, serverDir } = nitro.options.output;
+  const source = path.join(buildDir, 'build', 'ssr', 'instrument.server.js');
+  const destination = path.join(serverDir, 'instrument.server.mjs');
+
+  try {
+    await fs.promises.access(source, fs.constants.F_OK);
+    await fs.promises.copyFile(source, destination);
+
+    consoleSandbox(() => {
+      // eslint-disable-next-line no-console
+      console.log(`[Sentry SolidStart withSentry] Successfully created ${destination}.`);
+    });
+  } catch (error) {
+    consoleSandbox(() => {
+      // eslint-disable-next-line no-console
+      console.warn(`[Sentry SolidStart withSentry] Failed to create ${destination}.`, error);
+    });
+  }
+}
+
+/**
+ * Adds an `instrument.server.mjs` import to the top of the server entry file.
+ *
+ * This is meant as an escape hatch and should only be used in environments where
+ * it's not possible to `--import` the file instead as it comes with a limited
+ * tracing experience, only collecting http traces.
+ */
+export async function experimental_addInstrumentationFileTopLevelImportToServerEntry(
+  serverDir: string,
+  preset: string,
+): Promise<void> {
+  // other presets ('node-server' or 'vercel') have an index.mjs
+  const presetsWithServerFile = ['netlify'];
+  const instrumentationFile = path.join(serverDir, 'instrument.server.mjs');
+  const serverEntryFileName = presetsWithServerFile.includes(preset) ? 'server.mjs' : 'index.mjs';
+  const serverEntryFile = path.join(serverDir, serverEntryFileName);
+
+  try {
+    await fs.promises.access(instrumentationFile, fs.constants.F_OK);
+  } catch (error) {
+    consoleSandbox(() => {
+      // eslint-disable-next-line no-console
+      console.warn(
+        `[Sentry SolidStart withSentry] Tried to add \`${instrumentationFile}\` as top level import to \`${serverEntryFile}\`.`,
+        error,
+      );
+    });
+    return;
+  }
+
+  try {
+    const content = await fs.promises.readFile(serverEntryFile, 'utf-8');
+    const updatedContent = `import './instrument.server.mjs';\n${content}`;
+    await fs.promises.writeFile(serverEntryFile, updatedContent);
+
+    consoleSandbox(() => {
+      // eslint-disable-next-line no-console
+      console.log(
+        `[Sentry SolidStart withSentry] Added \`${instrumentationFile}\` as top level import to \`${serverEntryFile}\`.`,
+      );
+    });
+  } catch (error) {
+    // eslint-disable-next-line no-console
+    console.warn(
+      `[Sentry SolidStart withSentry] An error occurred when trying to add \`${instrumentationFile}\` as top level import to \`${serverEntryFile}\`.`,
+      error,
+    );
+  }
+}
diff --git a/packages/solidstart/src/config/index.ts b/packages/solidstart/src/config/index.ts
diff --git a/packages/solidstart/src/config/types.ts b/packages/solidstart/src/config/types.ts
@@ -0,0 +1,35 @@
+// Types to avoid pulling in extra dependencies
+// These are non-exhaustive
+export type Nitro = {
+  options: {
+    buildDir: string;
+    output: {
+      buildDir: string;
+      serverDir: string;
+    };
+    preset: string;
+  };
+};
+
+export type SolidStartInlineConfig = {
+  server?: {
+    hooks?: {
+      close?: () => unknown;
+      'rollup:before'?: (nitro: Nitro) => unknown;
+    };
+  };
+};
+
+export type SentrySolidStartConfigOptions = {
+  /**
+   * Enabling basic server tracing can be used for environments where modifying the node option `--import` is not possible.
+   * However, enabling this option only supports limited tracing instrumentation. Only http traces will be collected (but no database-specific traces etc.).
+   *
+   * If this option is `true`, the Sentry SDK will import the instrumentation.server.ts|js file at the top of the server entry file to load the SDK on the server.
+   *
+   * **DO NOT** enable this option if you've already added the node option `--import` in your node start script. This would initialize Sentry twice on the server-side and leads to unexpected issues.
+   *
+   * @default false
+   */
+  experimental_basicServerTracing?: boolean;
+};
diff --git a/packages/solidstart/src/config/withSentry.ts b/packages/solidstart/src/config/withSentry.ts
@@ -0,0 +1,47 @@
+import {
+  addInstrumentationFileToBuild,
+  experimental_addInstrumentationFileTopLevelImportToServerEntry,
+} from './addInstrumentation';
+import type { SentrySolidStartConfigOptions, SolidStartInlineConfig } from './types';
+
+export const withSentry = (
+  solidStartConfig: SolidStartInlineConfig = {},
+  sentrySolidStartConfigOptions: SentrySolidStartConfigOptions = {},
+): SolidStartInlineConfig => {
+  const server = solidStartConfig.server || {};
+  const hooks = server.hooks || {};
+
+  let serverDir: string;
+  let buildPreset: string;
+
+  return {
+    ...solidStartConfig,
+    server: {
+      ...server,
+      hooks: {
+        ...hooks,
+        async close() {
+          if (sentrySolidStartConfigOptions.experimental_basicServerTracing) {
+            await experimental_addInstrumentationFileTopLevelImportToServerEntry(serverDir, buildPreset);
+          }
+
+          // Run user provided hook
+          if (hooks.close) {
+            hooks.close();
+          }
+        },
+        async 'rollup:before'(nitro) {
+          serverDir = nitro.options.output.serverDir;
+          buildPreset = nitro.options.preset;
+
+          await addInstrumentationFileToBuild(nitro);
+
+          // Run user provided hook
+          if (hooks['rollup:before']) {
+            hooks['rollup:before'](nitro);
+          }
+        },
+      },
+    },
+  };
+};
diff --git a/packages/solidstart/src/vite/buildInstrumentationFile.ts b/packages/solidstart/src/vite/buildInstrumentationFile.ts
@@ -0,0 +1,55 @@
+import * as fs from 'fs';
+import * as path from 'path';
+import { consoleSandbox } from '@sentry/utils';
+import type { Plugin, UserConfig } from 'vite';
+
+/**
+ * A Sentry plugin for SolidStart to build the server
+ * `instrument.server.ts` file.
+ */
+export function makeBuildInstrumentationFilePlugin(
+  instrumentationFilePath: string = './src/instrument.server.ts',
+): Plugin {
+  return {
+    name: 'sentry-solidstart-build-instrumentation-file',
+    apply: 'build',
+    enforce: 'post',
+    async config(config: UserConfig, { command }) {
+      const router = (config as UserConfig & { router: { target: string; name: string; root: string } }).router;
+      const build = config.build || {};
+      const rollupOptions = build.rollupOptions || {};
+      const input = [...((rollupOptions.input || []) as string[])];
+
+      // plugin runs for client, server and sever-fns, we only want to run it for the server once.
+      if (command !== 'build' || router.target !== 'server' || router.name === 'server-fns') {
+        return config;
+      }
+
+      try {
+        await fs.promises.access(instrumentationFilePath, fs.constants.F_OK);
+      } catch (error) {
+        consoleSandbox(() => {
+          // eslint-disable-next-line no-console
+          console.warn(
+            `[Sentry SolidStart Plugin] Could not access \`${instrumentationFilePath}\`, please make sure it exists.`,
+            error,
+          );
+        });
+        return config;
+      }
+
+      input.push(path.join(router.root, instrumentationFilePath));
+
+      return {
+        ...config,
+        build: {
+          ...build,
+          rollupOptions: {
+            ...rollupOptions,
+            input,
+          },
+        },
+      };
+    },
+  };
+}
diff --git a/packages/solidstart/src/vite/sentrySolidStartVite.ts b/packages/solidstart/src/vite/sentrySolidStartVite.ts
@@ -1,4 +1,5 @@
 import type { Plugin } from 'vite';
+import { makeBuildInstrumentationFilePlugin } from './buildInstrumentationFile';
 import { makeSourceMapsVitePlugin } from './sourceMaps';
 import type { SentrySolidStartPluginOptions } from './types';
 
@@ -8,6 +9,8 @@ import type { SentrySolidStartPluginOptions } from './types';
 export const sentrySolidStartVite = (options: SentrySolidStartPluginOptions = {}): Plugin[] => {
   const sentryPlugins: Plugin[] = [];
 
+  sentryPlugins.push(makeBuildInstrumentationFilePlugin(options.instrumentation));
+
   if (process.env.NODE_ENV !== 'development') {
     if (options.sourceMapsUploadOptions?.enabled ?? true) {
       sentryPlugins.push(...makeSourceMapsVitePlugin(options));
diff --git a/packages/solidstart/src/vite/sourceMaps.ts b/packages/solidstart/src/vite/sourceMaps.ts
@@ -22,7 +22,7 @@ export function makeSourceMapsVitePlugin(options: SentrySolidStartPluginOptions)
           if (!sourceMapsUploadOptions?.filesToDeleteAfterUpload) {
             // eslint-disable-next-line no-console
             console.warn(
-              `[Sentry SolidStart PLugin] We recommend setting the \`sourceMapsUploadOptions.filesToDeleteAfterUpload\` option to clean up source maps after uploading.
+              `[Sentry SolidStart Plugin] We recommend setting the \`sourceMapsUploadOptions.filesToDeleteAfterUpload\` option to clean up source maps after uploading.
 [Sentry SolidStart Plugin] Otherwise, source maps might be deployed to production, depending on your configuration`,
             );
           }
diff --git a/packages/solidstart/src/vite/types.ts b/packages/solidstart/src/vite/types.ts
@@ -125,4 +125,12 @@ export type SentrySolidStartPluginOptions = {
    * Enabling this will give you, for example logs about source maps.
    */
   debug?: boolean;
+
+  /**
+   * The path to your `instrumentation.server.ts|js` file.
+   * e.g. './src/instrumentation.server.ts`
+   *
+   * Defaults to: `./src/instrumentation.server.ts`
+   */
+  instrumentation?: string;
 };
diff --git a/packages/solidstart/test/vite/buildInstrumentation.test.ts b/packages/solidstart/test/vite/buildInstrumentation.test.ts
@@ -0,0 +1,114 @@
+import type { UserConfig } from 'vite';
+import { describe, expect, it, vi } from 'vitest';
+import { makeBuildInstrumentationFilePlugin } from '../../src/vite/buildInstrumentationFile';
+
+const fsAccessMock = vi.fn();
+
+vi.mock('fs', async () => {
+  const actual = await vi.importActual('fs');
+  return {
+    ...actual,
+    promises: {
+      // @ts-expect-error this exists
+      ...actual.promises,
+      access: () => fsAccessMock(),
+    },
+  };
+});
+
+const consoleWarnSpy = vi.spyOn(console, 'warn');
+
+beforeEach(() => {
+  vi.clearAllMocks();
+});
+
+describe('makeBuildInstrumentationFilePlugin()', () => {
+  const viteConfig: UserConfig & { router: { target: string; name: string; root: string } } = {
+    router: {
+      target: 'server',
+      name: 'ssr',
+      root: '/some/project/path',
+    },
+    build: {
+      rollupOptions: {
+        input: ['/path/to/entry1.js', '/path/to/entry2.js'],
+      },
+    },
+  };
+
+  it('returns a plugin to set `sourcemaps` to `true`', () => {
+    const buildInstrumentationFilePlugin = makeBuildInstrumentationFilePlugin();
+
+    expect(buildInstrumentationFilePlugin.name).toEqual('sentry-solidstart-build-instrumentation-file');
+    expect(buildInstrumentationFilePlugin.apply).toEqual('build');
+    expect(buildInstrumentationFilePlugin.enforce).toEqual('post');
+    expect(buildInstrumentationFilePlugin.config).toEqual(expect.any(Function));
+  });
+
+  it('adds the instrumentation file for server builds', async () => {
+    const buildInstrumentationFilePlugin = makeBuildInstrumentationFilePlugin();
+    const config = await buildInstrumentationFilePlugin.config(viteConfig, { command: 'build' });
+    expect(config.build.rollupOptions.input).toContain('/some/project/path/src/instrument.server.ts');
+  });
+
+  it('adds the correct instrumentation file', async () => {
+    const buildInstrumentationFilePlugin = makeBuildInstrumentationFilePlugin('./src/myapp/instrument.server.ts');
+    const config = await buildInstrumentationFilePlugin.config(viteConfig, { command: 'build' });
+    expect(config.build.rollupOptions.input).toContain('/some/project/path/src/myapp/instrument.server.ts');
+  });
+
+  it("doesn't add the instrumentation file for server function builds", async () => {
+    const buildInstrumentationFilePlugin = makeBuildInstrumentationFilePlugin();
+    const config = await buildInstrumentationFilePlugin.config(
+      {
+        ...viteConfig,
+        router: {
+          ...viteConfig.router,
+          name: 'server-fns',
+        },
+      },
+      { command: 'build' },
+    );
+    expect(config.build.rollupOptions.input).not.toContain('/some/project/path/src/instrument.server.ts');
+  });
+
+  it("doesn't add the instrumentation file for client builds", async () => {
+    const buildInstrumentationFilePlugin = makeBuildInstrumentationFilePlugin();
+    const config = await buildInstrumentationFilePlugin.config(
+      {
+        ...viteConfig,
+        router: {
+          ...viteConfig.router,
+          target: 'client',
+        },
+      },
+      { command: 'build' },
+    );
+    expect(config.build.rollupOptions.input).not.toContain('/some/project/path/src/instrument.server.ts');
+  });
+
+  it("doesn't add the instrumentation file when serving", async () => {
+    const buildInstrumentationFilePlugin = makeBuildInstrumentationFilePlugin();
+    const config = await buildInstrumentationFilePlugin.config(viteConfig, { command: 'serve' });
+    expect(config.build.rollupOptions.input).not.toContain('/some/project/path/src/instrument.server.ts');
+  });
+
+  it("doesn't modify the config if the instrumentation file doesn't exist", async () => {
+    fsAccessMock.mockRejectedValueOnce(undefined);
+    const buildInstrumentationFilePlugin = makeBuildInstrumentationFilePlugin();
+    const config = await buildInstrumentationFilePlugin.config(viteConfig, { command: 'build' });
+    expect(config).toEqual(viteConfig);
+  });
+
+  it("logs a warning if the instrumentation file doesn't exist", async () => {
+    const error = new Error("File doesn't exist.");
+    fsAccessMock.mockRejectedValueOnce(error);
+    const buildInstrumentationFilePlugin = makeBuildInstrumentationFilePlugin();
+    const config = await buildInstrumentationFilePlugin.config(viteConfig, { command: 'build' });
+    expect(config).toEqual(viteConfig);
+    expect(consoleWarnSpy).toHaveBeenCalledWith(
+      '[Sentry SolidStart Plugin] Could not access `./src/instrument.server.ts`, please make sure it exists.',
+      error,
+    );
+  });
+});
diff --git a/packages/solidstart/test/vite/sentrySolidStartVite.test.ts b/packages/solidstart/test/vite/sentrySolidStartVite.test.ts

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ module.exports = {`
`11`	`11`	`},`
`12`	`12`	`},`
`13`	`13`	`{`
`14`		`- files: ['src/vite/', 'src/server/'],`
	`14`	`+ files: ['src/vite/', 'src/server/', 'src/config/**'],`
`15`	`15`	`rules: {`
`16`	`16`	`'@sentry-internal/sdk/no-optional-chaining': 'off',`
`17`	`17`	`'@sentry-internal/sdk/no-nullish-coalescing': 'off',`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ export function makeSourceMapsVitePlugin(options: SentrySolidStartPluginOptions)`
`22`	`22`	`if (!sourceMapsUploadOptions?.filesToDeleteAfterUpload) {`
`23`	`23`	`// eslint-disable-next-line no-console`
`24`	`24`	`console.warn(`
`25`		- `[Sentry SolidStart PLugin] We recommend setting the \`sourceMapsUploadOptions.filesToDeleteAfterUpload\` option to clean up source maps after uploading.
	`25`	+ `[Sentry SolidStart Plugin] We recommend setting the \`sourceMapsUploadOptions.filesToDeleteAfterUpload\` option to clean up source maps after uploading.
`26`	`26`	[Sentry SolidStart Plugin] Otherwise, source maps might be deployed to production, depending on your configuration`,
`27`	`27`	`);`
`28`	`28`	`}`