refactor(test): improve type safety for spawned process URLs (#7219)

- Split `spawnProc` into two functions with distinct return types:
  - `spawnProc`: expects long-running processes, always returns SpawnedProcess with `url`
  - `spawnProcAndExpectExit`: expects clean exit, may return void
- Make `url` property a lazy getter that throws if accessed before port message
- Ensures TypeScript knows `proc.url` is always defined for long-running processes

Author: watson

integration-tests/helpers/index.js

@@ -172,26 +172,50 @@ function assertTelemetryPoints (pid, msgs, expectedTelemetryPoints) {
 }
 
 /**
- * @typedef {childProcess.ChildProcess & { url: string }} SpawnedProcess
+ * @typedef {childProcess.ChildProcess & {
+ *   url: string,
+ *   stdout: NodeJS.ReadableStream,
+ *   stderr: NodeJS.ReadableStream
+ * }} SpawnedProcess
  */
 
 /**
  * Spawns a Node.js script in a child process and returns a promise that resolves when the process is ready.
  *
+ * This function expects the spawned process to stay alive (e.g., a server). If the process exits
+ * (even with code 0), the promise will reject with an error.
+ *
+ * For processes that are expected to run and exit cleanly, use `spawnProcAndExpectExit` instead.
+ *
  * @param {string|URL} filename - The filename of the Node.js script to spawn in a child process.
  * @param {childProcess.ForkOptions} [options] - The options to pass to the child process.
  * @param {(data: Buffer) => void} [stdioHandler] - A function that's called with one data argument to handle the
  *   standard output of the child process. If not provided, the output will be logged to the console.
  * @param {(data: Buffer) => void} [stderrHandler] - A function that's called with one data argument to handle the
  *   standard error of the child process. If not provided, the error will be logged to the console.
- * @returns {Promise<SpawnedProcess|void>} A promise that resolves when the process is either ready or terminated
- *   without an error. If the process is terminated without an error, the promise will resolve with `undefined`. The
- *   returned process will have a `url` property if the process didn't terminate.
+ * @returns {Promise<SpawnedProcess>} A promise that resolves with a SpawnedProcess when the process is ready.
+ *   The returned `SpawnedProcess` will have a `url` property that can be accessed to get the server URL.
+ *   Note: Accessing `url` before the spawned process sends its port message will throw an error.
  */
 function spawnProc (filename, options = {}, stdioHandler, stderrHandler) {
-  const proc = fork(filename, { ...options, stdio: 'pipe' })
+  const proc = spawnProcImpl(filename, options, stdioHandler, stderrHandler)
+
+  let urlValue
+  Object.defineProperty(proc, 'url', {
+    get () {
+      if (urlValue === undefined) {
+        throw new Error('Process URL is not available yet. The spawned process has not sent a port message.')
+      }
+      return urlValue
+    },
+    set (value) {
+      urlValue = value
+    },
+    enumerable: true,
+    configurable: true
+  })
 
-  return /** @type {Promise<SpawnedProcess|void>} */ (new Promise((resolve, reject) => {
+  return new Promise((resolve, reject) => {
     proc
       .on('message', ({ port }) => {
         if (typeof port !== 'number' && typeof port !== 'string') {
@@ -200,30 +224,74 @@ function spawnProc (filename, options = {}, stdioHandler, stderrHandler) {
         proc.url = `http://localhost:${port}`
         resolve(proc)
       })
+      .on('error', reject)
+      .on('exit', code => {
+        reject(new Error(`Process exited with status code ${code}.`))
+      })
+  })
+}
+
+/**
+ * Spawns a Node.js script in a child process that is expected to run and exit cleanly.
+ *
+ * This function expects the process to complete and exit with code 0, in which case the promise resolves
+ * with `undefined`. Use this for short-lived processes like validation scripts or tests that run to completion.
+ *
+ * For long-running processes (like servers) that should not exit, use `spawnProc` instead.
+ *
+ * @param {string|URL} filename - The filename of the Node.js script to spawn in a child process.
+ * @param {childProcess.ForkOptions} [options] - The options to pass to the child process.
+ * @param {(data: Buffer) => void} [stdioHandler] - A function that's called with one data argument to handle the
+ *   standard output of the child process. If not provided, the output will be logged to the console.
+ * @param {(data: Buffer) => void} [stderrHandler] - A function that's called with one data argument to handle the
+ *   standard error of the child process. If not provided, the error will be logged to the console.
+ * @returns {Promise<void>} A promise that resolves when the process exits with code 0.
+ */
+function spawnProcAndExpectExit (filename, options = {}, stdioHandler, stderrHandler) {
+  const proc = spawnProcImpl(filename, options, stdioHandler, stderrHandler)
+
+  return new Promise((resolve, reject) => {
+    proc
       .on('error', reject)
       .on('exit', code => {
         if (code !== 0) {
           return reject(new Error(`Process exited with status code ${code}.`))
         }
         resolve()
       })
+  })
+}
 
-    proc.stdout.on('data', data => {
-      if (stdioHandler) {
-        stdioHandler(data)
-      }
-      // eslint-disable-next-line no-console
-      if (!options.silent) console.log(data.toString())
-    })
+/**
+ * Internal implementation for spawnProc and spawnProcAndAllowExit.
+ *
+ * @param {string|URL} filename
+ * @param {childProcess.ForkOptions} options
+ * @param {(data: Buffer) => void} [stdioHandler]
+ * @param {(data: Buffer) => void} [stderrHandler]
+ * @returns {SpawnedProcess}
+ */
+function spawnProcImpl (filename, options, stdioHandler, stderrHandler) {
+  // Cast to SpawnedProcess type - when stdio is 'pipe', stdout/stderr are guaranteed non-null
+  const proc = /** @type {SpawnedProcess} */ (fork(filename, { ...options, stdio: 'pipe' }))
 
-    proc.stderr.on('data', data => {
-      if (stderrHandler) {
-        stderrHandler(data)
-      }
-      // eslint-disable-next-line no-console
-      if (!options.silent) console.error(data.toString())
-    })
-  }))
+  proc.stdout.on('data', data => {
+    if (stdioHandler) {
+      stdioHandler(data)
+    }
+    // eslint-disable-next-line no-console
+    if (!options.silent) console.log(data.toString())
+  })
+
+  proc.stderr.on('data', data => {
+    if (stderrHandler) {
+      stderrHandler(data)
+    }
+    // eslint-disable-next-line no-console
+    if (!options.silent) console.error(data.toString())
+  })
+
+  return proc
 }
 
 function log (...args) {
@@ -562,25 +630,24 @@ function checkSpansForServiceName (spans, name) {
 }
 
 /**
- * @overload
- * @param {string} cwd
- * @param {string} serverFile
- * @param {string|number} agentPort
- * @param {Record<string, string|undefined>} [additionalEnvArgs]
+ * @typedef {Record<string, string|undefined>} AdditionalEnvArgs
  */
+
 /**
+ * Prepares spawn options for plugin integration tests.
+ *
  * @param {string} cwd
  * @param {string} serverFile
  * @param {string|number} agentPort
+ * @param {AdditionalEnvArgs} [additionalEnvArgs]
+ * @param {string[]} [execArgv]
  * @param {(data: Buffer) => void} [stdioHandler]
- * @param {Record<string, string|undefined>} [additionalEnvArgs]
+ * @returns {{ filename: string, options: childProcess.ForkOptions,
+ *   stdioHandler: ((data: Buffer) => void) | undefined }}
  */
-async function spawnPluginIntegrationTestProc (
-  cwd, serverFile, agentPort, stdioHandler, additionalEnvArgs) {
-  if (typeof stdioHandler !== 'function' && !additionalEnvArgs) {
-    additionalEnvArgs = stdioHandler
-    stdioHandler = undefined
-  }
+function preparePluginIntegrationTestSpawnOptions (
+  cwd, serverFile, agentPort, additionalEnvArgs, execArgv, stdioHandler
+) {
   additionalEnvArgs = { ...additionalEnvArgs }
 
   let NODE_OPTIONS = `--loader=${hookFile}`
@@ -593,17 +660,65 @@ async function spawnPluginIntegrationTestProc (
     delete additionalEnvArgs.NODE_OPTIONS
   }
 
-  let env = /** @type {Record<string, string|undefined>} */ ({
-    NODE_OPTIONS,
-    DD_TRACE_AGENT_PORT: String(agentPort),
-    DD_TRACE_FLUSH_INTERVAL: '0'
-  })
-  env = { ...process.env, ...env, ...additionalEnvArgs }
-  return spawnProc(path.join(cwd, serverFile), {
-    cwd,
-    env,
-    execArgv: additionalEnvArgs.execArgv
-  }, stdioHandler)
+  return {
+    filename: path.join(cwd, serverFile),
+    options: {
+      cwd,
+      env: {
+        ...process.env,
+        NODE_OPTIONS,
+        DD_TRACE_AGENT_PORT: String(agentPort),
+        DD_TRACE_FLUSH_INTERVAL: '0',
+        ...additionalEnvArgs
+      },
+      execArgv
+    },
+    stdioHandler
+  }
+}
+
+/**
+ * Spawns a plugin integration test process that runs a long-lived server.
+ *
+ * The spawned process should call `process.send({ port })` to signal it's ready.
+ * Use this for tests that spawn HTTP servers or other long-running processes.
+ *
+ * For short-lived scripts that run and exit, use `spawnPluginIntegrationTestProcAndExpectExit` instead.
+ *
+ * @param {string} cwd
+ * @param {string} serverFile
+ * @param {string|number} agentPort
+ * @param {AdditionalEnvArgs} [additionalEnvArgs]
+ * @param {string[]} [execArgv]
+ * @param {(data: Buffer) => void} [stdioHandler]
+ */
+function spawnPluginIntegrationTestProc (cwd, serverFile, agentPort, additionalEnvArgs, execArgv, stdioHandler) {
+  const { filename, options, stdioHandler: handler } =
+    preparePluginIntegrationTestSpawnOptions(cwd, serverFile, agentPort, additionalEnvArgs, execArgv, stdioHandler)
+  return spawnProc(filename, options, handler)
+}
+
+/**
+ * Spawns a plugin integration test process that is expected to run and exit cleanly.
+ *
+ * Use this for short-lived test scripts that run instrumented code and exit (e.g., making a
+ * fetch request, DNS lookup, etc.) rather than starting a long-running server.
+ *
+ * For tests that spawn a server which should stay alive, use `spawnPluginIntegrationTestProc` instead.
+ *
+ * @param {string} cwd
+ * @param {string} serverFile
+ * @param {string|number} agentPort
+ * @param {AdditionalEnvArgs} [additionalEnvArgs]
+ * @param {string[]} [execArgv]
+ * @param {(data: Buffer) => void} [stdioHandler]
+ */
+function spawnPluginIntegrationTestProcAndExpectExit (
+  cwd, serverFile, agentPort, additionalEnvArgs, execArgv, stdioHandler
+) {
+  const { filename, options, stdioHandler: handler } =
+    preparePluginIntegrationTestSpawnOptions(cwd, serverFile, agentPort, additionalEnvArgs, execArgv, stdioHandler)
+  return spawnProcAndExpectExit(filename, options, handler)
 }
 
 /**
@@ -741,6 +856,7 @@ module.exports = {
   assertObjectContains,
   assertUUID,
   spawnProc,
+  spawnProcAndExpectExit,
   telemetryForwarder,
   assertTelemetryPoints,
   runAndCheckWithTelemetry,
@@ -750,6 +866,7 @@ module.exports = {
   getCiVisEvpProxyConfig,
   checkSpansForServiceName,
   spawnPluginIntegrationTestProc,
+  spawnPluginIntegrationTestProcAndExpectExit,
   useEnv,
   setShouldKill,
   sandboxCwd,

integration-tests/startup.spec.js

@@ -9,6 +9,7 @@ const semver = require('semver')
 const {
   FakeAgent,
   spawnProc,
+  spawnProcAndExpectExit,
   sandboxCwd,
   useSandbox,
   curlAndAssertMessage
@@ -238,7 +239,7 @@ execArgvs.forEach(({ execArgv, skip, optional = true }) => {
 
     context('with unsupported module', () => {
       it('skips the unsupported module', async () => {
-        await spawnProc(unsupportedTestFile, {
+        await spawnProcAndExpectExit(unsupportedTestFile, {
           cwd,
           execArgv
         })

packages/datadog-plugin-ai/test/integration-test/client.spec.js

@@ -7,7 +7,7 @@ const {
   FakeAgent,
   sandboxCwd,
   useSandbox,
-  spawnPluginIntegrationTestProc
+  spawnPluginIntegrationTestProcAndExpectExit
 } = require('../../../../integration-tests/helpers')
 const { withVersions } = require('../../../dd-trace/test/setup/mocha')
 
@@ -57,7 +57,7 @@ describe('esm', () => {
         assert.fail('No ai spans found')
       })
 
-      proc = await spawnPluginIntegrationTestProc(sandboxCwd(), 'server.mjs', agent.port, null, {
+      proc = await spawnPluginIntegrationTestProcAndExpectExit(sandboxCwd(), 'server.mjs', agent.port, {
         NODE_OPTIONS: '--import dd-trace/initialize.mjs'
       })
 

packages/datadog-plugin-amqp10/test/integration-test/client.spec.js

@@ -7,7 +7,7 @@ const {
   sandboxCwd,
   useSandbox,
   checkSpansForServiceName,
-  spawnPluginIntegrationTestProc
+  spawnPluginIntegrationTestProcAndExpectExit
 } = require('../../../../integration-tests/helpers')
 const { withVersions } = require('../../../dd-trace/test/setup/mocha')
 describe('esm', () => {
@@ -34,7 +34,7 @@ describe('esm', () => {
         assert.strictEqual(checkSpansForServiceName(payload, 'amqp.send'), true)
       })
 
-      proc = await spawnPluginIntegrationTestProc(sandboxCwd(), 'server.mjs', agent.port)
+      proc = await spawnPluginIntegrationTestProcAndExpectExit(sandboxCwd(), 'server.mjs', agent.port)
 
       await res
     }).timeout(20000)

packages/datadog-plugin-amqplib/test/integration-test/client.spec.js

@@ -5,7 +5,7 @@ const assert = require('node:assert/strict')
 const {
   FakeAgent,
   checkSpansForServiceName,
-  spawnPluginIntegrationTestProc,
+  spawnPluginIntegrationTestProcAndExpectExit,
   sandboxCwd,
   useSandbox,
   varySandbox
@@ -42,7 +42,7 @@ describe('esm', () => {
           assert.strictEqual(checkSpansForServiceName(payload, 'amqp.command'), true)
         })
 
-        proc = await spawnPluginIntegrationTestProc(sandboxCwd(), variants[variant], agent.port)
+        proc = await spawnPluginIntegrationTestProcAndExpectExit(sandboxCwd(), variants[variant], agent.port)
 
         await res
       }).timeout(20000)

packages/datadog-plugin-anthropic/test/integration-test/client.spec.js

@@ -8,7 +8,7 @@ const {
   sandboxCwd,
   useSandbox,
   checkSpansForServiceName,
-  spawnPluginIntegrationTestProc
+  spawnPluginIntegrationTestProcAndExpectExit
 } = require('../../../../integration-tests/helpers')
 const { withVersions } = require('../../../dd-trace/test/setup/mocha')
 
@@ -39,7 +39,7 @@ describe('esm', () => {
         assert.strictEqual(checkSpansForServiceName(payload, 'anthropic.request'), true)
       })
 
-      proc = await spawnPluginIntegrationTestProc(sandboxCwd(), 'server.mjs', agent.port, null, {
+      proc = await spawnPluginIntegrationTestProcAndExpectExit(sandboxCwd(), 'server.mjs', agent.port, {
         NODE_OPTIONS: '--import dd-trace/initialize.mjs',
         ANTHROPIC_API_KEY: '<not-a-real-key>'
       })

packages/datadog-plugin-aws-sdk/test/integration-test/client.spec.js

@@ -7,7 +7,7 @@ const {
   sandboxCwd,
   useSandbox,
   checkSpansForServiceName,
-  spawnPluginIntegrationTestProc
+  spawnPluginIntegrationTestProcAndExpectExit
 } = require('../../../../integration-tests/helpers')
 const { withVersions } = require('../../../dd-trace/test/setup/mocha')
 describe('esm', () => {
@@ -34,12 +34,10 @@ describe('esm', () => {
         assert.strictEqual(checkSpansForServiceName(payload, 'aws.request'), true)
       })
 
-      proc = await spawnPluginIntegrationTestProc(sandboxCwd(), 'server.mjs', agent.port, undefined,
-        {
-          AWS_SECRET_ACCESS_KEY: '0000000000/00000000000000000000000000000',
-          AWS_ACCESS_KEY_ID: '00000000000000000000'
-        }
-      )
+      proc = await spawnPluginIntegrationTestProcAndExpectExit(sandboxCwd(), 'server.mjs', agent.port, {
+        AWS_SECRET_ACCESS_KEY: '0000000000/00000000000000000000000000000',
+        AWS_ACCESS_KEY_ID: '00000000000000000000'
+      })
 
       await res
     }).timeout(20000)