feat(solidstart): Add server action instrumentation helper

andreiborza · andreiborza · commit 33fa0503f6a8 · 2024-07-25T09:47:38.000+02:00
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/package.json b/dev-packages/e2e-tests/test-applications/solidstart/package.json
@@ -11,6 +11,7 @@
       "This is currently not an issue outside of our repo. See: https://github.com/nksaraf/vinxi/issues/177"
     ],
     "preview": "HOST=localhost PORT=3030 NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi dev",
+    "start": "HOST=localhost PORT=3030 NODE_OPTIONS='--import ./src/instrument.server.mjs' vinxi start",
     "test:prod": "TEST_ENV=production playwright test",
     "test:build": "pnpm install && npx playwright install && pnpm build",
     "test:assert": "pnpm test:prod"
@@ -31,7 +32,7 @@
     "jsdom": "^24.0.0",
     "solid-js": "1.8.17",
     "typescript": "^5.4.5",
-    "vinxi": "^0.3.12",
+    "vinxi": "^0.4.0",
     "vite": "^5.2.8",
     "vite-plugin-solid": "^2.10.2",
     "vitest": "^1.5.0"
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/entry-client.tsx b/dev-packages/e2e-tests/test-applications/solidstart/src/entry-client.tsx
@@ -12,6 +12,7 @@ Sentry.init({
   tunnel: 'http://localhost:3031/', // proxy server
   // Performance Monitoring
   tracesSampleRate: 1.0, //  Capture 100% of the transactions
+  debug: !!import.meta.env.DEBUG,
 });
 
 mount(() => <StartClient />, document.getElementById('app')!);
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/instrument.server.mjs b/dev-packages/e2e-tests/test-applications/solidstart/src/instrument.server.mjs
@@ -5,4 +5,5 @@ Sentry.init({
   environment: 'qa', // dynamic sampling bias to keep transactions
   tracesSampleRate: 1.0, //  Capture 100% of the transactions
   tunnel: 'http://localhost:3031/', // proxy server
+  debug: !!process.env.DEBUG,
 });
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/routes/index.tsx b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/index.tsx
@@ -11,6 +11,9 @@ export default function Home() {
         <li>
           <A href="/client-error">Client error</A>
         </li>
+        <li>
+          <A href="/server-error">Server error</A>
+        </li>
         <li>
           <A id="navLink" href="/users/5">
             User 5
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/routes/server-error.tsx b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/server-error.tsx
@@ -0,0 +1,17 @@
+import { withServerActionInstrumentation } from '@sentry/solidstart';
+import { createAsync } from '@solidjs/router';
+
+const getPrefecture = async () => {
+  'use server';
+  return await withServerActionInstrumentation('getPrefecture', () => {
+    throw new Error('Error thrown from Solid Start E2E test app server route');
+
+    return { prefecture: 'Kanagawa' };
+  });
+};
+
+export default function ServerErrorPage() {
+  const data = createAsync(() => getPrefecture());
+
+  return <div>Prefecture: {data()?.prefecture}</div>;
+}
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/src/routes/users/[id].tsx b/dev-packages/e2e-tests/test-applications/solidstart/src/routes/users/[id].tsx
@@ -1,6 +1,20 @@
-import { useParams } from '@solidjs/router';
+import { withServerActionInstrumentation } from '@sentry/solidstart';
+import { createAsync, useParams } from '@solidjs/router';
 
+const getPrefecture = async () => {
+  'use server';
+  return await withServerActionInstrumentation('getPrefecture', () => {
+    return { prefecture: 'Ehime' };
+  });
+};
 export default function User() {
   const params = useParams();
-  return <div>User ID: {params.id}</div>;
+  const userData = createAsync(() => getPrefecture());
+
+  return (
+    <div>
+      User ID: {params.id}
+      Prefecture: {userData()?.prefecture}
+    </div>
+  );
 }
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/tests/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/solidstart/tests/errors.server.test.ts
@@ -0,0 +1,31 @@
+import { expect, test } from '@playwright/test';
+import { waitForError } from '@sentry-internal/test-utils';
+
+test.describe('server-side errors', () => {
+  test('captures server action error', async ({ page }) => {
+    const errorEventPromise = waitForError('solidstart', errorEvent => {
+      return errorEvent?.exception?.values?.[0]?.value === 'Error thrown from Solid Start E2E test app server route';
+    });
+
+    await page.goto(`/server-error`);
+
+    const error = await errorEventPromise;
+
+    expect(error.tags).toMatchObject({ runtime: 'node' });
+    expect(error).toMatchObject({
+      exception: {
+        values: [
+          {
+            type: 'Error',
+            value: 'Error thrown from Solid Start E2E test app server route',
+            mechanism: {
+              type: 'solidstart',
+              handled: false,
+            },
+          },
+        ],
+      },
+      transaction: 'getPrefecture',
+    });
+  });
+});
diff --git a/dev-packages/e2e-tests/test-applications/solidstart/tests/performance.server.test.ts b/dev-packages/e2e-tests/test-applications/solidstart/tests/performance.server.test.ts
@@ -0,0 +1,25 @@
+import { expect, test } from '@playwright/test';
+import { waitForTransaction } from '@sentry-internal/test-utils';
+
+test('sends a server action transaction', async ({ page }) => {
+  const transactionPromise = waitForTransaction('solidstart', transactionEvent => {
+    return transactionEvent?.transaction === 'getPrefecture';
+  });
+
+  await page.goto('/users/6');
+
+  const transaction = await transactionPromise;
+
+  expect(transaction).toMatchObject({
+    transaction: 'getPrefecture',
+    tags: { runtime: 'node' },
+    transaction_info: { source: 'url' },
+    type: 'transaction',
+    contexts: {
+      trace: {
+        op: 'function.server_action',
+        origin: 'manual',
+      },
+    },
+  });
+});
diff --git a/packages/solidstart/rollup.npm.config.mjs b/packages/solidstart/rollup.npm.config.mjs
@@ -16,7 +16,7 @@ export default makeNPMConfigVariants(
     // prevent this internal code from ending up in our built package (this doesn't happen automatially because
     // the name doesn't match an SDK dependency)
     packageSpecificConfig: {
-      external: ['solid-js', '@sentry/solid', '@sentry/solid/solidrouter'],
+      external: ['solid-js/web', 'solid-js', '@sentry/solid', '@sentry/solid/solidrouter'],
       output: {
         dynamicImportInCjs: true,
       },
diff --git a/packages/solidstart/src/index.types.ts b/packages/solidstart/src/index.types.ts
@@ -1,5 +1,5 @@
 // We export everything from both the client part of the SDK and from the server part.
-// Some of the exports collide, which is not allowed, unless we redifine the colliding
+// Some of the exports collide, which is not allowed, unless we redefine the colliding
 // exports in this file - which we do below.
 export * from './client';
 export * from './server';
diff --git a/packages/solidstart/src/server/index.ts b/packages/solidstart/src/server/index.ts
@@ -126,3 +126,5 @@ export { withSentryErrorBoundary } from '@sentry/solid';
 // -------------------------
 // Solid Start SDK exports:
 export { init } from './sdk';
+
+export * from './withServerActionInstrumentation';
diff --git a/packages/solidstart/src/server/utils.ts b/packages/solidstart/src/server/utils.ts
@@ -0,0 +1,50 @@
+import { flush } from '@sentry/node';
+import { logger } from '@sentry/utils';
+import type { RequestEvent } from 'solid-js/web';
+import { DEBUG_BUILD } from '../common/debug-build';
+
+/**
+ * Takes a request event and extracts traceparent and DSC data
+ * from the `sentry-trace` and `baggage` DSC headers.
+ */
+export function getTracePropagationData(event: RequestEvent | undefined): {
+  sentryTrace: string | undefined;
+  baggage: string | null;
+} {
+  const request = event && event.request;
+  const headers = request && request.headers;
+  const sentryTrace = (headers && headers.get('sentry-trace')) || undefined;
+  const baggage = (headers && headers.get('baggage')) || null;
+
+  return { sentryTrace, baggage };
+}
+
+/** Flush the event queue to ensure that events get sent to Sentry before the response is finished and the lambda ends */
+export async function flushIfServerless(): Promise<void> {
+  const isServerless = !!process.env.LAMBDA_TASK_ROOT || !!process.env.VERCEL;
+
+  if (isServerless) {
+    try {
+      DEBUG_BUILD && logger.log('Flushing events...');
+      await flush(2000);
+      DEBUG_BUILD && logger.log('Done flushing events');
+    } catch (e) {
+      DEBUG_BUILD && logger.log('Error while flushing events:\n', e);
+    }
+  }
+}
+
+/**
+ * Determines if a thrown "error" is a redirect Response which Solid Start users can throw to redirect to another route.
+ * see: https://docs.solidjs.com/solid-router/reference/data-apis/response-helpers#redirect
+ * @param error the potential redirect error
+ */
+export function isRedirect(error: unknown): boolean {
+  if (error == null || !(error instanceof Response)) {
+    return false;
+  }
+
+  const hasValidLocation = typeof error.headers.get('location') === 'string';
+  const hasValidStatus = error.status >= 300 && error.status <= 308;
+  return hasValidLocation && hasValidStatus;
+}
diff --git a/packages/solidstart/src/server/withServerActionInstrumentation.ts b/packages/solidstart/src/server/withServerActionInstrumentation.ts
@@ -0,0 +1,65 @@
+import { SPAN_STATUS_ERROR, handleCallbackErrors } from '@sentry/core';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+  captureException,
+  continueTrace,
+  startSpan,
+  withIsolationScope,
+} from '@sentry/node';
+import { winterCGRequestToRequestData } from '@sentry/utils';
+import { getRequestEvent } from 'solid-js/web';
+import { flushIfServerless, getTracePropagationData, isRedirect } from './utils';
+
+/**
+ * Wraps a server action (functions that use the 'use server' directive) function body with Sentry Error and Performance instrumentation.
+ */
+export async function withServerActionInstrumentation<A extends (...args: unknown[]) => unknown>(
+  serverActionName: string,
+  callback: A,
+): Promise<ReturnType<A>> {
+  return withIsolationScope(isolationScope => {
+    const event = getRequestEvent();
+
+    if (event && event.request) {
+      isolationScope.setSDKProcessingMetadata({ request: winterCGRequestToRequestData(event.request) });
+    }
+    isolationScope.setTransactionName(serverActionName);
+
+    return continueTrace(getTracePropagationData(event), () => instrumentServerAction(serverActionName, callback));
+  });
+}
+
+async function instrumentServerAction<A extends (...args: unknown[]) => unknown>(
+  name: string,
+  callback: A,
+): Promise<ReturnType<A>> {
+  try {
+    return await startSpan(
+      {
+        op: 'function.server_action',
+        name,
+        forceTransaction: true,
+        attributes: {
+          [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+        },
+      },
+      async span => {
+        const result = await handleCallbackErrors(callback, error => {
+          if (!isRedirect(error)) {
+            span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
+            captureException(error, {
+              mechanism: {
+                handled: false,
+                type: 'solidstart',
+              },
+            });
+          }
+        });
+
+        return result;
+      },
+    );
+  } finally {
+    await flushIfServerless();
+  }
+}
diff --git a/packages/solidstart/test/server/withServerActionInstrumentation.test.ts b/packages/solidstart/test/server/withServerActionInstrumentation.test.ts
