Repository navigation
FormData not logged with RequestData integration #10238
Description
Is there an existing issue for this?
- I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
- I have reviewed the documentation https://docs.sentry.io/
- I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases
How do you use Sentry?
Sentry Saas (sentry.io)
Which SDK are you using?
@sentry/remix
SDK Version
7.93.0
Framework Version
No response
Link to Sentry event
SDK Setup
Sentry.init({
dsn: ENV.SENTRY_DSN,
tracesSampleRate: env.APP_ENV === "production" ? 0.5 : 0.1,
environment: env.SENTRY_ENVIRONMENT,
denyUrls: [
/\/resources\/healthcheck/,
/\/build\//,
/\/favicons\//,
/\/images\//,
/\/fonts\//,
/\/favicon.ico/,
/\/site\.webmanifest/,
/.*\.map$/,
],
maxValueLength: 5000,
integrations: [
new Sentry.Integrations.Http({ tracing: true }),
new Sentry.Integrations.Prisma({ client: prisma }),
new ProfilingIntegration(),
new Sentry.Integrations.RequestData(),
],
beforeSendTransaction(event) {
if (event.request?.url?.includes("/healthcheck")) return null;
return event;
},
});Steps to Reproduce
Expected Result
When a fetcher is submitted with form data or a Form is submitted with POST, it should log FormData in Sentry. I went through the source code of RequestData integration, I even debugged it and found out that the request body in @sentry/utils requestdata.js -> extractRequestData does not exist.
Actual Result
Cannot find any mention of request data. Sometimes the only thing it shows is something like request_body_size
Metadata
Metadata
Assignees
Projects
- Status
Activity
Lms24 commentedon Jan 19, 2024
Hi @jansedlon thanks for writing in! After discussing this in our team, I think we have two issues here which we should both tackle individually:
RequestDataintegration picks up the request fromevent.sdkProcessingMetadata. We suspect that the Remix SDK might not set this field correctly. We should verify if this is the problem, i.e. if the data is even available to the integration. Unless I'm completely missing something, the form data should be available inreq.bodywhich we already extract.sendDefaultPIISDK option.cc @onurtemizkan - when you have time, please take a look. ideally we resolve both (in separate PRs).
jansedlon commentedon Jan 19, 2024
Hi @Lms24 thank you very much for your response. The request data should be available, but the
bodyproperty inevent.sdkProcessingMetadata.reqis undefined or doesn't exist at all.onurtemizkan commentedon Jan 22, 2024
Thanks for reporting this @jansedlon.
Yes, it seems
body/formDatais not always available when using an Express server in Remix. Which looks like the expected behaviour (See: remix-run/remix#3616)@Lms24 - We can manually extract
formDataby cloning the request inactionand consuming it. Then we can attach it to the scope inaction. Which is not the most performance-efficient way but it's possible.Instead of using
setSDKProcessingMetadata, we can attach it to theactionspan, similarly how it's done in@sentry/nextjssentry-javascript/packages/nextjs/src/common/withServerActionInstrumentation.ts
Lines 110 to 117 in 5f0b506
What do you think? Should I go forward with that workaround?
jansedlon commentedon Jan 22, 2024
Hi @onurtemizkan ! Thank you for the follow up. It seems you're right. I have no idea what kind of slowdown we're talking about. Of course if you're uploading a file, then the slowdown is noticeable.
Lms24 commentedon Jan 23, 2024
If I understand the linked issue correctly (also worth noting, I have very limited context on using remix with express):
body-parserdoesn't work because remix can't consume it again)body-parsermiddleware work with our express request handler middleware?If I'm totally off here, I'd appreciate an explanation, thanks 😅
All in all the workaround to clone the request sounds reasonable to me if
sendDefaultPii)feat(remix): Set `formData` as `action` span data. (#10836)