Skip to content

Commit d381ace

Browse files
onurtemizkanAbhiPrasad
onurtemizkan
and
AbhiPrasad
authored
feat(remix): Set formData as action span data. (#10836)
Resolves: #10238 Clones the `request` in `action` to read `formData` and sets each entry as an attribute to the `action` span. Also adds a new e2e test application using latest Remix with Express, which both tests server and client-side. There seem to be a few limitations regarding the availability of complete data (multiple file uploads from a single input for example), but I think we can consider this as the best effort. This will only work when `sendDefaultPii` is set to `true`, but we can also add another option to control this. --------- Co-authored-by: Abhijeet Prasad <[email protected]>
1 parent cda367c commit d381ace

File tree

28 files changed

+1056
-10
lines changed

28 files changed

+1056
-10
lines changed

‎.github/workflows/build.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,7 @@ jobs:
998998
'create-next-app',
999999
'create-remix-app',
10001000
'create-remix-app-v2',
1001+
'create-remix-app-express',
10011002
'create-remix-app-express-vite-dev',
10021003
'debug-id-sourcemaps',
10031004
# 'esm-loader-node-express-app', # This is currently broken for upstream reasons. See https://github.com/getsentry/sentry-javascript/pull/11338#issuecomment-2025450675

‎dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
"vite-tsconfig-paths": "^4.2.1",
5050
"ts-node": "10.9.1"
5151
},
52-
"engines": {
53-
"node": ">=18.0.0"
52+
"volta": {
53+
"extends": "../../package.json"
5454
}
5555
}

‎dev-packages/e2e-tests/test-applications/create-remix-app-express/.eslintrc.cjs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/**
2+
* This is intended to be a basic starting point for linting in your app.
3+
* It relies on recommended configs out of the box for simplicity, but you can
4+
* and should modify this configuration to best suit your team's needs.
5+
*/
6+
7+
/** @type {import('eslint').Linter.Config} */
8+
module.exports = {
9+
root: true,
10+
parserOptions: {
11+
ecmaVersion: 'latest',
12+
sourceType: 'module',
13+
ecmaFeatures: {
14+
jsx: true,
15+
},
16+
},
17+
env: {
18+
browser: true,
19+
commonjs: true,
20+
es6: true,
21+
},
22+
23+
// Base config
24+
extends: ['eslint:recommended'],
25+
26+
overrides: [
27+
// React
28+
{
29+
files: ['**/*.{js,jsx,ts,tsx}'],
30+
plugins: ['react', 'jsx-a11y'],
31+
extends: [
32+
'plugin:react/recommended',
33+
'plugin:react/jsx-runtime',
34+
'plugin:react-hooks/recommended',
35+
'plugin:jsx-a11y/recommended',
36+
],
37+
settings: {
38+
react: {
39+
version: 'detect',
40+
},
41+
formComponents: ['Form'],
42+
linkComponents: [
43+
{ name: 'Link', linkAttribute: 'to' },
44+
{ name: 'NavLink', linkAttribute: 'to' },
45+
],
46+
'import/resolver': {
47+
typescript: {},
48+
},
49+
},
50+
},
51+
52+
// Typescript
53+
{
54+
files: ['**/*.{ts,tsx}'],
55+
plugins: ['@typescript-eslint', 'import'],
56+
parser: '@typescript-eslint/parser',
57+
settings: {
58+
'import/internal-regex': '^~/',
59+
'import/resolver': {
60+
node: {
61+
extensions: ['.ts', '.tsx'],
62+
},
63+
typescript: {
64+
alwaysTryTypes: true,
65+
},
66+
},
67+
},
68+
extends: ['plugin:@typescript-eslint/recommended', 'plugin:import/recommended', 'plugin:import/typescript'],
69+
},
70+
71+
// Node
72+
{
73+
files: ['.eslintrc.cjs', 'server.js'],
74+
env: {
75+
node: true,
76+
},
77+
},
78+
],
79+
};

‎dev-packages/e2e-tests/test-applications/create-remix-app-express/.gitignore

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
node_modules
2+
3+
/.cache
4+
/build
5+
/public/build
6+
.env

‎dev-packages/e2e-tests/test-applications/create-remix-app-express/.npmrc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
@sentry:registry=http://127.0.0.1:4873
2+
@sentry-internal:registry=http://127.0.0.1:4873

‎dev-packages/e2e-tests/test-applications/create-remix-app-express/app/entry.client.tsx

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { RemixBrowser, useLocation, useMatches } from '@remix-run/react';
2+
import * as Sentry from '@sentry/remix';
3+
import { StrictMode, startTransition, useEffect } from 'react';
4+
import { hydrateRoot } from 'react-dom/client';
5+
6+
Sentry.init({
7+
environment: 'qa', // dynamic sampling bias to keep transactions
8+
dsn: window.ENV.SENTRY_DSN,
9+
integrations: [
10+
Sentry.browserTracingIntegration({
11+
useEffect,
12+
useLocation,
13+
useMatches,
14+
}),
15+
Sentry.replayIntegration(),
16+
],
17+
// Performance Monitoring
18+
tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
19+
replaysSessionSampleRate: 0.1, // This sets the sample rate at 10%. You may want to change it to 100% while in development and then sample at a lower rate in production.
20+
replaysOnErrorSampleRate: 1.0, // If you're not already sampling the entire session, change the sample rate to 100% when sampling sessions where errors occur.
21+
tunnel: 'http://localhost:3031/', // proxy server
22+
});
23+
24+
Sentry.addEventProcessor(event => {
25+
if (
26+
event.type === 'transaction' &&
27+
(event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation')
28+
) {
29+
const eventId = event.event_id;
30+
if (eventId) {
31+
window.recordedTransactions = window.recordedTransactions || [];
32+
window.recordedTransactions.push(eventId);
33+
}
34+
}
35+
36+
return event;
37+
});
38+
39+
startTransition(() => {
40+
hydrateRoot(
41+
document,
42+
<StrictMode>
43+
<RemixBrowser />
44+
</StrictMode>,
45+
);
46+
});

‎dev-packages/e2e-tests/test-applications/create-remix-app-express/app/entry.server.tsx

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
import * as Sentry from '@sentry/remix';
2+
import * as isbotModule from 'isbot';
3+
4+
Sentry.init({
5+
tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
6+
environment: 'qa', // dynamic sampling bias to keep transactions
7+
dsn: process.env.E2E_TEST_DSN,
8+
tunnel: 'http://localhost:3031/', // proxy server
9+
sendDefaultPii: true, // Testing the FormData
10+
});
11+
12+
import { PassThrough } from 'node:stream';
13+
14+
import type { AppLoadContext, EntryContext } from '@remix-run/node';
15+
import { createReadableStreamFromReadable } from '@remix-run/node';
16+
import { installGlobals } from '@remix-run/node';
17+
import { RemixServer } from '@remix-run/react';
18+
import { renderToPipeableStream } from 'react-dom/server';
19+
20+
installGlobals();
21+
22+
const ABORT_DELAY = 5_000;
23+
24+
export const handleError = Sentry.wrapRemixHandleError;
25+
26+
export default function handleRequest(
27+
request: Request,
28+
responseStatusCode: number,
29+
responseHeaders: Headers,
30+
remixContext: EntryContext,
31+
loadContext: AppLoadContext,
32+
) {
33+
return isBotRequest(request.headers.get('user-agent'))
34+
? handleBotRequest(request, responseStatusCode, responseHeaders, remixContext)
35+
: handleBrowserRequest(request, responseStatusCode, responseHeaders, remixContext);
36+
}
37+
38+
// We have some Remix apps in the wild already running with isbot@3 so we need
39+
// to maintain backwards compatibility even though we want new apps to use
40+
// isbot@4. That way, we can ship this as a minor Semver update to @remix-run/dev.
41+
function isBotRequest(userAgent: string | null) {
42+
if (!userAgent) {
43+
return false;
44+
}
45+
46+
// isbot >= 3.8.0, >4
47+
if ('isbot' in isbotModule && typeof isbotModule.isbot === 'function') {
48+
return isbotModule.isbot(userAgent);
49+
}
50+
51+
// isbot < 3.8.0
52+
if ('default' in isbotModule && typeof isbotModule.default === 'function') {
53+
return isbotModule.default(userAgent);
54+
}
55+
56+
return false;
57+
}
58+
59+
function handleBotRequest(
60+
request: Request,
61+
responseStatusCode: number,
62+
responseHeaders: Headers,
63+
remixContext: EntryContext,
64+
) {
65+
return new Promise((resolve, reject) => {
66+
let shellRendered = false;
67+
const { pipe, abort } = renderToPipeableStream(
68+
<RemixServer context={remixContext} url={request.url} abortDelay={ABORT_DELAY} />,
69+
{
70+
onAllReady() {
71+
shellRendered = true;
72+
const body = new PassThrough();
73+
const stream = createReadableStreamFromReadable(body);
74+
75+
responseHeaders.set('Content-Type', 'text/html');
76+
77+
resolve(
78+
new Response(stream, {
79+
headers: responseHeaders,
80+
status: responseStatusCode,
81+
}),
82+
);
83+
84+
pipe(body);
85+
},
86+
onShellError(error: unknown) {
87+
reject(error);
88+
},
89+
onError(error: unknown) {
90+
responseStatusCode = 500;
91+
// Log streaming rendering errors from inside the shell. Don't log
92+
// errors encountered during initial shell rendering since they'll
93+
// reject and get logged in handleDocumentRequest.
94+
if (shellRendered) {
95+
console.error(error);
96+
}
97+
},
98+
},
99+
);
100+
101+
setTimeout(abort, ABORT_DELAY);
102+
});
103+
}
104+
105+
function handleBrowserRequest(
106+
request: Request,
107+
responseStatusCode: number,
108+
responseHeaders: Headers,
109+
remixContext: EntryContext,
110+
) {
111+
return new Promise((resolve, reject) => {
112+
let shellRendered = false;
113+
const { pipe, abort } = renderToPipeableStream(
114+
<RemixServer context={remixContext} url={request.url} abortDelay={ABORT_DELAY} />,
115+
{
116+
onShellReady() {
117+
shellRendered = true;
118+
const body = new PassThrough();
119+
const stream = createReadableStreamFromReadable(body);
120+
121+
responseHeaders.set('Content-Type', 'text/html');
122+
123+
resolve(
124+
new Response(stream, {
125+
headers: responseHeaders,
126+
status: responseStatusCode,
127+
}),
128+
);
129+
130+
pipe(body);
131+
},
132+
onShellError(error: unknown) {
133+
reject(error);
134+
},
135+
onError(error: unknown) {
136+
responseStatusCode = 500;
137+
// Log streaming rendering errors from inside the shell. Don't log
138+
// errors encountered during initial shell rendering since they'll
139+
// reject and get logged in handleDocumentRequest.
140+
if (shellRendered) {
141+
console.error(error);
142+
}
143+
},
144+
},
145+
);
146+
147+
setTimeout(abort, ABORT_DELAY);
148+
});
149+
}

‎dev-packages/e2e-tests/test-applications/create-remix-app-express/app/root.tsx

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { cssBundleHref } from '@remix-run/css-bundle';
2+
import { LinksFunction, MetaFunction, json } from '@remix-run/node';
3+
import {
4+
Links,
5+
LiveReload,
6+
Meta,
7+
Outlet,
8+
Scripts,
9+
ScrollRestoration,
10+
useLoaderData,
11+
useRouteError,
12+
} from '@remix-run/react';
13+
import { captureRemixErrorBoundaryError, withSentry } from '@sentry/remix';
14+
import type { SentryMetaArgs } from '@sentry/remix';
15+
16+
export const links: LinksFunction = () => [...(cssBundleHref ? [{ rel: 'stylesheet', href: cssBundleHref }] : [])];
17+
18+
export const loader = () => {
19+
return json({
20+
ENV: {
21+
SENTRY_DSN: process.env.E2E_TEST_DSN,
22+
},
23+
});
24+
};
25+
26+
export const meta = ({ data }: SentryMetaArgs<MetaFunction<typeof loader>>) => {
27+
return [
28+
{
29+
env: data.ENV,
30+
},
31+
{
32+
name: 'sentry-trace',
33+
content: data.sentryTrace,
34+
},
35+
{
36+
name: 'baggage',
37+
content: data.sentryBaggage,
38+
},
39+
];
40+
};
41+
42+
export function ErrorBoundary() {
43+
const error = useRouteError();
44+
const eventId = captureRemixErrorBoundaryError(error);
45+
46+
return (
47+
<div>
48+
<span>ErrorBoundary Error</span>
49+
<span id="event-id">{eventId}</span>
50+
</div>
51+
);
52+
}
53+
54+
function App() {
55+
const { ENV } = useLoaderData();
56+
57+
return (
58+
<html lang="en">
59+
<head>
60+
<meta charSet="utf-8" />
61+
<meta name="viewport" content="width=device-width,initial-scale=1" />
62+
<script
63+
dangerouslySetInnerHTML={{
64+
__html: `window.ENV = ${JSON.stringify(ENV)}`,
65+
}}
66+
/>
67+
<Meta />
68+
<Links />
69+
</head>
70+
<body>
71+
<Outlet />
72+
<ScrollRestoration />
73+
<Scripts />
74+
<LiveReload />
75+
</body>
76+
</html>
77+
);
78+
}
79+
80+
export default withSentry(App);

0 commit comments

Comments
 (0)