diff --git a/dev-packages/e2e-tests/test-applications/solidstart/package.json b/dev-packages/e2e-tests/test-applications/solidstart/package.json index 6409d191de5b..dfcf8a47402a 100644 --- 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 index 9391faa9652d..11087fbb5918 100644 --- 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(() => , 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 index 4146470295e1..3dd5d8933b7b 100644 --- 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 index f1635dee3b63..873d542e4bae 100644 --- 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() {
  • Client error
  • +
  • + Server error +
  • 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 new file mode 100644 index 000000000000..05dce5e10a56 --- /dev/null +++ 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
    Prefecture: {data()?.prefecture}
    ; +} 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 index 639ab0be8118..73f1e9c43fac 100644 --- 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
    User ID: {params.id}
    ; + const userData = createAsync(() => getPrefecture()); + + return ( +
    + User ID: {params.id} + Prefecture: {userData()?.prefecture} +
    + ); } 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 new file mode 100644 index 000000000000..008ab1bb8ed9 --- /dev/null +++ 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 new file mode 100644 index 000000000000..52b0efc44727 --- /dev/null +++ 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 index 5a36889c70f0..b0087a93c6fe 100644 --- 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 index a2c4d1ab7c06..89eaa14662e3 100644 --- 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 index 8b6ab6148fb8..75a67d3bb847 100644 --- 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 new file mode 100644 index 000000000000..71e8c8915199 --- /dev/null +++ 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 { + 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 new file mode 100644 index 000000000000..9976b528a973 --- /dev/null +++ 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
    unknown>( + serverActionName: string, + callback: A, +): Promise> { + 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 unknown>( + name: string, + callback: A, +): Promise> { + 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 new file mode 100644 index 000000000000..b1df7f8d76f5 --- /dev/null +++ b/packages/solidstart/test/server/withServerActionInstrumentation.test.ts @@ -0,0 +1,215 @@ +import * as SentryNode from '@sentry/node'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + createTransport, + getCurrentScope, + getIsolationScope, + setCurrentClient, + spanToJSON, +} from '@sentry/node'; +import { NodeClient } from '@sentry/node'; +import { solidRouterBrowserTracingIntegration } from '@sentry/solidstart/solidrouter'; +import { redirect } from '@solidjs/router'; +import { Headers } from 'node-fetch'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockCaptureException = vi.spyOn(SentryNode, 'captureException').mockImplementation(() => ''); +const mockFlush = vi.spyOn(SentryNode, 'flush').mockImplementation(async () => true); +const mockContinueTrace = vi.spyOn(SentryNode, 'continueTrace'); + +const mockGetRequestEvent = vi.fn(); +vi.mock('solid-js/web', async () => { + const original = await vi.importActual('solid-js/web'); + return { + ...original, + getRequestEvent: (...args: unknown[]) => mockGetRequestEvent(...args), + }; +}); + +import { withServerActionInstrumentation } from '../../src/server'; + +describe('withServerActionInstrumentation', () => { + function createMockNodeClient(): NodeClient { + return new NodeClient({ + integrations: [], + tracesSampleRate: 1, + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + stackParser: () => [], + _metadata: { + sdk: { + name: 'sentry.javascript.solidstart', + }, + }, + }); + } + + // Mimics a server action function using sentry instrumentation + const serverActionGetPrefecture = async function getPrefecture() { + return withServerActionInstrumentation('getPrefecture', () => { + return { prefecture: 'Kagoshima' }; + }); + }; + + beforeEach(() => { + vi.clearAllMocks(); + vi.unstubAllEnvs(); + getCurrentScope().setClient(undefined); + getCurrentScope().clear(); + getIsolationScope().clear(); + }); + + afterEach(() => { + mockCaptureException.mockClear(); + }); + + it('calls captureException', async () => { + const error = new Error('Sample server action error'); + const serverAction = async function getData() { + return withServerActionInstrumentation('getData', () => { + throw error; + }); + }; + + const res = serverAction(); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).toHaveBeenCalledTimes(1); + expect(mockCaptureException).toHaveBeenCalledWith(error, { mechanism: { handled: false, type: 'solidstart' } }); + }); + + it("doesn't call captureException for thrown redirects", async () => { + const serverRedirectAction = async function getData() { + return withServerActionInstrumentation('getData', () => { + throw redirect('/'); + }); + }; + + const res = serverRedirectAction(); + await expect(res).rejects.toThrow(); + + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + + it('calls `startSpan`', async () => { + const spanStartMock = vi.fn(); + const client = createMockNodeClient(); + setCurrentClient(client); + + client.on('spanStart', span => spanStartMock(spanToJSON(span))); + client.addIntegration(solidRouterBrowserTracingIntegration()); + + await serverActionGetPrefecture(); + expect(spanStartMock).toHaveBeenCalledWith( + expect.objectContaining({ + op: 'function.server_action', + description: 'getPrefecture', + data: expect.objectContaining({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.server_action', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', + }), + }), + ); + }); + + it('calls `continueTrace` with the right sentry-trace and baggage', async () => { + const baggage = + 'sentry-environment=qa,sentry-public_key=12345678,sentry-trace_id=4c9b164c5b5f4a0c8db3ce490b935ea8,sentry-sample_rate=1,sentry-sampled=true'; + const sentryTrace = '4c9b164c5b5f4a0c8db3ce490b935ea8'; + mockGetRequestEvent.mockReturnValue({ + request: { + method: 'GET', + url: '/_server', + headers: new Headers([ + ['sentry-trace', sentryTrace], + ['baggage', baggage], + ]), + }, + }); + + await serverActionGetPrefecture(); + + expect(mockContinueTrace).to.toHaveBeenCalledWith( + expect.objectContaining({ + sentryTrace, + baggage, + }), + expect.any(Function), + ); + }); + + it('calls `continueTrace` with no sentry-trace or baggage', async () => { + mockGetRequestEvent.mockReturnValue({ request: {} }); + + await serverActionGetPrefecture(); + + expect(mockContinueTrace).to.toHaveBeenCalledWith( + expect.objectContaining({ + sentryTrace: undefined, + baggage: null, + }), + expect.any(Function), + ); + }); + + it('calls `flush` if lambda', async () => { + vi.stubEnv('LAMBDA_TASK_ROOT', '1'); + + await serverActionGetPrefecture(); + expect(mockFlush).toHaveBeenCalledTimes(1); + }); + + it('calls `flush` if vercel', async () => { + vi.stubEnv('VERCEL', '1'); + + await serverActionGetPrefecture(); + expect(mockFlush).toHaveBeenCalledTimes(1); + }); + + it('sets a server action transaction name', async () => { + const getPrefecture = async function load() { + return withServerActionInstrumentation('getPrefecture', () => { + return { prefecture: 'Kagoshima' }; + }); + }; + + await getPrefecture(); + + expect(getIsolationScope().getScopeData().transactionName).toEqual('getPrefecture'); + }); + + it('sets request data on the isolation scope', async () => { + const baggage = + 'sentry-environment=qa,sentry-public_key=12345678,sentry-trace_id=4c9b164c5b5f4a0c8db3ce490b935ea8,sentry-sample_rate=1,sentry-sampled=true'; + const sentryTrace = '4c9b164c5b5f4a0c8db3ce490b935ea8'; + mockGetRequestEvent.mockReturnValue({ + request: { + method: 'POST', + url: '/_server', + headers: new Headers([ + ['sentry-trace', sentryTrace], + ['baggage', baggage], + ]), + }, + }); + + await serverActionGetPrefecture(); + + expect(getIsolationScope().getScopeData()).toEqual( + expect.objectContaining({ + sdkProcessingMetadata: { + request: { + method: 'POST', + url: '/_server', + headers: { + 'sentry-trace': sentryTrace, + baggage, + }, + }, + }, + }), + ); + }); +});