feat(remix): Enable RequestData integration for server-side requests (

#6007) This enables the new `RequestData` integration in the remix SDK in order to add request data to server-side events. The integration itself is already installed automatically when the SDK initializes, because it's one of the underlying node SDK's default integrations, but without access to the request, it currently no-ops. This enables it to function as intended by storing the request object in the scope's `sdkProcessingMetadata`.
getsentry · Oct 27, 2022 · 6c8d6c3 · 6c8d6c3
1 parent 18b29d2
commit 6c8d6c3
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 4 deletions.
diff --git a/packages/node/src/requestdata.ts b/packages/node/src/requestdata.ts
@@ -169,7 +169,7 @@ export function extractRequestData(
   //   koa, nextjs: req.url
   const originalUrl = req.originalUrl || req.url || '';
   // absolute url
-  const absoluteUrl = `${protocol}://${host}${originalUrl}`;
+  const absoluteUrl = originalUrl.startsWith(protocol) ? originalUrl : `${protocol}://${host}${originalUrl}`;
   include.forEach(key => {
     switch (key) {
       case 'headers': {

diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts
@@ -70,14 +70,15 @@ function extractData(response: Response): Promise<unknown> {
   return responseClone.text();
 }
 
-function captureRemixServerException(err: Error, name: string): void {
+function captureRemixServerException(err: Error, name: string, request: Request): void {
   // Skip capturing if the thrown error is not a 5xx response
   // https://remix.run/docs/en/v1/api/conventions#throwing-responses-in-loaders
   if (isResponse(err) && err.status < 500) {
     return;
   }
 
   captureException(isResponse(err) ? extractData(err) : err, scope => {
+    scope.setSDKProcessingMetadata({ request });
     scope.addEventProcessor(event => {
       addExceptionMechanism(event, {
         type: 'instrument',
@@ -127,7 +128,7 @@ function makeWrappedDocumentRequestFunction(
 
       span?.finish();
     } catch (err) {
-      captureRemixServerException(err, 'documentRequest');
+      captureRemixServerException(err, 'documentRequest', request);
       throw err;
     }
 
@@ -164,7 +165,7 @@ function makeWrappedDataFunction(origFn: DataFunction, id: string, name: 'action
       currentScope.setSpan(activeTransaction);
       span?.finish();
     } catch (err) {
-      captureRemixServerException(err, name);
+      captureRemixServerException(err, name, args.request);
       throw err;
     }
 
@@ -353,6 +354,11 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui
     return local.bind(async () => {
       const hub = getCurrentHub();
       const options = hub.getClient()?.getOptions();
+      const scope = hub.getScope();
+
+      if (scope) {
+        scope.setSDKProcessingMetadata({ request });
+      }
 
       if (!options || !hasTracingEnabled(options)) {
         return origRequestHandler.call(this, request, loadContext);

diff --git a/packages/remix/src/utils/serverAdapters/express.ts b/packages/remix/src/utils/serverAdapters/express.ts
@@ -60,6 +60,11 @@ function wrapExpressRequestHandler(
     const request = extractRequestData(req);
     const hub = getCurrentHub();
     const options = hub.getClient()?.getOptions();
+    const scope = hub.getScope();
+
+    if (scope) {
+      scope.setSDKProcessingMetadata({ request });
+    }
 
     if (!options || !hasTracingEnabled(options) || !request.url || !request.method) {
       return origRequestHandler.call(this, req, res, next);

diff --git a/packages/remix/test/integration/test/server/action.test.ts b/packages/remix/test/integration/test/server/action.test.ts
@@ -30,6 +30,10 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada
           op: 'function.remix.document_request',
         },
       ],
+      request: {
+        method: 'POST',
+        url,
+      },
     });
   });
 
@@ -78,6 +82,44 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada
     });
   });
 
+  it('includes request data in transaction and error events', async () => {
+    const env = await RemixTestEnv.init(adapter);
+    const url = `${env.url}/action-json-response/-1`;
+
+    const envelopes = await env.getMultipleEnvelopeRequest({
+      url,
+      count: 2,
+      method: 'post',
+      envelopeType: ['transaction', 'event'],
+    });
+
+    const [transaction] = envelopes.filter(envelope => envelope[1].type === 'transaction');
+    const [event] = envelopes.filter(envelope => envelope[1].type === 'event');
+
+    assertSentryTransaction(transaction[2], {
+      transaction: 'routes/action-json-response/$id',
+      request: {
+        method: 'POST',
+        url,
+      },
+    });
+
+    assertSentryEvent(event[2], {
+      exception: {
+        values: [
+          {
+            type: 'Error',
+            value: 'Unexpected Server Error',
+          },
+        ],
+      },
+      request: {
+        method: 'POST',
+        url,
+      },
+    });
+  });
+
   it('handles a thrown 500 response', async () => {
     const env = await RemixTestEnv.init(adapter);
     const url = `${env.url}/action-json-response/-2`;

diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts
@@ -175,4 +175,9 @@ export interface Scope {
    * Clears attachments from the scope
    */
   clearAttachments(): this;
+
+  /**
+   * Add data which will be accessible during event processing but won't get sent to Sentry
+   */
+  setSDKProcessingMetadata(newData: { [key: string]: unknown }): this;
 }