ref(nextjs): Improve integration test debugging (#4144)

This includes two changes I made to help myself while debugging the nextjs integration tests for a recent PR:

1) Add the ability to set `debug: true` in `Sentry.init()` via a command line flag. In order to do this, I changed the `--debug` flag from a boolean to one which can appear multiple times and which accepts an optional string, either `requests` or `logs`, with the following behavior:
    - `yarn test:integration` -> no debug logging of any kind (matches current behavior)
    - `yarn test:integration --debug` -> sets `debug: true` in `Sentry.init()` (change in behavior)
    - `yarn test:integration --debug logs` -> sets `debug: true` in `Sentry.init()` (change in behavior)
    - `yarn test:integration --debug requests` -> logs intercepted requests (current behavior of `--debug` flag)

    I chose to make SDK debug logging the default (what you get if you just use `--debug` without an argument) because of the two options, it is significantly more useful in my experience. (Logging intercepted requests gets unwieldy fast, as they are large objects and end up just creating a lot of noise to sift through in order to find the one piece of data you might want.) Since bare `--debug` has up until now logged intercepted requests, this is technically a breaking change, but since these tests have a userbase of under 5 people, all of them on our team, I figured we could probably roll with it. :-)

2) Improve the VSCode debugger setup for the integration tests. This involved:
    - Adding a bash function to link sentry packages into the test app, so that we can test against any changes we make without having to reinstall all of the test app's dependencies again just to use the updated files.
    - Creating a task to be executed before each run of the debugger, which uses the above script to link monorepo packages (if not already linked) and then rebuilds the test app. (This happens by calling a new `yarn` script.)
    - Setting up the debugger's sourcemap config, so we can step through TS files rather than the generated JS files.
    - Adding more comments to the debug config to explain what each bit does and why.

Author: lobsterkatie

.vscode/launch.json

@@ -51,28 +51,47 @@
 
 
     // @sentry/nextjs - Run a specific integration test file
-    // Must have file in currently active tab when hitting the play button
+    // Must have test file in currently active tab when hitting the play button, and must already have run `yarn` in test app directory
     {
       "name": "Debug @sentry/nextjs integration tests - just open file",
       "type": "node",
       "cwd": "${workspaceFolder}/packages/nextjs",
       "request": "launch",
-      // TODO create a build task
-      // "preLaunchTask": "yarn build",
-
-      // this is going straight to `server.js` (rather than running the tests through yarn) in order to be able to skip
-      // having to reinstall dependencies on every new test run
+      // since we're not using the normal test runner, we need to make sure we're using the current version of all local
+      // SDK packages and then manually rebuild the test app
+      "preLaunchTask": "Prepare nextjs integration test app for debugging",
+      // running `server.js` directly (rather than running the tests through yarn) allows us to skip having to reinstall
+      // dependencies on every new test run
       "program": "${workspaceFolder}/packages/nextjs/test/integration/test/server.js",
       "args": [
         "--debug",
         // remove these two lines to run all integration tests
         "--filter",
         "${fileBasename}"
       ],
-      "sourceMaps": true,
+
       "skipFiles": [
-        "<node_internals>/**", "**/tslib/**"
+        "<node_internals>/**",
+        // this prevents us from landing in a neverending cycle of TS async-polyfill functions as we're stepping through
+        // our code
+        "${workspaceFolder}/node_modules/tslib/**/*"
       ],
+      "sourceMaps": true,
+      // this controls which files are sourcemapped
+      "outFiles": [
+        // our SDK code
+        "${workspaceFolder}/**/dist/**/*.js",
+        // the built test app
+        "${workspaceFolder}/packages/nextjs/test/integration/.next/**/*.js",
+        "!**/node_modules/**"
+      ],
+      "resolveSourceMapLocations": [
+        "${workspaceFolder}/**/dist/**",
+        "${workspaceFolder}/packages/nextjs/test/integration/.next/**",
+        "!**/node_modules/**"
+      ],
+      "internalConsoleOptions": "openOnSessionStart"
+
     },
   ]
 }

.vscode/tasks.json

@@ -0,0 +1,13 @@
+{
+  // See https://go.microsoft.com/fwlink/?LinkId=733558 for documentation about `tasks.json` syntax
+	"version": "2.0.0",
+	"tasks": [
+		{
+			"label": "Prepare nextjs integration test app for VSCode debugger",
+			"type": "npm",
+			"script": "predebug",
+			"path": "packages/nextjs/test/integration/",
+			"detail": "Link the SDK (if not already linked) and build test app"
+		}
+	]
+}

packages/nextjs/test/integration/package.json

@@ -4,6 +4,7 @@
   "scripts": {
     "dev": "next",
     "build": "next build",
+    "predebug": "source ../integration_test_utils.sh && link_monorepo_packages '../../..' && yarn build",
     "start": "next start"
   },
   "dependencies": {

packages/nextjs/test/integration/pages/api/http/index.ts

@@ -3,7 +3,9 @@ import { get } from 'http';
 import { NextApiRequest, NextApiResponse } from 'next';
 
 const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void> => {
+  // make an outgoing request in order to test that the `Http` integration creates a span
   await new Promise(resolve => get('http://example.com', resolve));
+
   res.status(200).json({});
 };
 

packages/nextjs/test/integration/pages/fetch.tsx

@@ -1,6 +1,7 @@
 const ButtonPage = (): JSX.Element => (
   <button
     onClick={() => {
+      // test that a span is created in the pageload transaction for this fetch request
       fetch('http://example.com').catch(() => {
         // no-empty
       });

packages/nextjs/test/integration/sentry.client.config.js

@@ -4,6 +4,8 @@ import { Integrations } from '@sentry/tracing';
 Sentry.init({
   dsn: 'https://[email protected]/1337',
   tracesSampleRate: 1,
+  debug: process.env.SDK_DEBUG,
+
   integrations: [
     new Integrations.BrowserTracing({
       // Used for testing http tracing

packages/nextjs/test/integration/sentry.server.config.js

@@ -3,6 +3,7 @@ import * as Sentry from '@sentry/nextjs';
 Sentry.init({
   dsn: 'https://[email protected]/1337',
   tracesSampleRate: 1,
+  debug: process.env.SDK_DEBUG,
 
   integrations: defaults => [
     ...defaults.filter(

packages/nextjs/test/integration/test/runner.js

@@ -14,10 +14,13 @@ const argv = yargs(process.argv.slice(2))
   .option('silent', {
     type: 'boolean',
     description: 'Hide all stdout and console logs except test results',
+    conflicts: ['debug'],
   })
   .option('debug', {
-    type: 'boolean',
-    description: 'Log intercepted requests and debug messages',
+    type: 'string',
+    description: 'Log intercepted requests and/or debug messages',
+    choices: ['', 'requests', 'logs'], // empty string will be equivalent to "logs"
+    conflicts: ['silent'],
   })
   .option('depth', {
     type: 'number',
@@ -82,6 +85,22 @@ module.exports.run = async ({
         scenarios.forEach(s => log(`⊙ Scenario found: ${path.basename(s)}`));
       }
     }
+
+    // Turn on the SDK's `debug` option or the logging of intercepted requests, or both
+
+    // `yarn test:integration --debug` or
+    // `yarn test:integration --debug logs` or
+    // `yarn test:integration --debug logs --debug requests`
+    if (argv.debug === '' || argv.debug === 'logs' || (Array.isArray(argv.debug) && argv.debug.includes('logs'))) {
+      process.env.SDK_DEBUG = true; // will set `debug: true` in `Sentry.init()
+    }
+
+    // `yarn test:integration --debug requests` or
+    // `yarn test:integration --debug logs --debug requests`
+    if (argv.debug === 'requests' || (Array.isArray(argv.debug) && argv.debug.includes('requests'))) {
+      process.env.LOG_REQUESTS = true;
+    }
+
     // Silence all the unnecessary server noise. We are capturing errors manualy anyway.
     if (argv.silent) {
       for (const level of ['log', 'warn', 'info', 'error']) {

packages/nextjs/test/integration/test/server/tracingHttp.js

@@ -8,6 +8,7 @@ const { getAsync, interceptTracingRequest } = require('../utils/server');
 module.exports = async ({ url: urlBase, argv }) => {
   const url = `${urlBase}/api/http`;
 
+  // this intercepts the outgoing request made by the route handler (which it makes in order to test span creation)
   nock('http://example.com')
     .get('/')
     .reply(200, 'ok');

packages/nextjs/test/integration/test/utils/client.js

@@ -20,13 +20,13 @@ const createRequestInterceptor = env => {
     }
 
     if (isEventRequest(request)) {
-      logIf(env.argv.debug, 'Intercepted Event', extractEventFromRequest(request), env.argv.depth);
+      logIf(process.env.LOG_REQUESTS, 'Intercepted Event', extractEventFromRequest(request), env.argv.depth);
       env.requests.events.push(request);
     } else if (isSessionRequest(request)) {
-      logIf(env.argv.debug, 'Intercepted Session', extractEnvelopeFromRequest(request), env.argv.depth);
+      logIf(process.env.LOG_REQUESTS, 'Intercepted Session', extractEnvelopeFromRequest(request), env.argv.depth);
       env.requests.sessions.push(request);
     } else if (isTransactionRequest(request)) {
-      logIf(env.argv.debug, 'Intercepted Transaction', extractEnvelopeFromRequest(request), env.argv.depth);
+      logIf(process.env.LOG_REQUESTS, 'Intercepted Transaction', extractEnvelopeFromRequest(request), env.argv.depth);
       env.requests.transactions.push(request);
     }
 

packages/nextjs/test/integration/test/utils/server.js

@@ -27,7 +27,7 @@ const interceptEventRequest = (expectedEvent, argv, testName = '') => {
   return nock('https://dsn.ingest.sentry.io')
     .post('/api/1337/store/', body => {
       logIf(
-        argv.debug,
+        process.env.LOG_REQUESTS,
         '\nIntercepted Event' + (testName.length ? ` (from test \`${testName}\`)` : ''),
         body,
         argv.depth,
@@ -42,7 +42,7 @@ const interceptSessionRequest = (expectedItem, argv, testName = '') => {
     .post('/api/1337/envelope/', body => {
       const { envelopeHeader, itemHeader, item } = parseEnvelope(body);
       logIf(
-        argv.debug,
+        process.env.LOG_REQUESTS,
         '\nIntercepted Session' + (testName.length ? ` (from test \`${testName}\`)` : ''),
         { envelopeHeader, itemHeader, item },
         argv.depth,
@@ -57,7 +57,7 @@ const interceptTracingRequest = (expectedItem, argv, testName = '') => {
     .post('/api/1337/envelope/', body => {
       const { envelopeHeader, itemHeader, item } = parseEnvelope(body);
       logIf(
-        argv.debug,
+        process.env.LOG_REQUESTS,
         '\nIntercepted Transaction' + (testName.length ? ` (from test \`${testName}\`)` : ''),
         { envelopeHeader, itemHeader, item },
         argv.depth,

packages/nextjs/test/integration_test_utils.sh

@@ -1,6 +1,7 @@
 function link_package() {
   local package_abs_path=$1
-  local package_name=$2
+  # strip the 'sentry-' prefix from the repo name of packages not in the monorepo (`cli`, `webpack-plugin`, and `wizard`)
+  local package_name=$(basename $package_abs_path | sed s/sentry-//)
 
   echo "Setting up @sentry/${package_name} for linking"
   pushd $package_abs_path
@@ -21,7 +22,7 @@ function linkcli() {
 
   # check to make sure the repo directory exists
   if [[ -d $LINKED_CLI_REPO ]]; then
-    link_package $LINKED_CLI_REPO "cli"
+    link_package $LINKED_CLI_REPO
   else
     # the $1 lets us insert a string in that spot if one is passed to `linkcli` (useful for when we're calling this from
     # within another linking function)
@@ -36,7 +37,7 @@ function linkplugin() {
 
   # check to make sure the repo directory exists
   if [[ -d $LINKED_PLUGIN_REPO ]]; then
-    link_package $LINKED_PLUGIN_REPO "webpack-plugin"
+    link_package $LINKED_PLUGIN_REPO
 
     # the webpack plugin depends on `@sentry/cli`, so if we're also using a linked version of the cli package, the
     # plugin needs to link to it, too
@@ -49,3 +50,30 @@ function linkplugin() {
     echo "ERROR: Can't link @sentry/wepack-plugin because $LINKED_PLUGIN_REPO does not exist."
   fi
 }
+
+# This is only really useful for running tests in the debugger, as the normal test runner reinstalls all SDK packages
+# from the local files on each test run
+function link_monorepo_packages() {
+  local repo_packages_dir=$1
+
+  for abs_package_path in ${repo_packages_dir}/*; do
+    local package_name=$(basename $abs_package_path)
+
+    # Skip packages under the `@sentry-internal` namespace (our function is only linking packages in the `@sentry`
+    # namespace, and besides, there's no reason to link such packages, as they're purely SDK dev dependencies).
+    #
+    # (The regex test ( `=~` ) is a sneaky way of testing if `package_name` is any of the three packages listed: if the
+    # string containing all of the packages containes a match to the regex solely consisting of the current package
+    # name, the current package must be in the list.)
+    if [[ "eslint-config-sdk eslint-plugin-sdk typescript" =~ $package_name ]]; then
+      continue
+    fi
+
+    # `-L` tests if the given file is a symbolic link, to see if linking has already been done
+    if [[ ! -L node_modules/@sentry/$package_name ]]; then
+      echo "Linking @sentry/$package_name"
+      link_package $abs_package_path >/dev/null 2>&1
+    fi
+
+  done
+}