Skip to content

Commit

Permalink
feat(solidstart): Add server action instrumentation helper (#13035)
Browse files Browse the repository at this point in the history
Can be used like this:

```js
const getUserData = async () => {
  'use server';
  return await withServerActionInstrumentation('getData', () => {
    return { prefecture: 'Kanagawa' };
  });
};
```

Can also be used for api routes like this:

```js
export async function GET() {
    return await withServerActionInstrumentation('getUser', () => {
        return json({ prefecture: 'Akita' })
    })
}
```
  • Loading branch information
andreiborza authored Jul 30, 2024
1 parent 7dc6a25 commit 71af30f
Show file tree
Hide file tree
Showing 16 changed files with 392 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')!);
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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>;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
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}
<br />
Prefecture: {userData()?.prefecture}
</div>
);
}
Original file line number Diff line number Diff line change
@@ -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: 'GET /server-error',
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { expect, test } from '@playwright/test';
import { waitForTransaction } from '@sentry-internal/test-utils';
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
} from '@sentry/core';

test('sends a server action transaction on pageload', async ({ page }) => {
const transactionPromise = waitForTransaction('solidstart', transactionEvent => {
return transactionEvent?.transaction === 'GET /users/6';
});

await page.goto('/users/6');

const transaction = await transactionPromise;

expect(transaction.spans).toEqual(
expect.arrayContaining([
expect.objectContaining({
description: 'getPrefecture',
data: {
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.server_action',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.solidstart',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component',
},
}),
]),
);
});

test('sends a server action transaction on client navigation', async ({ page }) => {
const transactionPromise = waitForTransaction('solidstart', transactionEvent => {
return transactionEvent?.transaction === 'POST getPrefecture';
});

await page.goto('/');
await page.locator('#navLink').click();
await page.waitForURL('/users/5');

const transaction = await transactionPromise;

expect(transaction.spans).toEqual(
expect.arrayContaining([
expect.objectContaining({
description: 'getPrefecture',
data: {
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.server_action',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.solidstart',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component',
},
}),
]),
);
});
3 changes: 2 additions & 1 deletion packages/solidstart/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@
"@sentry/solid": "8.20.0",
"@sentry/types": "8.20.0",
"@sentry/utils": "8.20.0",
"@sentry/vite-plugin": "2.19.0"
"@sentry/vite-plugin": "2.19.0",
"@opentelemetry/instrumentation": "^0.52.1"
},
"devDependencies": {
"@solidjs/router": "^0.13.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/solidstart/rollup.npm.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/solidstart/src/index.types.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
2 changes: 2 additions & 0 deletions packages/solidstart/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,5 @@ export { withSentryErrorBoundary } from '@sentry/solid';
// -------------------------
// Solid Start SDK exports:
export { init } from './sdk';

export * from './withServerActionInstrumentation';
33 changes: 33 additions & 0 deletions packages/solidstart/src/server/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { flush } from '@sentry/node';
import { logger } from '@sentry/utils';
import { DEBUG_BUILD } from '../common/debug-build';

/** 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;
}
58 changes: 58 additions & 0 deletions packages/solidstart/src/server/withServerActionInstrumentation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, handleCallbackErrors } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, captureException, getActiveSpan, spanToJSON, startSpan } from '@sentry/node';
import { flushIfServerless, 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>> {
const activeSpan = getActiveSpan();

if (activeSpan) {
const spanData = spanToJSON(activeSpan).data;

// In solid start, server function calls are made to `/_server` which doesn't tell us
// a lot. We rewrite the span's route to be that of the sever action name but only
// if the target is `/_server`, otherwise we'd overwrite pageloads on routes that use
// server actions (which are more meaningful, e.g. a request to `GET /users/5` is more
// meaningful than overwriting it with `GET doSomeFunctionCall`).
if (spanData && !spanData['http.route'] && spanData['http.target'] === '/_server') {
activeSpan.setAttribute('http.route', serverActionName);
activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'component');
}
}

try {
return await startSpan(
{
op: 'function.server_action',
name: serverActionName,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.solidstart',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component',
},
},
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();
}
}
Loading

0 comments on commit 71af30f

Please sign in to comment.