test(remix): Add server-side instrumentation integration tests. (#5538)

Adds a basic set of tests to ensure `loader`, `action` and `documentRequest` functions are properly instrumented.

Also, switches `node-integration-test` utilities to use `axios` to simulate requests, instead of `http` package, to easily switch between `post` and `get` requests.

Author: onurtemizkan

packages/node-integration-tests/package.json

@@ -23,6 +23,7 @@
     "@types/mysql": "^2.15.21",
     "@types/pg": "^8.6.5",
     "apollo-server": "^3.6.7",
+    "axios": "^0.27.2",
     "cors": "^2.8.5",
     "express": "^4.17.3",
     "graphql": "^16.3.0",

packages/node-integration-tests/utils/index.ts

@@ -1,12 +1,12 @@
 /* eslint-disable @typescript-eslint/no-unsafe-member-access */
-import { parseSemver } from '@sentry/utils';
+import { logger, parseSemver } from '@sentry/utils';
+import axios from 'axios';
 import { Express } from 'express';
 import * as http from 'http';
 import { RequestOptions } from 'https';
 import nock from 'nock';
 import * as path from 'path';
 import { getPortPromise } from 'portfinder';
-
 /**
  * Returns`describe` or `describe.skip` depending on allowed major versions of Node.
  *
@@ -33,7 +33,7 @@ export const conditionalTest = (allowedVersion: { min?: number; max?: number }):
 export const assertSentryEvent = (actual: Record<string, unknown>, expected: Record<string, unknown>): void => {
   expect(actual).toMatchObject({
     event_id: expect.any(String),
-    timestamp: expect.any(Number),
+    timestamp: expect.anything(),
     ...expected,
   });
 };
@@ -47,8 +47,8 @@ export const assertSentryEvent = (actual: Record<string, unknown>, expected: Rec
 export const assertSentryTransaction = (actual: Record<string, unknown>, expected: Record<string, unknown>): void => {
   expect(actual).toMatchObject({
     event_id: expect.any(String),
-    timestamp: expect.any(Number),
-    start_timestamp: expect.any(Number),
+    timestamp: expect.anything(),
+    start_timestamp: expect.anything(),
     spans: expect.any(Array),
     type: 'transaction',
     ...expected,
@@ -71,12 +71,18 @@ export const parseEnvelope = (body: string): Array<Record<string, unknown>> => {
  * @param url The url the intercepted requests will be directed to.
  * @param count The expected amount of requests to the envelope endpoint. If
  * the amount of sentrequests is lower than`count`, this function will not resolve.
+ * @param method The method of the request. Defaults to `GET`.
  * @returns The intercepted envelopes.
  */
-export const getMultipleEnvelopeRequest = async (url: string, count: number): Promise<Record<string, unknown>[][]> => {
+export const getMultipleEnvelopeRequest = async (
+  url: string,
+  count: number,
+  method: 'get' | 'post' = 'get',
+): Promise<Record<string, unknown>[][]> => {
   const envelopes: Record<string, unknown>[][] = [];
 
-  return new Promise(resolve => {
+  // eslint-disable-next-line no-async-promise-executor
+  return new Promise(async resolve => {
     nock('https://dsn.ingest.sentry.io')
       .post('/api/1337/envelope/', body => {
         const envelope = parseEnvelope(body);
@@ -92,7 +98,17 @@ export const getMultipleEnvelopeRequest = async (url: string, count: number): Pr
       .query(true) // accept any query params - used for sentry_key param
       .reply(200);
 
-    http.get(url);
+    try {
+      if (method === 'get') {
+        await axios.get(url);
+      } else {
+        await axios.post(url);
+      }
+    } catch (e) {
+      // We sometimes expect the request to fail, but not the test.
+      // So, we do nothing.
+      logger.warn(e);
+    }
   });
 };
 
@@ -133,10 +149,14 @@ export const getAPIResponse = async (url: URL, headers?: Record<string, string>)
  * Intercepts and extracts a single request containing a Sentry envelope
  *
  * @param url The url the intercepted request will be directed to.
+ * @param method The method of the request. Defaults to `GET`.
  * @returns The extracted envelope.
  */
-export const getEnvelopeRequest = async (url: string): Promise<Array<Record<string, unknown>>> => {
-  return (await getMultipleEnvelopeRequest(url, 1))[0];
+export const getEnvelopeRequest = async (
+  url: string,
+  method: 'get' | 'post' = 'get',
+): Promise<Array<Record<string, unknown>>> => {
+  return (await getMultipleEnvelopeRequest(url, 1, method))[0];
 };
 
 /**

packages/remix/test/integration/app/entry.server.tsx

@@ -6,6 +6,8 @@ import * as Sentry from '@sentry/remix';
 Sentry.init({
   dsn: 'https://public@dsn.ingest.sentry.io/1337',
   tracesSampleRate: 1,
+  // Disabling to test series of envelopes deterministically.
+  autoSessionTracking: false,
 });
 
 export default function handleRequest(

packages/remix/test/integration/app/root.tsx

@@ -10,7 +10,7 @@ export const meta: MetaFunction = ({ data }) => ({
   baggage: data.sentryBaggage,
 });
 
-function App() {
+export function App() {
   return (
     <html lang="en">
       <head>

packages/remix/test/integration/app/routes/action-json-response/$id.tsx

@@ -0,0 +1,31 @@
+import { ActionFunction, json, redirect, LoaderFunction } from '@remix-run/node';
+import { useActionData } from '@remix-run/react';
+
+export const loader: LoaderFunction = async ({ params: { id } }) => {
+  if (id === '-1') {
+    throw new Error('Unexpected Server Error from Loader');
+  }
+};
+
+export const action: ActionFunction = async ({ params: { id } }) => {
+  if (id === '-1') {
+    throw new Error('Unexpected Server Error');
+  }
+
+  if (id === '-2') {
+    // Note: This GET request triggers to the `Loader` of the URL, not the `Action`.
+    throw redirect('/action-json-response/-1');
+  }
+
+  return json({ test: 'test' });
+};
+
+export default function ActionJSONResponse() {
+  const data = useActionData();
+
+  return (
+    <div>
+      <h1>{data && data.test ? data.test : 'Not Found'}</h1>
+    </div>
+  );
+}

packages/remix/test/integration/app/routes/loader-json-response/$id.tsx

@@ -1,9 +1,17 @@
-import { json, LoaderFunction } from '@remix-run/node';
+import { json, LoaderFunction, redirect } from '@remix-run/node';
 import { useLoaderData } from '@remix-run/react';
 
 type LoaderData = { id: string };
 
 export const loader: LoaderFunction = async ({ params: { id } }) => {
+  if (id === '-2') {
+    throw new Error('Unexpected Server Error from Loader');
+  }
+
+  if (id === '-1') {
+    throw redirect('/loader-json-response/-2');
+  }
+
   return json({
     id,
   });
@@ -14,7 +22,7 @@ export default function LoaderJSONResponse() {
 
   return (
     <div>
-      <h1>{data.id}</h1>
+      <h1>{data && data.id ? data.id : 'Not Found'}</h1>
     </div>
   );
 }

packages/remix/test/integration/test/server/action.test.ts

@@ -0,0 +1,131 @@
+import {
+  assertSentryTransaction,
+  getEnvelopeRequest,
+  runServer,
+  getMultipleEnvelopeRequest,
+  assertSentryEvent,
+} from './utils/helpers';
+
+jest.spyOn(console, 'error').mockImplementation();
+
+describe('Remix API Actions', () => {
+  it('correctly instruments a parameterized Remix API action', async () => {
+    const baseURL = await runServer();
+    const url = `${baseURL}/action-json-response/123123`;
+    const envelope = await getEnvelopeRequest(url, 'post');
+    const transaction = envelope[2];
+
+    assertSentryTransaction(transaction, {
+      transaction: 'routes/action-json-response/$id',
+      spans: [
+        {
+          description: 'routes/action-json-response/$id',
+          op: 'remix.server.action',
+        },
+        {
+          description: 'routes/action-json-response/$id',
+          op: 'remix.server.loader',
+        },
+        {
+          description: 'routes/action-json-response/$id',
+          op: 'remix.server.documentRequest',
+        },
+      ],
+    });
+  });
+
+  it('reports an error thrown from the action', async () => {
+    const baseURL = await runServer();
+    const url = `${baseURL}/action-json-response/-1`;
+
+    const [transaction, event] = await getMultipleEnvelopeRequest(url, 2, 'post');
+
+    assertSentryTransaction(transaction[2], {
+      contexts: {
+        trace: {
+          status: 'internal_error',
+          tags: {
+            'http.status_code': '500',
+          },
+        },
+      },
+    });
+
+    assertSentryEvent(event[2], {
+      exception: {
+        values: [
+          {
+            type: 'Error',
+            value: 'Unexpected Server Error',
+            stacktrace: expect.any(Object),
+            mechanism: {
+              data: {
+                function: 'action',
+              },
+              handled: true,
+              type: 'instrument',
+            },
+          },
+        ],
+      },
+    });
+  });
+
+  it('handles a thrown 500 response', async () => {
+    const baseURL = await runServer();
+    const url = `${baseURL}/action-json-response/-2`;
+
+    const [transaction_1, event, transaction_2] = await getMultipleEnvelopeRequest(url, 3, 'post');
+
+    assertSentryTransaction(transaction_1[2], {
+      contexts: {
+        trace: {
+          op: 'http.server',
+          status: 'ok',
+          tags: {
+            method: 'POST',
+            'http.status_code': '302',
+          },
+        },
+      },
+      tags: {
+        transaction: 'routes/action-json-response/$id',
+      },
+    });
+
+    assertSentryTransaction(transaction_2[2], {
+      contexts: {
+        trace: {
+          op: 'http.server',
+          status: 'internal_error',
+          tags: {
+            method: 'GET',
+            'http.status_code': '500',
+          },
+        },
+      },
+      tags: {
+        transaction: 'routes/action-json-response/$id',
+      },
+    });
+
+    assertSentryEvent(event[2], {
+      exception: {
+        values: [
+          {
+            type: 'Error',
+            value: 'Unexpected Server Error from Loader',
+            stacktrace: expect.any(Object),
+            mechanism: {
+              data: {
+                function: 'loader',
+              },
+              handled: true,
+              type: 'instrument',
+            },
+          },
+        ],
+      },
+    });
+  });
+});