From 06c74e16bfcfc29f0ae6fab5394c351f02b18785 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Wed, 2 Sep 2026 16:53:08 -0400 Subject: [PATCH 1/3] feat(apps): wire local execution into the real dev server Squashed from 8 commits (see git reflog b84888db for prior history) ahead of rebasing onto master's upload/publish removal (PR #494). Co-Authored-By: Claude Sonnet 5 --- packages/plugins/apps/package.json | 5 +- ...t-connection-ids-from-module-graph.test.ts | 15 + ...xtract-connection-ids-from-module-graph.ts | 4 +- .../src/backend/ast-parsing/module-graph.ts | 4 +- .../src/vite/dev-server-module-graph.test.ts | 207 ++++++ .../apps/src/vite/dev-server-module-graph.ts | 183 +++++ .../src/vite/dev-server.integration.test.ts | 422 +++++++++++ .../plugins/apps/src/vite/dev-server.test.ts | 663 ++++++++++++++++-- packages/plugins/apps/src/vite/dev-server.ts | 298 ++++++-- packages/plugins/apps/src/vite/index.test.ts | 190 ++++- packages/plugins/apps/src/vite/index.ts | 100 ++- .../apps/src/vite/local-execution.test.ts | 169 ++++- .../plugins/apps/src/vite/local-execution.ts | 249 ++++++- packages/plugins/apps/src/vite/retry-delay.ts | 28 + .../published/esbuild-plugin/package.json | 2 + packages/published/rollup-plugin/package.json | 2 + packages/published/rspack-plugin/package.json | 2 + packages/published/vite-plugin/package.json | 198 +++--- .../published/webpack-plugin/package.json | 2 + .../action-execution.js | 13 + .../fixtures/action_catalog_project/index.js | 15 + .../action_catalog_project/package.json | 14 + .../actionCatalogCall.backend.ts | 9 + .../fixtures/apps_backend_project/helper.ts | 9 + .../helperWithBannedImport.ts | 9 + .../mixedImports.backend.ts | 16 + .../nestedImport.backend.ts | 9 + .../apps_backend_project/package.json | 1 + .../viaBannedHelper.backend.ts | 9 + .../apps_backend_project/viaHelper.backend.ts | 9 + .../tests/src/_jest/fixtures/package.json | 3 +- packages/tests/src/_jest/fixtures/yarn.lock | 13 + packages/tests/src/_jest/helpers/mocks.ts | 69 ++ packages/tools/src/rollupConfig.mjs | 66 +- packages/tools/src/rollupConfig.test.ts | 79 +++ yarn.lock | 1 + 36 files changed, 2751 insertions(+), 336 deletions(-) create mode 100644 packages/plugins/apps/src/vite/dev-server-module-graph.test.ts create mode 100644 packages/plugins/apps/src/vite/dev-server-module-graph.ts create mode 100644 packages/plugins/apps/src/vite/dev-server.integration.test.ts create mode 100644 packages/plugins/apps/src/vite/retry-delay.ts create mode 100644 packages/tests/src/_jest/fixtures/action_catalog_project/action-execution.js create mode 100644 packages/tests/src/_jest/fixtures/action_catalog_project/index.js create mode 100644 packages/tests/src/_jest/fixtures/action_catalog_project/package.json create mode 100644 packages/tests/src/_jest/fixtures/apps_backend_project/actionCatalogCall.backend.ts create mode 100644 packages/tests/src/_jest/fixtures/apps_backend_project/helper.ts create mode 100644 packages/tests/src/_jest/fixtures/apps_backend_project/helperWithBannedImport.ts create mode 100644 packages/tests/src/_jest/fixtures/apps_backend_project/mixedImports.backend.ts create mode 100644 packages/tests/src/_jest/fixtures/apps_backend_project/nestedImport.backend.ts create mode 100644 packages/tests/src/_jest/fixtures/apps_backend_project/viaBannedHelper.backend.ts create mode 100644 packages/tests/src/_jest/fixtures/apps_backend_project/viaHelper.backend.ts diff --git a/packages/plugins/apps/package.json b/packages/plugins/apps/package.json index b2ca47558..025a90c45 100644 --- a/packages/plugins/apps/package.json +++ b/packages/plugins/apps/package.json @@ -31,14 +31,15 @@ }, "dependencies": { "@dd/core": "workspace:*", + "esbuild": "0.25.8", "eslint-scope": "7.2.2", "glob": "11.1.0", - "jszip": "3.10.1" + "jszip": "3.10.1", + "rollup": "4.45.1" }, "devDependencies": { "@types/eslint-scope": "3.7.7", "@types/estree": "1.0.8", - "rollup": "4.45.1", "typescript": "5.4.3", "vite": "6.3.5" } diff --git a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.test.ts b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.test.ts index b71aa327a..87f327844 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.test.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.test.ts @@ -73,6 +73,21 @@ describe('Backend Functions - extractConnectionIdsFromModuleGraph', () => { expect(extract([entry, helper])).toEqual(['conn-helper']); }); + test('Should extract a declared empty-string connection ID rather than treating it as absent', () => { + const entry = createRecord( + entryId, + ` + import { request } from '@datadog/action-catalog/http/http'; + + export function run() { + return request({ connectionId: '', inputs: {} }); + } + `, + ); + + expect(extract([entry])).toEqual(['']); + }); + test('Should resolve same-module connection ID values inside reachable helpers', () => { const helperId = '/project/src/backend/helpers/http.js'; const entry = createRecord( diff --git a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.ts b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.ts index ecd0fbd24..704b5db23 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.ts @@ -37,7 +37,9 @@ export function extractConnectionIdsFromModuleGraph( for (const callSite of findActionCatalogCallSites(record.ast, scopeAnalysis, record.id)) { const connectionId = extractConnectionIdFromActionCall(callSite, modules, record); - if (connectionId) { + // `''` is a valid declared connectionId, not "none" — only the absence of a + // `connectionId` property (`undefined`) means no restriction to record. + if (connectionId !== undefined) { connectionIds.add(connectionId); } } diff --git a/packages/plugins/apps/src/backend/ast-parsing/module-graph.ts b/packages/plugins/apps/src/backend/ast-parsing/module-graph.ts index 171f90066..5ce6e351e 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/module-graph.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/module-graph.ts @@ -190,7 +190,9 @@ function collectStaticModuleDependencies( })); } -function getStaticModuleSources(ast: Program): string[] { +// Exported so a caller without build-time Rollup ModuleInfo (the dev server) can resolve each +// specifier against this same list instead of a second AST walk that could drift from it. +export function getStaticModuleSources(ast: Program): string[] { return ast.body.flatMap((node) => { if ( (node.type === 'ImportDeclaration' || diff --git a/packages/plugins/apps/src/vite/dev-server-module-graph.test.ts b/packages/plugins/apps/src/vite/dev-server-module-graph.test.ts new file mode 100644 index 000000000..c651c155d --- /dev/null +++ b/packages/plugins/apps/src/vite/dev-server-module-graph.test.ts @@ -0,0 +1,207 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import { getMockLogger } from '@dd/tests/_jest/helpers/mocks'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import type { ViteDevServer } from 'vite'; + +import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; + +import { collectModuleGraphFromServer } from './dev-server-module-graph'; + +const FIXTURE_ROOT = path.resolve( + __dirname, + '../../../../tests/src/_jest/fixtures/apps_backend_project', +); +const ENTRY_ID = path.join(FIXTURE_ROOT, 'helper.ts'); +const SUFFIXED_ENTRY_ID = ENTRY_ID + LOCAL_EXECUTION_LOAD_SUFFIX; + +/** A minimal fake ModuleNode shape, matching only the fields collectModuleGraphFromServer reads. */ +interface FakeModuleNode { + id: string; + file?: string; + importedModules: Set; +} + +function makeFakeServer( + resolveId: (specifier: string) => Promise<{ id: string } | null>, + entryNode: FakeModuleNode = { + id: SUFFIXED_ENTRY_ID, + file: ENTRY_ID, + importedModules: new Set(), + }, +) { + return { + moduleGraph: { + getModuleById: (id: string) => (id === SUFFIXED_ENTRY_ID ? entryNode : undefined), + }, + pluginContainer: { + resolveId: (specifier: string) => resolveId(specifier), + }, + // Real Vite resolves/transforms (never executes) each node before this collector reads + // its `importedModules`; these fixtures pre-wire the full graph instead, so this is a + // no-op stand-in for that priming call. + transformRequest: async () => null, + } as unknown as ViteDevServer; +} + +describe('dev-server-module-graph — collectModuleGraphFromServer', () => { + test('Should fail closed, not fall back to the raw specifier, when resolveId fails to resolve a static import', async () => { + const server = makeFakeServer(async () => null); + + await expect( + collectModuleGraphFromServer(server, ENTRY_ID, FIXTURE_ROOT, getMockLogger()), + ).rejects.toThrow(/unresolvable import specifier ".\/getRuntimeUsers\.backend"/); + }); + + test('Should use the resolved id when resolveId succeeds', async () => { + const resolvedPath = path.join(FIXTURE_ROOT, 'getRuntimeUsers.backend.ts'); + const server = makeFakeServer(async () => ({ id: resolvedPath })); + + const records = await collectModuleGraphFromServer( + server, + ENTRY_ID, + FIXTURE_ROOT, + getMockLogger(), + ); + + expect(records.has(ENTRY_ID)).toBe(true); + }); + + test('Should throw a clear error when a module file cannot be read from disk', async () => { + const missingFile = path.join(FIXTURE_ROOT, 'does-not-exist.ts'); + const entryNode: FakeModuleNode = { + id: SUFFIXED_ENTRY_ID, + file: missingFile, + importedModules: new Set(), + }; + const server = makeFakeServer(async () => null, entryNode); + + await expect( + collectModuleGraphFromServer(server, ENTRY_ID, FIXTURE_ROOT, getMockLogger()), + ).rejects.toThrow(/unreadable module source/); + }); + + describe('when a module file fails to parse', () => { + let tempDir: string; + let badFile: string; + + beforeAll(() => { + tempDir = mkdtempSync(path.join(tmpdir(), 'dev-server-module-graph-test-')); + badFile = path.join(tempDir, 'broken.ts'); + writeFileSync(badFile, 'export function broken( {{{ this is not valid syntax'); + }); + + afterAll(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + test('Should throw a clear error instead of propagating the raw parser exception', async () => { + const entryNode: FakeModuleNode = { + id: SUFFIXED_ENTRY_ID, + file: badFile, + importedModules: new Set(), + }; + const server = makeFakeServer(async () => null, entryNode); + + await expect( + collectModuleGraphFromServer(server, ENTRY_ID, FIXTURE_ROOT, getMockLogger()), + ).rejects.toThrow(/unparseable module source/); + }); + }); + + test('Should fail closed on a dependency carrying a semantic Vite resource query (e.g. ?raw), instead of parsing it as ordinary source', async () => { + const rawImportPath = path.join(FIXTURE_ROOT, 'snippet.ts'); + const rawImportNode: FakeModuleNode = { + id: `${rawImportPath}?raw`, + file: rawImportPath, + importedModules: new Set(), + }; + const entryNode: FakeModuleNode = { + id: SUFFIXED_ENTRY_ID, + file: ENTRY_ID, + importedModules: new Set([rawImportNode]), + }; + const server = makeFakeServer(async () => ({ id: rawImportPath }), entryNode); + + await expect( + collectModuleGraphFromServer(server, ENTRY_ID, FIXTURE_ROOT, getMockLogger()), + ).rejects.toThrow(/Vite resource query on module id/); + }); + + test('Should fail closed on a semantic Vite resource query even when a plain (unqueried) node for the same file was visited first', async () => { + const sharedFile = path.join(FIXTURE_ROOT, 'getRuntimeUsers.backend.ts'); + const plainNode: FakeModuleNode = { + id: sharedFile, + file: sharedFile, + importedModules: new Set(), + }; + const queriedNode: FakeModuleNode = { + id: `${sharedFile}?raw`, + file: sharedFile, + importedModules: new Set(), + }; + const entryNode: FakeModuleNode = { + id: SUFFIXED_ENTRY_ID, + file: ENTRY_ID, + // Insertion order matters: the plain sibling (visited first) normalizes to the same + // moduleId as the query'd node, which is what let the query'd node's rejection be + // silently skipped by the visited-set dedup before the fix. + importedModules: new Set([plainNode, queriedNode]), + }; + const server = makeFakeServer(async () => ({ id: sharedFile }), entryNode); + + await expect( + collectModuleGraphFromServer(server, ENTRY_ID, FIXTURE_ROOT, getMockLogger()), + ).rejects.toThrow(/Vite resource query on module id/); + }); + + test('Should not infinite-loop or double-process a module reached through a cycle in the import graph', async () => { + const resolvedPath = path.join(FIXTURE_ROOT, 'getRuntimeUsers.backend.ts'); + const entryNode: FakeModuleNode = { + id: SUFFIXED_ENTRY_ID, + file: ENTRY_ID, + importedModules: new Set(), + }; + // A self-referential cycle: the entry "imports" itself via node.importedModules, the + // same shape a real circular backend-to-backend import produces in Vite's own module + // graph. The `visited` Set must stop this from being processed a second time. + entryNode.importedModules.add(entryNode); + const server = makeFakeServer(async () => ({ id: resolvedPath }), entryNode); + + const records = await collectModuleGraphFromServer( + server, + ENTRY_ID, + FIXTURE_ROOT, + getMockLogger(), + ); + + expect(records.size).toBe(1); + expect(records.has(ENTRY_ID)).toBe(true); + }); + + // The production build path checks every app-local module transitively, not just the + // .backend.ts entry — local execution must reject the same banned helper the same way. + test('Should reject a helper module transitively imported by a backend entry when it imports a banned Node builtin', async () => { + const entryPath = path.join(FIXTURE_ROOT, 'viaBannedHelper.backend.ts'); + const bannedHelperPath = path.join(FIXTURE_ROOT, 'helperWithBannedImport.ts'); + const bannedHelperNode: FakeModuleNode = { + id: bannedHelperPath, + file: bannedHelperPath, + importedModules: new Set(), + }; + const entryNode: FakeModuleNode = { + id: SUFFIXED_ENTRY_ID, + file: entryPath, + importedModules: new Set([bannedHelperNode]), + }; + const server = makeFakeServer(async () => ({ id: bannedHelperPath }), entryNode); + + await expect( + collectModuleGraphFromServer(server, ENTRY_ID, FIXTURE_ROOT, getMockLogger()), + ).rejects.toThrow(/Importing Node built-in module "fs" is not supported/); + }); +}); diff --git a/packages/plugins/apps/src/vite/dev-server-module-graph.ts b/packages/plugins/apps/src/vite/dev-server-module-graph.ts new file mode 100644 index 000000000..afc76abbb --- /dev/null +++ b/packages/plugins/apps/src/vite/dev-server-module-graph.ts @@ -0,0 +1,183 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +/* eslint-disable no-await-in-loop */ + +import { readFile } from '@dd/core/helpers/fs'; +import type { Logger } from '@dd/core/types'; +import { transform } from 'esbuild'; +import { parseAst } from 'rollup/parseAst'; +import type { ModuleNode, ViteDevServer } from 'vite'; + +import { + createParsedModuleRecord, + getStaticModuleSources, + type ParsedModuleRecord, + shouldTraverseCollectedModule, + unsupportedModuleGraphDependency, +} from '../backend/ast-parsing/module-graph'; +import { runBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; +import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; + +import { normalizeViteModuleId } from './backend-module-graph-collector'; + +/** + * Rebuilds `createBackendModuleGraphCollector`'s `ParsedModuleRecord` map for the dev server + * (no `moduleParsed` hook here), re-parsing each module from disk via `esbuild.transform`. + * Primes each node via `transformRequest` (resolve + transform, never executes) before running + * static checks, so a module can't dodge them by having `ssrLoadModule` already run its code. + */ +export async function collectModuleGraphFromServer( + server: ViteDevServer, + bareEntryId: string, + buildRoot: string, + log: Logger, +): Promise> { + const records = new Map(); + const visited = new Set(); + const pending: ModuleNode[] = []; + + const entryUrl = bareEntryId + LOCAL_EXECUTION_LOAD_SUFFIX; + await server.transformRequest(entryUrl, { ssr: true }); + const entryNode = server.moduleGraph.getModuleById(entryUrl); + if (entryNode) { + pending.push(entryNode); + } + + while (pending.length > 0) { + const node = pending.shift()!; + + const moduleId = node.id ? normalizeDevServerModuleId(node.id) : undefined; + if (!moduleId || !node.file) { + continue; + } + + // Checked by normalized moduleId (extension-based), before the query check below, so a + // non-traversable non-code import (e.g. `./template.html?raw`) is skipped like the + // build-time collector skips it, regardless of its query. + if (!shouldTraverseCollectedModule(moduleId, buildRoot)) { + continue; + } + + // Checked before the visited-set dedup below: a query'd id and its plain counterpart + // normalize to the same moduleId, so deduping first would let a query'd form silently + // skip this check once the plain form had already been visited. + if (node.id && hasSemanticViteQuery(node.id)) { + throw unsupportedModuleGraphDependency( + moduleId, + `Vite resource query on module id "${node.id}"`, + ); + } + + if (visited.has(moduleId)) { + continue; + } + visited.add(moduleId); + + // Known, accepted gap: reads straight from disk, so a project's own `load`/`transform` + // hook rewriting this file is invisible here. `transformRequest`'s output isn't usable + // instead — it already carries Vite's SSR import-rewrite, which our parser can't read. + let source: string; + try { + source = await readFile(node.file); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw unsupportedModuleGraphDependency( + moduleId, + `unreadable module source (${reason})`, + ); + } + + let ast; + try { + const stripped = await transform(source, { + loader: loaderForModuleId(moduleId), + format: 'esm', + }); + ast = parseAst(stripped.code); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw unsupportedModuleGraphDependency( + moduleId, + `unparseable module source (${reason})`, + ); + } + + // `createParsedModuleRecord` zips dependency ids positionally against the AST's static + // imports, so this list must be static-only, same order — resolved individually since + // the dev server has no Rollup-style `ModuleInfo.importedIds`. + const staticModuleSources = getStaticModuleSources(ast); + const importerFile = node.file; + const resolutions = await Promise.all( + staticModuleSources.map((moduleSource) => + server.pluginContainer.resolveId(moduleSource, importerFile ?? undefined, { + ssr: true, + }), + ), + ); + const staticDependencyIds = resolutions.map((resolved, index) => { + if (!resolved) { + // Fail closed — falling back to the raw specifier would let a connectionId + // silently drop out of the allowlist instead of failing loudly. + throw unsupportedModuleGraphDependency( + moduleId, + `unresolvable import specifier "${staticModuleSources[index]}"`, + ); + } + return normalizeDevServerModuleId(resolved.id); + }); + + const record = createParsedModuleRecord(moduleId, buildRoot, ast, staticDependencyIds); + if (record) { + // No build-time moduleParsed hook here (Rollup-only), so this is what catches a + // banned import or restricted global locally instead of only at publish time. + runBackendStaticChecks(record.ast, record.id, log, record.scopeAnalysis); + records.set(record.id, record); + } + + for (const dependencyNode of node.importedModules) { + // Primes this dependency's own `importedModules` before it's dequeued, the same + // non-evaluating priming the entry got above — so no node in the traversal is ever + // read before it has itself gone through this same transform-only step. + if (dependencyNode.id) { + await server.transformRequest(dependencyNode.id, { ssr: true }); + } + pending.push(dependencyNode); + } + } + + return records; +} + +function loaderForModuleId(moduleId: string): 'ts' | 'tsx' | 'jsx' | 'js' { + if (moduleId.endsWith('.tsx')) { + return 'tsx'; + } + if (moduleId.endsWith('.ts') || moduleId.endsWith('.mts') || moduleId.endsWith('.cts')) { + return 'ts'; + } + if (moduleId.endsWith('.jsx')) { + return 'jsx'; + } + return 'js'; +} + +// The marker is always a literal trailing suffix, never combined with another query — exact +// suffix match, not cutting at the first `?`, which would also discard a real resource query. +function stripLocalExecutionMarker(id: string): string { + return id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX) + ? id.slice(0, -LOCAL_EXECUTION_LOAD_SUFFIX.length) + : id; +} + +function hasSemanticViteQuery(id: string): boolean { + return stripLocalExecutionMarker(id).includes('?'); +} + +// Named distinctly from backend-module-graph-collector.ts's normalizeViteModuleId, which this +// wraps, since same-named-different-behavior would invite editing the wrong copy. +function normalizeDevServerModuleId(id: string): string { + const unsuffixedId = stripLocalExecutionMarker(id); + return normalizeViteModuleId(unsuffixedId); +} diff --git a/packages/plugins/apps/src/vite/dev-server.integration.test.ts b/packages/plugins/apps/src/vite/dev-server.integration.test.ts new file mode 100644 index 000000000..4588014b8 --- /dev/null +++ b/packages/plugins/apps/src/vite/dev-server.integration.test.ts @@ -0,0 +1,422 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +/** + * Real coverage for local execution's module resolution: a real Vite dev server runs against + * the `apps_backend_project` fixture with no mocked `viteBuild`/`loadModule`/`this.resolve()`, + * so `resolveId`'s suffix propagation and the connection-ID collector exercise Vite's actual + * SSR transform output, not a hand-crafted stand-in. + */ + +import { getAuthenticatedRequest } from '@dd/apps-plugin/auth'; +import { collectModuleGraphFromServer } from '@dd/apps-plugin/vite/dev-server-module-graph'; +import { createDevServerMiddleware } from '@dd/apps-plugin/vite/dev-server'; +import { getVitePlugin } from '@dd/apps-plugin/vite/index'; +import type { AuthOptionsWithDefaults } from '@dd/core/types'; +import { cleanEnv } from '@dd/tests/_jest/helpers/env'; +import { + createMockRequest, + createMockResponse, + getContextMock, + getMockLogger, +} from '@dd/tests/_jest/helpers/mocks'; +import nock from 'nock'; +import path from 'path'; +import { build, createServer, type Plugin, type ViteDevServer } from 'vite'; + +import { extractConnectionIdsFromModuleGraph } from '../backend/ast-parsing/extract-connection-ids-from-module-graph'; +import { encodeQueryName } from '../backend/encodeQueryName'; +import type { BackendFunction } from '../backend/types'; + +const FIXTURE_ROOT = path.resolve( + __dirname, + '../../../../tests/src/_jest/fixtures/apps_backend_project', +); + +// Disable jitter/backoff so retry-relevant tests don't add unnecessary delay. +const mockLongPolling = { + maxRetries: 10, + timeoutMs: 40_000, + jitter: false, + exponentialBackoff: false, +}; + +// getAuthenticatedRequest reads API-key auth from the environment (see +// setupAfterEnv's cleanEnv, which strips these after collection). +const restoreModuleEnv = cleanEnv(); +process.env.DD_API_KEY = 'test-api-key'; +process.env.DD_APP_KEY = 'test-app-key'; +const testApiKeyRequest = getAuthenticatedRequest(); + +afterAll(() => { + restoreModuleEnv(); +}); + +const getRuntimeUsersFunc: BackendFunction = { + relativePath: 'getRuntimeUsers', + name: 'getRuntimeUsers', + absolutePath: path.join(FIXTURE_ROOT, 'getRuntimeUsers.backend.ts'), + allowedConnectionIds: [], +}; + +const nestedImportFunc: BackendFunction = { + relativePath: 'nestedImport', + name: 'usesNestedImport', + absolutePath: path.join(FIXTURE_ROOT, 'nestedImport.backend.ts'), + allowedConnectionIds: [], +}; + +const viaHelperFunc: BackendFunction = { + relativePath: 'viaHelper', + name: 'usesHelper', + absolutePath: path.join(FIXTURE_ROOT, 'viaHelper.backend.ts'), + allowedConnectionIds: [], +}; + +// Never referenced by another test in this file — the cold-entry test below +// needs a module its shared beforeAll server has genuinely never loaded. +const noSdkFunc: BackendFunction = { + relativePath: 'noSdk', + name: 'noSdkFunction', + absolutePath: path.join(FIXTURE_ROOT, 'noSdk.backend.ts'), + allowedConnectionIds: [], +}; + +const actionCatalogCallFunc: BackendFunction = { + relativePath: 'actionCatalogCall', + name: 'postMessage', + absolutePath: path.join(FIXTURE_ROOT, 'actionCatalogCall.backend.ts'), + allowedConnectionIds: [], +}; + +const mixedImportsFunc: BackendFunction = { + relativePath: 'mixedImports', + name: 'usesMixedImports', + absolutePath: path.join(FIXTURE_ROOT, 'mixedImports.backend.ts'), + allowedConnectionIds: [], +}; + +describe('Dev Server Middleware — real end-to-end local execution', () => { + let server: ViteDevServer; + + beforeAll(async () => { + // The real configureServer hook (via getVitePlugin below) calls getAuthenticatedRequest() + // itself — set after setupAfterEnv's own beforeAll (which runs first and strips these via + // cleanEnv) so the real dev server actually resolves auth instead of warning and disabling it. + process.env.DD_API_KEY = 'test-api-key'; + process.env.DD_APP_KEY = 'test-app-key'; + + const appsPlugin: Plugin = { + name: 'dd-apps-test', + ...getVitePlugin({ + bundler: { build }, + context: getContextMock({ buildRoot: FIXTURE_ROOT }), + options: { + include: [], + longPolling: mockLongPolling, + }, + }), + }; + + server = await createServer({ + configFile: false, + root: FIXTURE_ROOT, + logLevel: 'silent', + server: { middlewareMode: true, hmr: false }, + plugins: [appsPlugin], + // Local execution never uses the browser pre-bundle step, and the fake + // @datadog/action-catalog fixture below can trip it up, so disable it. + optimizeDeps: { noDiscovery: true }, + }); + }); + + afterAll(async () => { + await server.close(); + }); + + test('Should import a real backend function directly via the real Vite dev server and execute it locally, with a real @datadog/apps-backend typed import resolving $.Source correctly', async () => { + const auth: AuthOptionsWithDefaults = { + apiKey: 'test-api-key', + appKey: 'test-app-key', + site: 'datadoghq.com', + }; + const middleware = createDevServerMiddleware( + build, + server.ssrLoadModule.bind(server), + () => [getRuntimeUsersFunc], + async () => [], + auth, + testApiKeyRequest, + mockLongPolling, + FIXTURE_ROOT, + getMockLogger(), + ); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(getRuntimeUsersFunc), + args: ['e2e-test'], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ + data: { + label: 'e2e-test', + executionUser: { id: 'local-dev', orgId: 'local-dev-org' }, + initiatingUser: { id: 'local-dev', orgId: 'local-dev-org' }, + }, + }); + }, 30000); + + // Covers resolveId's suffix propagation onto nestedImport.backend.ts's static import of + // getRuntimeUsers.backend.ts — without it, that import would resolve unsuffixed and get + // swapped for the frontend RPC-proxy stub instead of running for real. + test('Should preserve real code for a nested *.backend.ts import, not swap it for the frontend RPC-proxy stub', async () => { + const auth: AuthOptionsWithDefaults = { + apiKey: 'test-api-key', + appKey: 'test-app-key', + site: 'datadoghq.com', + }; + const middleware = createDevServerMiddleware( + build, + server.ssrLoadModule.bind(server), + () => [nestedImportFunc], + async () => [], + auth, + testApiKeyRequest, + mockLongPolling, + FIXTURE_ROOT, + getMockLogger(), + ); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(nestedImportFunc), + args: ['nested-value'], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: { value: 'nested-value' } }); + }, 30000); + + // Multi-hop case: viaHelper.backend.ts imports plain helper.ts, which imports + // getRuntimeUsers.backend.ts. Suffix propagation must follow through helper.ts even + // though helper.ts itself never gets suffixed. + test('Should preserve real code for a *.backend.ts import reached through an intermediate non-backend module', async () => { + const auth: AuthOptionsWithDefaults = { + apiKey: 'test-api-key', + appKey: 'test-app-key', + site: 'datadoghq.com', + }; + const middleware = createDevServerMiddleware( + build, + server.ssrLoadModule.bind(server), + () => [viaHelperFunc], + async () => [], + auth, + testApiKeyRequest, + mockLongPolling, + FIXTURE_ROOT, + getMockLogger(), + ); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(viaHelperFunc), + args: ['via-helper-value'], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: { value: 'via-helper-value' } }); + }, 30000); + + // Every other test bypasses getAllowedConnectionIds' real wiring via + // createDevServerMiddleware(..., () => [], ...); this one sends the request through + // server.middlewares — the real stack configureServer installs — to exercise it for real. + test('Should execute successfully through the real configureServer-installed middleware, walking a real multi-hop import graph', async () => { + // Registers viaHelperFunc in the real backend-function registry — a side effect of + // transforming the file as a normal (unsuffixed) frontend import, exactly like a real + // frontend entry point importing the generated client SDK would. + await server.ssrLoadModule(viaHelperFunc.absolutePath); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(viaHelperFunc), + args: ['real-middleware-value'], + }); + const res = createMockResponse(); + + server.middlewares(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: { value: 'real-middleware-value' } }); + }, 30000); + + // Uses noSdkFunc since every other function here already has a warm moduleGraph node from + // an earlier test, which would mask this invariant: on a cold entry, Vite only registers + // the node under its fully-resolved (suffixed) id, not the bare path. + test('Should compute allowed connection IDs on the very first request for an entry, with no prior priming import', async () => { + const loadModule = server.ssrLoadModule.bind(server); + // collectModuleGraphFromServer appends LOCAL_EXECUTION_LOAD_SUFFIX internally, so this + // closure only handles the bare id — matching vite/index.ts's real wiring. + const getAllowedConnectionIds = async (entryId: string) => + extractConnectionIdsFromModuleGraph( + entryId, + await collectModuleGraphFromServer(server, entryId, FIXTURE_ROOT, getMockLogger()), + FIXTURE_ROOT, + ); + + const auth: AuthOptionsWithDefaults = { + apiKey: 'test-api-key', + appKey: 'test-app-key', + site: 'datadoghq.com', + }; + const middleware = createDevServerMiddleware( + build, + loadModule, + () => [noSdkFunc], + getAllowedConnectionIds, + auth, + testApiKeyRequest, + mockLongPolling, + FIXTURE_ROOT, + getMockLogger(), + ); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(noSdkFunc), + args: [], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: { ok: true } }); + }, 30000); + + // Vite's SSR transform rewrites imports into `__vite_ssr_import__(...)` calls that + // collectActionCatalogImports's `ImportDeclaration` search can't parse, so the collector + // must read each module's original source from disk instead of the transformed output. + test('Should recognize a connectionId-scoped action-catalog call and allow it, not silently reject it', async () => { + const loadModule = server.ssrLoadModule.bind(server); + const getAllowedConnectionIds = async (entryId: string) => + extractConnectionIdsFromModuleGraph( + entryId, + await collectModuleGraphFromServer(server, entryId, FIXTURE_ROOT, getMockLogger()), + FIXTURE_ROOT, + ); + + const auth: AuthOptionsWithDefaults = { + apiKey: 'test-api-key', + appKey: 'test-app-key', + site: 'datadoghq.com', + }; + const middleware = createDevServerMiddleware( + build, + loadModule, + () => [actionCatalogCallFunc], + getAllowedConnectionIds, + auth, + testApiKeyRequest, + mockLongPolling, + FIXTURE_ROOT, + getMockLogger(), + ); + + // The connection-ID collector is under test here, not the preview-async round trip + // (already covered elsewhere) — the request just needs to pass the allowedConnectionIds + // check, so a minimal reply is enough. + const apiScope = nock('https://api.datadoghq.com') + .post('/api/v2/app-builder/queries/preview-async') + .reply(200, { data: { id: 'receipt-action-catalog' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-action-catalog') + .reply(200, { data: { attributes: { done: true, outputs: { ok: true } } } }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(actionCatalogCallFunc), + args: [], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: { ok: true } }); + expect(apiScope.isDone()).toBe(true); + }, 30000); + + // A dynamic import sits between two static ones, so resolution must come from the AST + // itself rather than node.importedModules's undocumented ordering for mixed imports. + test('Should recognize a connectionId-scoped action-catalog call even when a top-level dynamic import sits between two static imports', async () => { + const loadModule = server.ssrLoadModule.bind(server); + const getAllowedConnectionIds = async (entryId: string) => + extractConnectionIdsFromModuleGraph( + entryId, + await collectModuleGraphFromServer(server, entryId, FIXTURE_ROOT, getMockLogger()), + FIXTURE_ROOT, + ); + + const auth: AuthOptionsWithDefaults = { + apiKey: 'test-api-key', + appKey: 'test-app-key', + site: 'datadoghq.com', + }; + const middleware = createDevServerMiddleware( + build, + loadModule, + () => [mixedImportsFunc], + getAllowedConnectionIds, + auth, + testApiKeyRequest, + mockLongPolling, + FIXTURE_ROOT, + getMockLogger(), + ); + + const apiScope = nock('https://api.datadoghq.com') + .post('/api/v2/app-builder/queries/preview-async') + .reply(200, { data: { id: 'receipt-mixed-imports' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-mixed-imports') + .reply(200, { data: { attributes: { done: true, outputs: { ok: true } } } }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mixedImportsFunc), + args: ['hello'], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: { ok: true } }); + expect(apiScope.isDone()).toBe(true); + }, 30000); +}); diff --git a/packages/plugins/apps/src/vite/dev-server.test.ts b/packages/plugins/apps/src/vite/dev-server.test.ts index ba7f9352b..83e9abd76 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -2,22 +2,41 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +/* global globalThis */ + import { getAuthenticatedRequest } from '@dd/apps-plugin/auth'; import { createDevServerMiddleware, getRetryDelay } from '@dd/apps-plugin/vite/dev-server'; import type { AuthOptionsWithDefaults } from '@dd/core/types'; import { cleanEnv } from '@dd/tests/_jest/helpers/env'; -import { getMockLogger } from '@dd/tests/_jest/helpers/mocks'; -import { EventEmitter } from 'events'; -import type { IncomingMessage, ServerResponse } from 'http'; +import { + createMockRequest, + createMockResponse, + getMockLogger, + mockLogFn, + moduleResolverFor, +} from '@dd/tests/_jest/helpers/mocks'; +import type { IncomingMessage } from 'http'; import nock from 'nock'; import { parseAst } from 'rollup/parseAst'; import { encodeQueryName } from '../backend/encodeQueryName'; import type { BackendFunction } from '../backend/types'; +import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; import type { AppsOptionsWithDefaults } from '../types'; +/** Shape of the `$.Actions` dynamic proxy — a nested property path (e.g. `$.Actions.slack.chat.postMessage`) callable at any depth; types `globalThis.$` in tests without an `any` cast. */ +type ActionsProxy = { [key: string]: ActionsProxy } & ((...args: unknown[]) => Promise); + +/** Reads the `$.Actions` local-execution.ts installs onto `globalThis` — genuinely untyped, so the cast is centralized here instead of repeated at each call site. */ +function testDollarActions(): ActionsProxy { + return (globalThis as typeof globalThis & { $: { Actions: ActionsProxy } }).$.Actions; +} + const mockViteBuild = jest.fn(); +/** Stands in for the real `server.ssrLoadModule` — the local executeAction path doesn't bundle, so tests exercising it configure this directly instead of `mockBuildWithParsedBackend`. */ +const mockLoadModule = jest.fn(); + const DD_API_ORIGIN = 'https://api.datadoghq.com'; const mockFunctions: BackendFunction[] = [ @@ -63,49 +82,6 @@ const mockLongPolling: AppsOptionsWithDefaults['longPolling'] = { exponentialBackoff: false, }; -/** - * Create a mock IncomingMessage with a JSON body. - */ -function createMockRequest(url: string, body: Record): IncomingMessage { - const req = new EventEmitter() as unknown as IncomingMessage; - req.method = 'POST'; - req.url = url; - - // Simulate body stream in next tick. - process.nextTick(() => { - (req as unknown as EventEmitter).emit('data', Buffer.from(JSON.stringify(body))); - (req as unknown as EventEmitter).emit('end'); - }); - - return req; -} - -/** - * Create a mock ServerResponse that captures output. - * Exposes a `done` promise that resolves when `end()` is called. - */ -function createMockResponse() { - let body = ''; - let resolveDone: () => void; - const done = new Promise((resolve) => { - resolveDone = resolve; - }); - - const res = { - statusCode: 200, - setHeader: jest.fn(), - end: jest.fn((data: string) => { - body = data || ''; - resolveDone(); - }), - getBody() { - return body; - }, - done, - }; - return res as typeof res & ServerResponse; -} - /** * Helper to create a fake Vite build result. */ @@ -231,10 +207,21 @@ describe('getRetryDelay', () => { }); }); +/** + * Configures `mockLoadModule` to resolve `func`'s absolute path to a module + * exporting a single named function, matching what the real `ssrLoadModule` + * returns for a real backend-function file. + */ +function mockLoadModuleReturning(func: BackendFunction, fn: (...args: never[]) => unknown) { + const resolveModule = moduleResolverFor(func, { [func.name]: fn }); + mockLoadModule.mockImplementation(resolveModule); +} + describe('Dev Server Middleware', () => { beforeEach(() => { jest.clearAllMocks(); mockViteBuild.mockReset(); + mockLoadModule.mockReset(); }); afterEach(() => { @@ -244,7 +231,9 @@ describe('Dev Server Middleware', () => { describe('createDevServerMiddleware routing', () => { const middleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, testAuthenticatedRequest, mockLongPolling, @@ -293,7 +282,28 @@ describe('Dev Server Middleware', () => { expect(res.end).toHaveBeenCalled(); }); - test('Should handle /__dd/executeAction POST', async () => { + test('Should handle /__dd/executeAction POST by running the function directly, no bundling, no network call', async () => { + mockLoadModuleReturning(mockFunctions[0], (arg) => arg); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: ['world'], + }); + const res = createMockResponse(); + const next = jest.fn(); + + middleware(req, res, next); + expect(next).not.toHaveBeenCalled(); + + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: 'world' }); + }); + + test('Should handle /__dd/executeActionViaCloud POST', async () => { mockBuildWithParsedBackend(); // Mock the Datadog API via nock. @@ -310,7 +320,7 @@ describe('Dev Server Middleware', () => { }, }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: ['world'], }); @@ -333,7 +343,9 @@ describe('Dev Server Middleware', () => { describe('debugBundle handler', () => { const middleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, testAuthenticatedRequest, mockLongPolling, @@ -441,10 +453,12 @@ describe('Dev Server Middleware', () => { }); }); - describe('executeAction handler', () => { + describe('executeActionViaCloud handler', () => { const middleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, testAuthenticatedRequest, mockLongPolling, @@ -453,7 +467,7 @@ describe('Dev Server Middleware', () => { ); test('Should return 400 for missing functionRef', async () => { - const req = createMockRequest('/__dd/executeAction', {}); + const req = createMockRequest('/__dd/executeActionViaCloud', {}); const res = createMockResponse(); middleware(req, res, jest.fn()); @@ -463,7 +477,7 @@ describe('Dev Server Middleware', () => { }); test('Should return 404 for unknown function', async () => { - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: 'nonexistent.nonexistent', }); const res = createMockResponse(); @@ -474,6 +488,34 @@ describe('Dev Server Middleware', () => { expect(res.statusCode).toBe(404); }); + test('Should reject a request with no auth configured upfront, matching the /__dd/executeAction gate', async () => { + const noAuthMiddleware = createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => mockFunctions, + async () => [], + mockAuth, + undefined, + mockLongPolling, + '/project', + mockLog, + ); + + const req = createMockRequest('/__dd/executeActionViaCloud', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + noAuthMiddleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(400); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(false); + expect(body.error).toContain('Auth credentials not configured'); + }); + /* * The nock mock replies with 403 to simulate the upstream Datadog API * rejecting the request (e.g. bad credentials). The middleware still @@ -490,7 +532,7 @@ describe('Dev Server Middleware', () => { .post('/api/v2/app-builder/queries/preview-async') .reply(403, 'Forbidden'); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -541,7 +583,7 @@ describe('Dev Server Middleware', () => { data: { attributes: { done: true, outputs: { data: { value: 42 } } } }, }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: ['hello', 42], }); @@ -572,7 +614,9 @@ describe('Dev Server Middleware', () => { const bearerMiddleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, getAuthenticatedRequest(), mockLongPolling, @@ -593,7 +637,7 @@ describe('Dev Server Middleware', () => { data: { attributes: { done: true, outputs: { data: { ok: true } } } }, }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -612,7 +656,9 @@ describe('Dev Server Middleware', () => { test('Should return 400 with auth guidance when the access token is missing', async () => { const noKeyMiddleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, undefined, mockLongPolling, @@ -620,7 +666,7 @@ describe('Dev Server Middleware', () => { mockLog, ); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -670,7 +716,7 @@ describe('Dev Server Middleware', () => { }); const trickyArgs = ["don't break", "'); alert(1); //", '😀']; - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: trickyArgs, }); @@ -699,7 +745,9 @@ describe('Dev Server Middleware', () => { ]; const middlewareWithAllowlist = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => functionsWithAllowlist, + async () => [], mockAuth, testAuthenticatedRequest, mockLongPolling, @@ -730,7 +778,7 @@ describe('Dev Server Middleware', () => { data: { attributes: { done: true, outputs: { data: { ok: true } } } }, }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(functionsWithAllowlist[1]), args: [], }); @@ -785,7 +833,7 @@ describe('Dev Server Middleware', () => { data: { attributes: { done: true, outputs: { data: { ok: true } } } }, }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -812,7 +860,7 @@ describe('Dev Server Middleware', () => { errors: [{ title: 'ExecutionFailed', detail: 'Script threw an error' }], }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -840,7 +888,7 @@ describe('Dev Server Middleware', () => { data: { attributes: { done: true, outputs: { data: { ok: true } } } }, }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -861,7 +909,9 @@ describe('Dev Server Middleware', () => { const singleAttemptMiddleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, testAuthenticatedRequest, { ...mockLongPolling, maxRetries: 1 }, @@ -875,7 +925,7 @@ describe('Dev Server Middleware', () => { .get('/api/v2/app-builder/queries/execution-long-polling/receipt-no-retry') .reply(200, { data: { attributes: { done: false } } }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -898,7 +948,9 @@ describe('Dev Server Middleware', () => { // as a failed action: the receipt stays valid across attempts. const stallingMiddleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, testAuthenticatedRequest, { ...mockLongPolling, timeoutMs: 100 }, @@ -917,7 +969,7 @@ describe('Dev Server Middleware', () => { data: { attributes: { done: true, outputs: { data: { ok: true } } } }, }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -942,7 +994,7 @@ describe('Dev Server Middleware', () => { .get('/api/v2/app-builder/queries/execution-long-polling/receipt-bad-request') .reply(403, { errors: [{ detail: 'Forbidden receipt' }] }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -960,12 +1012,497 @@ describe('Dev Server Middleware', () => { }); }); + describe('executeAction handler (local)', () => { + const middleware = createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => mockFunctions, + async () => [], + mockAuth, + testAuthenticatedRequest, + mockLongPolling, + '/project', + mockLog, + ); + + test('Should return 400 for missing functionRef', async () => { + const req = createMockRequest('/__dd/executeAction', {}); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(400); + }); + + test('Should return 404 for unknown function', async () => { + const req = createMockRequest('/__dd/executeAction', { + functionName: 'nonexistent.nonexistent', + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(404); + }); + + test('Should run the function directly in-process and return its result, with no bundling and no network call', async () => { + mockLoadModuleReturning(mockFunctions[0], (arg: number) => arg * 2); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [21], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + expect(body.result).toEqual({ data: 42 }); + expect(mockViteBuild).not.toHaveBeenCalled(); + }); + + test('Should reject a request with no auth configured upfront, even for a function that never calls $.Actions — matching production, which authenticates before any backend code runs', async () => { + const noAuthMiddleware = createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => mockFunctions, + async () => [], + mockAuth, + undefined, + mockLongPolling, + '/project', + mockLog, + ); + mockLoadModuleReturning(mockFunctions[0], () => 'pure result, no $.Actions call'); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + noAuthMiddleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(400); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(false); + expect(body.error).toContain('Auth credentials not configured'); + }); + + test('Should route a real $.Actions call (including connectionId) through a direct single-action preview-async query, not the jsFunctionWithActions wrapper', async () => { + const funcWithConnection: BackendFunction = { + ...mockFunctions[0], + allowedConnectionIds: ['conn-1'], + }; + const middlewareWithConnection = createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => [funcWithConnection, mockFunctions[1]], + async (entryId: string) => + entryId === funcWithConnection.absolutePath ? ['conn-1'] : [], + mockAuth, + testAuthenticatedRequest, + mockLongPolling, + '/project', + mockLog, + ); + mockLoadModuleReturning(funcWithConnection, () => + testDollarActions().slack.chat.postMessage({ + inputs: { text: 'hi' }, + connectionId: 'conn-1', + }), + ); + + type PreviewAsyncBody = { + data: { + attributes: { + query: { + properties: { + spec: { + fqn: string; + inputs: Record; + connectionId?: string; + }; + }; + }; + }; + }; + }; + let capturedBody: PreviewAsyncBody | undefined; + const apiScope = nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async', (body) => { + capturedBody = body as PreviewAsyncBody; + return true; + }) + .reply(200, { data: { id: 'receipt-action' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-action') + .reply(200, { + data: { attributes: { done: true, outputs: { ok: true, ts: '123' } } }, + }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(funcWithConnection), + args: [], + }); + const res = createMockResponse(); + + middlewareWithConnection(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.getBody()); + expect(body.success).toBe(true); + // The action's raw output ({ok, ts}) is what $.Actions.foo.bar() resolves to; the + // outer {data: ...} comes from executeScriptLocally's usual return-value wrapping, + // not anything action-specific. + expect(body.result).toEqual({ data: { ok: true, ts: '123' } }); + expect(apiScope.isDone()).toBe(true); + expect(capturedBody?.data.attributes.query.properties.spec).toEqual({ + fqn: 'com.datadoghq.slack.chat.postMessage', + inputs: { text: 'hi' }, + connectionId: 'conn-1', + }); + }); + + // makeExecuteActionRemotely must check connectionId with the same `!== undefined` + // strictness assertConnectionIdAllowed uses, not a truthy check that would drop an + // allowed empty string and send an unscoped call instead. + test('Should forward an empty-string connectionId to the preview-async query spec rather than silently dropping it', async () => { + const funcWithEmptyConnection: BackendFunction = { + ...mockFunctions[0], + allowedConnectionIds: [''], + }; + const middlewareWithEmptyConnection = createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => [funcWithEmptyConnection, mockFunctions[1]], + async (entryId: string) => + entryId === funcWithEmptyConnection.absolutePath ? [''] : [], + mockAuth, + testAuthenticatedRequest, + mockLongPolling, + '/project', + mockLog, + ); + mockLoadModuleReturning(funcWithEmptyConnection, () => + testDollarActions().slack.chat.postMessage({ + inputs: { text: 'hi' }, + connectionId: '', + }), + ); + + let capturedBody: + | { + data: { + attributes: { + query: { properties: { spec: { connectionId?: string } } }; + }; + }; + } + | undefined; + const apiScope = nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async', (body) => { + capturedBody = body as typeof capturedBody; + return true; + }) + .reply(200, { data: { id: 'receipt-empty-connection' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-empty-connection') + .reply(200, { + data: { attributes: { done: true, outputs: { ok: true } } }, + }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(funcWithEmptyConnection), + args: [], + }); + const res = createMockResponse(); + + middlewareWithEmptyConnection(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + expect(apiScope.isDone()).toBe(true); + expect(capturedBody?.data.attributes.query.properties.spec.connectionId).toBe(''); + }); + + test("Should surface a successful $.Actions call's result to the local console", async () => { + mockLoadModuleReturning(mockFunctions[0], () => + testDollarActions().slack.chat.postMessage({ + inputs: { text: 'hi' }, + }), + ); + + nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async') + .reply(200, { data: { id: 'receipt-success' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-success') + .reply(200, { + data: { attributes: { done: true, outputs: { ok: true } } }, + }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining('com.datadoghq.slack.chat.postMessage'), + 'info', + ); + expect(mockLogFn).toHaveBeenCalledWith(expect.stringContaining('"ok":true'), 'info'); + }); + + test("Should surface a failed $.Actions call's error detail to the local console", async () => { + mockLoadModuleReturning(mockFunctions[0], () => + testDollarActions().slack.chat.postMessage({ + inputs: { text: 'hi' }, + }), + ); + + nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async') + .reply(200, { data: { id: 'receipt-failure' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-failure') + .reply(200, { + errors: [{ detail: 'Connection is not authorized for this action' }], + }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(500); + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining('com.datadoghq.slack.chat.postMessage'), + 'error', + ); + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining('Connection is not authorized for this action'), + 'error', + ); + }); + + // The priming loadModule call (see handleExecuteAction) is the only place the entry's + // top-level code runs (Vite caches the module for executeScriptLocally's reuse), so it + // must carry the same $-scoping guarantee executeScriptLocally's own load would provide. + test('Should read $ as undefined when a customer module reaches for it during its own top-level evaluation, before runScriptLocally installs its own execution-scoped $', async () => { + let dollarDuringTopLevelLoad: unknown = 'not captured'; + mockLoadModule.mockImplementation(async (specifier: string) => { + if (specifier === mockFunctions[0].absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + dollarDuringTopLevelLoad = (globalThis as Record).$; + return { [mockFunctions[0].name]: () => 'done' }; + } + throw new Error(`Cannot find module '${specifier}'`); + }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + expect(dollarDuringTopLevelLoad).toBeUndefined(); + }); + + // The priming load runs before runScriptLocally's own hang-detection timeout is + // installed, so a hanging top-level await would otherwise wedge this request and every + // request queued behind it forever. + test('Should eventually time out and return a clear error when the priming load never settles', async () => { + jest.useFakeTimers(); + try { + mockLoadModule.mockImplementation( + // Never settles. + () => new Promise(() => {}), + ); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + middleware(req, res, jest.fn()); + const doneAssertion = res.done; + + // createMockRequest emits the body via a real process.nextTick, which fake timers + // don't advance — drain it first so the priming load's setTimeout is scheduled + // before runAllTimersAsync tries to advance past it. + await jest.advanceTimersByTimeAsync(0); + await jest.runAllTimersAsync(); + await doneAssertion; + + expect(res.statusCode).toBe(500); + const body = JSON.parse(res.getBody()); + expect(body.error).toMatch(/timed out after 10000ms/); + } finally { + jest.useRealTimers(); + } + }); + + // Priming evaluates real top-level customer code — if it ran outside executeColdActionLocally's + // enqueue() call, two concurrent requests for two different cold functions could evaluate + // their top-level code in genuine parallel instead of one fully finishing before the other starts. + test('Should never let two concurrent requests for different cold functions race their priming loads', async () => { + const order: string[] = []; + let releaseGreetPriming: (() => void) | undefined; + const greetPrimingGate = new Promise((resolve) => { + releaseGreetPriming = resolve; + }); + + mockLoadModule.mockImplementation(async (specifier: string) => { + if (specifier === mockFunctions[0].absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + order.push('greet-priming-start'); + await greetPrimingGate; + order.push('greet-priming-end'); + return { [mockFunctions[0].name]: () => 'greet-done' }; + } + if (specifier === mockFunctions[1].absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + order.push('compute-priming-start'); + order.push('compute-priming-end'); + return { [mockFunctions[1].name]: () => 'compute-done' }; + } + throw new Error(`Cannot find module '${specifier}'`); + }); + + const reqGreet = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const resGreet = createMockResponse(); + const reqCompute = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[1]), + args: [], + }); + const resCompute = createMockResponse(); + + // Fired back-to-back, before either request's own body has even finished parsing. + middleware(reqGreet, resGreet, jest.fn()); + middleware(reqCompute, resCompute, jest.fn()); + + // Give compute's request every chance to race ahead while greet's priming is gated — + // if it weren't serialized behind greet's still-pending turn, compute's ungated + // priming would already show up here, before greet's gate is ever released. + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(order).toEqual(['greet-priming-start']); + + releaseGreetPriming?.(); + await resGreet.done; + await resCompute.done; + + expect(order).toEqual([ + 'greet-priming-start', + 'greet-priming-end', + 'compute-priming-start', + 'compute-priming-end', + ]); + }); + + // Regression test: getAllowedConnectionIds (which only resolves/transforms modules, never + // executes them) must run and reject before the priming load ever evaluates the entry's + // real top-level code — otherwise a banned import there could run before being rejected. + test('Should never prime (evaluate) the entry when getAllowedConnectionIds rejects', async () => { + const primingCalls: string[] = []; + mockLoadModule.mockImplementation(async (specifier: string) => { + primingCalls.push(specifier); + return { [mockFunctions[0].name]: () => 'done' }; + }); + const rejectingMiddleware = createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => mockFunctions, + async () => { + throw new Error('Importing Node built-in module "fs" is not supported'); + }, + mockAuth, + testAuthenticatedRequest, + mockLongPolling, + '/project', + mockLog, + ); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + rejectingMiddleware(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(500); + expect(primingCalls).toEqual([]); + }); + + // Guards getAllowedConnectionIds — it reads and transforms every reachable module from + // disk (dev-server-module-graph.ts) with no bound of its own, unlike the sibling priming + // load next to it in executeColdActionLocally. + test('Should eventually time out and return a clear error when getAllowedConnectionIds never settles', async () => { + jest.useFakeTimers(); + try { + mockLoadModuleReturning(mockFunctions[0], () => 'done'); + const hangingMiddleware = createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => mockFunctions, + // Never settles. + () => new Promise(() => {}), + mockAuth, + testAuthenticatedRequest, + mockLongPolling, + '/project', + mockLog, + ); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + hangingMiddleware(req, res, jest.fn()); + const doneAssertion = res.done; + + await jest.advanceTimersByTimeAsync(0); + await jest.runAllTimersAsync(); + await doneAssertion; + + expect(res.statusCode).toBe(500); + const body = JSON.parse(res.getBody()); + expect(body.error).toMatch(/timed out after 10000ms/); + } finally { + jest.useRealTimers(); + } + }); + }); + describe('dynamic discovery', () => { test('Should not find stale function after re-transform (HMR)', async () => { let currentFunctions: BackendFunction[] = [...mockFunctions]; const middleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => currentFunctions, + async () => [], mockAuth, testAuthenticatedRequest, mockLongPolling, diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index 7ca6815a4..0383e1c94 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -20,6 +20,9 @@ import type { LongPollingOptions } from '../types'; import { createBackendConnectionIdCollector } from './backend-connection-id-collector'; import { createBackendStaticChecksPlugin } from './backend-static-checks-plugin'; import { getBaseBackendBuildConfig } from './build-config'; +import type { ExecuteAction, LoadModule } from './local-execution'; +import { DEFAULT_TIMEOUT_MS, executeColdActionLocally } from './local-execution'; +import { getMaxRetryDelayMs } from './retry-delay'; interface BundleResult { func: BackendFunction; @@ -33,11 +36,6 @@ const DEV_VIRTUAL_PREFIX = 'virtual:dd-backend-dev:'; type AuthConfig = AuthOptionsWithDefaults; type LongPollingConfig = Required; -// Kept small: `done: false` is healthy, so this delay is dead time. It only -// exists to de-synchronize concurrent pollers. -const RETRY_BASE_DELAY_MS = 250; -const RETRY_MAX_DELAY_MS = 2_000; - function delay(ms: number): Promise { return new Promise((resolve) => { setTimeout(resolve, ms); @@ -56,9 +54,7 @@ function isAbortError(error: unknown): boolean { // Equal jitter (half fixed, half random) so the delay keeps a floor. export function getRetryDelay(attempt: number, config: LongPollingConfig): number { - const backoffDelay = config.exponentialBackoff - ? Math.min(RETRY_BASE_DELAY_MS * 2 ** (attempt - 1), RETRY_MAX_DELAY_MS) - : RETRY_BASE_DELAY_MS; + const backoffDelay = getMaxRetryDelayMs(attempt, config.exponentialBackoff); return config.jitter ? backoffDelay / 2 + Math.random() * (backoffDelay / 2) : backoffDelay; } @@ -159,19 +155,19 @@ async function bundleBackendFunction( } /** - * Execute a script via Datadog's app-builder queries API. + * Submits a query to Datadog's `preview-async` endpoint and long-polls until it resolves, + * returning the raw `outputs`. `querySpec` is either the `jsFunctionWithActions` wrapper or + * a single action's `{fqn, inputs}` — `submitQuery` doesn't care which. */ -async function executeScriptViaDatadog( - scriptBody: string, - func: BackendFunction, - args: unknown[], +async function submitQuery( + querySpec: Record, + displayName: string, auth: AuthConfig, doAuthenticatedRequest: DoAuthenticatedRequest, longPolling: LongPollingConfig, log: Logger, -): Promise { +): Promise { const endpoint = `https://api.${auth.site}/api/v2/app-builder/queries/preview-async`; - const displayName = formatRef(func); log.debug(`Calling Datadog API: ${endpoint}`); @@ -184,14 +180,7 @@ async function executeScriptViaDatadog( name: displayName, type: 'action', properties: { - spec: { - fqn: 'com.datadoghq.datatransformation.jsFunctionWithActions', - inputs: { - script: scriptBody, - allowedConnectionIds: func.allowedConnectionIds, - context: { backendFunctionArgs: args }, - }, - }, + spec: querySpec, onlyTriggerManually: true, }, }, @@ -221,26 +210,92 @@ async function executeScriptViaDatadog( return pollQueryExecution(receiptId, auth, doAuthenticatedRequest, longPolling, log); } +/** Executes a script via Datadog's app-builder queries API — the production round trip, wrapping the whole script as a `jsFunctionWithActions` query. */ +async function executeScriptViaDatadog( + scriptBody: string, + func: BackendFunction, + args: unknown[], + auth: AuthConfig, + doAuthenticatedRequest: DoAuthenticatedRequest, + longPolling: LongPollingConfig, + log: Logger, +): Promise { + const displayName = formatRef(func); + + const outputs = await submitQuery( + { + fqn: 'com.datadoghq.datatransformation.jsFunctionWithActions', + inputs: { + script: scriptBody, + allowedConnectionIds: func.allowedConnectionIds, + context: { backendFunctionArgs: args }, + }, + }, + displayName, + auth, + doAuthenticatedRequest, + longPolling, + log, + ); + + if (typeof outputs !== 'object' || outputs === null || !('data' in outputs)) { + throw new Error('Query execution completed without a "data" field in its outputs'); + } + return outputs; +} + +/** Submits a single-action `preview-async` query per `$.Actions` call and logs its result/error, since production's equivalent signal never reaches the `npm run dev` console. Callers must have already confirmed auth is configured (see `createDevServerMiddleware`). */ +function makeExecuteActionRemotely( + auth: AuthConfig, + doAuthenticatedRequest: DoAuthenticatedRequest, + longPolling: LongPollingConfig, + log: Logger, +): ExecuteAction { + return async ( + fqn: string, + inputs: unknown, + connectionId: string | undefined, + ): Promise => { + try { + const result = await submitQuery( + connectionId !== undefined ? { fqn, inputs, connectionId } : { fqn, inputs }, + fqn, + auth, + doAuthenticatedRequest, + longPolling, + log, + ); + log.info(`$.Actions call to "${fqn}" succeeded: ${JSON.stringify(result)}`); + return result; + } catch (error: unknown) { + const message = error instanceof Error ? error.message : String(error); + log.error(`$.Actions call to "${fqn}" failed: ${message}`); + throw error; + } + }; +} + interface PollResult { - data?: { attributes?: { done?: boolean; outputs?: BackendOutputs } }; + data?: { attributes?: { done?: boolean; outputs?: unknown } }; errors?: Array<{ detail?: string; title?: string }>; } +/** + * Long-polls until a submitted query completes or times out, returning the raw `outputs` — + * shape varies by query type, so callers interpret it. The server holds each poll open ~30s + * and responds `done: false` on timeout; this loop only handles that re-polling, since + * `doRequest` already retries transient HTTP failures. + */ async function pollQueryExecution( receiptId: string, auth: AuthConfig, doAuthenticatedRequest: DoAuthenticatedRequest, longPolling: LongPollingConfig, log: Logger, -): Promise { +): Promise { const endpoint = `https://api.${auth.site}/api/v2/app-builder/queries/execution-long-polling/${receiptId}`; const { maxRetries, timeoutMs } = longPolling; - /* - * The server holds each request open (~30s) and answers `done: false` when its - * window expires, so we re-poll. This is not an HTTP retry loop: doRequest - * already retries transient failures. `maxRetries: 1` disables re-polling. - */ for (let attempt = 0; attempt < maxRetries; attempt++) { if (attempt > 0) { const retryDelay = getRetryDelay(attempt, longPolling); @@ -277,7 +332,7 @@ async function pollQueryExecution( log.debug(`Long-poll response, done: ${attrs?.done}`); if (attrs?.done) { - if (!attrs.outputs) { + if (attrs.outputs === undefined || attrs.outputs === null) { throw new Error('Query execution completed without outputs'); } return attrs.outputs; @@ -298,6 +353,42 @@ function sendError(res: ServerResponse, statusCode: number, message: string): vo res.end(JSON.stringify({ success: false, error: message } satisfies ExecuteActionResponse)); } +/** + * Send a JSON success response. + */ +function sendSuccess(res: ServerResponse, result: { data: unknown }): void { + res.statusCode = 200; + res.setHeader('Content-Type', 'application/json'); + res.end(JSON.stringify({ success: true, result } satisfies ExecuteActionResponse)); +} + +/** + * Runs `run` only once auth is configured, matching production's auth-before-execution + * ordering — checked upfront, not lazily inside a $.Actions call, so a function that never + * calls $.Actions isn't a loophole. A local presence check only, adding no latency. + */ +function guardAuthenticated( + res: ServerResponse, + doAuthenticatedRequest: DoAuthenticatedRequest | undefined, + run: (doAuthenticatedRequest: DoAuthenticatedRequest) => Promise, +): void { + if (!doAuthenticatedRequest) { + sendError(res, 400, `Auth credentials not configured. ${AUTH_GUIDANCE}`); + return; + } + run(doAuthenticatedRequest).catch(() => sendError(res, 500, 'Unexpected error')); +} + +/** Shared catch-block shape for every handler below: an `HttpError` carries its own status code, anything else is a 500. `label` is omitted for handlers with no `log` in scope. */ +function handleHttpError(res: ServerResponse, error: unknown, log?: Logger, label?: string): void { + const statusCode = error instanceof HttpError ? error.statusCode : 500; + const message = error instanceof Error ? error.message : 'Internal server error'; + if (log && label) { + log.debug(`Error handling ${label}: ${message}`); + } + sendError(res, statusCode, message); +} + class HttpError extends Error { constructor( public statusCode: number, @@ -308,14 +399,13 @@ class HttpError extends Error { } /** - * Shared request pipeline: parse body, validate functionName, look up - * the backend function by encoded query name, and bundle it. + * Split out from `validateAndBundle` so `handleExecuteAction`'s no-bundling local path can + * reuse the same parse-and-lookup step without pulling in a bundle. */ -async function validateAndBundle( +async function parseAndLookupFunction( req: IncomingMessage, functionsByName: Map, - bundle: BundleFn, -): Promise<{ func: BackendFunction; code: string; args: unknown[] }> { +): Promise<{ func: BackendFunction; args: unknown[] }> { const { functionName, args = [] } = await parseRequestBody(req); if (!functionName || typeof functionName !== 'string') { @@ -327,6 +417,20 @@ async function validateAndBundle( throw new HttpError(404, `Backend function "${functionName}" not found`); } + return { func, args }; +} + +/** + * Shared by `handleDebugBundle` and `handleExecuteActionViaCloud` — the two handlers that + * still need a bundle; `handleExecuteAction`'s no-bundling path calls `parseAndLookupFunction` + * directly instead. + */ +async function validateAndBundle( + req: IncomingMessage, + functionsByName: Map, + bundle: BundleFn, +): Promise<{ func: BackendFunction; code: string; args: unknown[] }> { + const { func, args } = await parseAndLookupFunction(req, functionsByName); const bundled = await bundle(func); return { ...bundled, args }; } @@ -347,16 +451,65 @@ async function handleDebugBundle( res.setHeader('Content-Type', 'text/plain'); res.end(code); } catch (error: unknown) { - const statusCode = error instanceof HttpError ? error.statusCode : 500; - const message = error instanceof Error ? error.message : 'Internal server error'; - sendError(res, statusCode, message); + handleHttpError(res, error); } } /** - * Handle POST /__dd/executeAction — bundles a backend function and executes it via Datadog API. + * Handles POST /__dd/executeAction — imports a backend function's real file and executes + * it in-process (see local-execution.ts), with no bundling. Auth is checked upfront by the + * caller in `createDevServerMiddleware`, matching production's auth-before-execution ordering. */ async function handleExecuteAction( + req: IncomingMessage, + res: ServerResponse, + functionsByName: Map, + auth: AuthConfig, + doAuthenticatedRequest: DoAuthenticatedRequest, + longPolling: LongPollingConfig, + loadModule: LoadModule, + getAllowedConnectionIds: (entryId: string) => Promise, + projectRoot: string, + log: Logger, +): Promise { + try { + const { func, args } = await parseAndLookupFunction(req, functionsByName); + const displayName = formatRef(func); + + log.debug(`Executing action locally: ${displayName} with args`); + + // Priming and connection-ID collection must happen inside the same serialization + // boundary as execution — see executeColdActionLocally's doc comment for why. + const executeAction = makeExecuteActionRemotely( + auth, + doAuthenticatedRequest, + longPolling, + log, + ); + const result = await executeColdActionLocally( + func, + projectRoot, + args, + executeAction, + loadModule, + getAllowedConnectionIds, + log, + DEFAULT_TIMEOUT_MS, + longPolling, + ); + + sendSuccess(res, result); + } catch (error: unknown) { + handleHttpError(res, error, log, 'executeAction'); + } +} + +/** + * Handles POST /__dd/executeActionViaCloud — bundles a backend function and executes it via + * the production round trip (queue + Deno subprocess), kept as its own endpoint + * (`npm run dev:verify`) for pre-publish parity checks rather than a mode flag. + */ +async function handleExecuteActionViaCloud( req: IncomingMessage, res: ServerResponse, functionsByName: Map, @@ -370,7 +523,7 @@ async function handleExecuteAction( const { func, code, args } = await validateAndBundle(req, functionsByName, bundle); const displayName = formatRef(func); - log.debug(`Executing action: ${displayName} with args`); + log.debug(`Executing action via cloud: ${displayName} with args`); const result = await executeScriptViaDatadog( code, @@ -382,14 +535,9 @@ async function handleExecuteAction( log, ); - res.statusCode = 200; - res.setHeader('Content-Type', 'application/json'); - res.end(JSON.stringify({ success: true, result } satisfies ExecuteActionResponse)); + sendSuccess(res, result); } catch (error: unknown) { - const statusCode = error instanceof HttpError ? error.statusCode : 500; - const message = error instanceof Error ? error.message : 'Internal server error'; - log.debug(`Error handling executeAction: ${message}`); - sendError(res, statusCode, message); + handleHttpError(res, error, log, 'executeActionViaCloud'); } } @@ -401,16 +549,15 @@ function buildFunctionMap(backendFunctions: BackendFunction[]): Map BackendFunction[], + getAllowedConnectionIds: (entryId: string) => Promise, auth: AuthConfig, doAuthenticatedRequest: DoAuthenticatedRequest | undefined, longPolling: LongPollingConfig, @@ -440,22 +587,33 @@ export function createDevServerMiddleware( sendError(res, 500, 'Unexpected error'); }); } else if (req.url === '/__dd/executeAction') { - if (!doAuthenticatedRequest) { - sendError(res, 400, `Auth credentials not configured. ${AUTH_GUIDANCE}`); - return; - } - handleExecuteAction( - req, - res, - functionsByName, - bundle, - auth, - doAuthenticatedRequest, - longPolling, - log, - ).catch(() => { - sendError(res, 500, 'Unexpected error'); - }); + guardAuthenticated(res, doAuthenticatedRequest, (authedRequest) => + handleExecuteAction( + req, + res, + functionsByName, + auth, + authedRequest, + longPolling, + loadModule, + getAllowedConnectionIds, + projectRoot, + log, + ), + ); + } else if (req.url === '/__dd/executeActionViaCloud') { + guardAuthenticated(res, doAuthenticatedRequest, (authedRequest) => + handleExecuteActionViaCloud( + req, + res, + functionsByName, + bundle, + auth, + authedRequest, + longPolling, + log, + ), + ); } else { next(); } diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 62d218d59..ee8028800 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -4,6 +4,7 @@ import { getVitePlugin } from '@dd/apps-plugin/vite/index'; import type { ViteBundler } from '@dd/apps-plugin/vite/index'; +import { localExecutionResolutionContext } from '@dd/apps-plugin/vite/local-execution'; import { InjectPosition } from '@dd/core/types'; import { getContextMock, getRepositoryDataMock, mockLogFn } from '@dd/tests/_jest/helpers/mocks'; import { parseAst } from 'rollup/parseAst'; @@ -75,6 +76,20 @@ const functions: BackendFunction[] = [ const bundleName1 = encodeQueryName(functions[0]); const bundleName2 = encodeQueryName(functions[1]); +/** Narrows a Vite plugin's `resolveId` hook to its full-object form (`{ handler, ... }`) so tests can call it directly. */ +function getResolveIdHandler(plugin: ReturnType): Function { + const resolveId = plugin?.resolveId; + if ( + typeof resolveId !== 'object' || + resolveId === null || + !('handler' in resolveId) || + typeof resolveId.handler !== 'function' + ) { + throw new Error('Expected plugin.resolveId to be an object with a handler function.'); + } + return resolveId.handler; +} + const mockViteBuild = jest.fn(); const mockVite = { build: mockViteBuild, @@ -210,7 +225,10 @@ describe('Backend Functions - getVitePlugin', () => { // Only middleware registration is exercised here; a full ViteDevServer // is not needed, so cast a minimal stand-in at this library boundary. - const server = { middlewares: { use: jest.fn() } } as unknown as ViteDevServer; + const server = { + middlewares: { use: jest.fn() }, + ssrLoadModule: jest.fn(), + } as unknown as ViteDevServer; // The hooks are typed with Rollup's `this: PluginContext`, but the // plugin closures never read `this`, so a stand-in satisfies the call. const thisArg = {} as unknown as PluginContext; @@ -224,6 +242,57 @@ describe('Backend Functions - getVitePlugin', () => { expect(buildPackage.buildAppPackage).not.toHaveBeenCalled(); }); + // Regression test: this warning previously lived in createDevServerMiddleware itself: moved + // here since configureServer is where auth is actually resolved (or fails to be). + test('Should warn that both executeAction endpoints will be unavailable when no auth is configured', () => { + const plugin = getVitePlugin(defaultOptions); + if (!plugin || Array.isArray(plugin)) { + throw new Error('Expected getVitePlugin to return a single Vite plugin'); + } + if (typeof plugin.configureServer !== 'function') { + throw new Error('Expected a configureServer hook on the plugin'); + } + + const server = { + middlewares: { use: jest.fn() }, + ssrLoadModule: jest.fn(), + } as unknown as ViteDevServer; + + plugin.configureServer(server); + + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining( + 'Both the /__dd/executeAction and /__dd/executeActionViaCloud endpoints will be unavailable', + ), + 'warn', + ); + }); + + // Regression: the negative case for the warning test above, previously covered by + // dev-server.test.ts's own deleted 'startup auth warning' describe block. + test('Should not warn about missing authentication when it is configured', () => { + jest.spyOn(auth, 'getAuthenticatedRequest').mockReturnValue(jest.fn()); + const plugin = getVitePlugin(defaultOptions); + if (!plugin || Array.isArray(plugin)) { + throw new Error('Expected getVitePlugin to return a single Vite plugin'); + } + if (typeof plugin.configureServer !== 'function') { + throw new Error('Expected a configureServer hook on the plugin'); + } + + const server = { + middlewares: { use: jest.fn() }, + ssrLoadModule: jest.fn(), + } as unknown as ViteDevServer; + + plugin.configureServer(server); + + expect(mockLogFn).not.toHaveBeenCalledWith( + expect.stringContaining('No authentication configured'), + 'warn', + ); + }); + test('does not resolve authentication during production packaging', async () => { const authSpy = jest.spyOn(auth, 'getAuthenticatedRequest'); const plugin = getVitePlugin(defaultOptions); @@ -429,11 +498,9 @@ describe('Backend Functions - getVitePlugin', () => { '/build/src/backend/myHandler.backend.ts', ); - // An unrelated query-bearing import of the SAME file with zero exports (not `export - // default` — Vite's own `?raw`/`?url`/`?worker` load hooks all produce a default export, - // which this file's static checks already reject with a loud throw before this branch is - // ever reached; this covers whatever else might legitimately produce no named exports - // without throwing). + // A query-bearing import of the same file with zero exports, not `export default` — + // Vite's own `?raw`/`?url`/`?worker` hooks produce a default export, already rejected + // elsewhere, so this covers whatever else could legitimately have no named exports. await handler.call( { parse: parseAst, @@ -452,6 +519,101 @@ describe('Backend Functions - getVitePlugin', () => { expect(mockViteBuild).toHaveBeenCalledTimes(1); }); + describe('resolveId suffix propagation through a plain helper module', () => { + const entryFile = '/build/src/backend/entry.backend.ts'; + const helperImporter = '/build/src/helper.ts'; + const nestedBackendFile = '/build/src/backend/otherHandler.backend.ts'; + + /** Resolves the entry's own `./helper` import, marking `helperImporter` as part of whichever subgraph tracking Set (if any) is active on the AsyncLocalStorage store at call time — the same first hop a real local execution's traversal makes. */ + const resolveEntryToHelper = (resolveIdHandler: Function) => + resolveIdHandler.call( + { resolve: jest.fn(async () => ({ id: helperImporter })) }, + './helper', + `${entryFile}${LOCAL_EXECUTION_LOAD_SUFFIX}`, + { ssr: true }, + ); + + /** Resolves a nested backend import from the helper — the second hop that should only inherit the suffix if `helperImporter` is still recognized as part of the current subgraph. */ + const resolveHelperToBackendFile = (resolveIdHandler: Function) => + resolveIdHandler.call( + { resolve: jest.fn(async () => ({ id: nestedBackendFile })) }, + './otherHandler.backend', + helperImporter, + { ssr: true }, + ); + + test('Should give the helper itself a suffixed identity, distinct from an ordinary (unsuffixed) resolution of the same file', async () => { + const plugin = getVitePlugin(defaultOptions); + const resolveIdHandler = getResolveIdHandler(plugin); + + const result = await localExecutionResolutionContext.run(new Set(), () => + resolveEntryToHelper(resolveIdHandler), + ); + + expect((result as { id: string } | null)?.id).toBe( + `${helperImporter}${LOCAL_EXECUTION_LOAD_SUFFIX}`, + ); + }); + + test('Should propagate the suffix onto a nested backend import reached through a helper resolved earlier in the same local execution', async () => { + const plugin = getVitePlugin(defaultOptions); + const resolveIdHandler = getResolveIdHandler(plugin); + + const result = await localExecutionResolutionContext.run(new Set(), async () => { + // Chains the first hop's actual returned id into the second call's importer, the + // same suffixed identity a real nested import from this helper would present. + const { id: suffixedHelperImporter } = (await resolveEntryToHelper( + resolveIdHandler, + )) as { id: string }; + return resolveIdHandler.call( + { resolve: jest.fn(async () => ({ id: nestedBackendFile })) }, + './otherHandler.backend', + suffixedHelperImporter, + { ssr: true }, + ); + }); + + expect((result as { id: string } | null)?.id).toBe( + `${nestedBackendFile}${LOCAL_EXECUTION_LOAD_SUFFIX}`, + ); + }); + + // A plain module-level Set (instead of one scoped per execution via AsyncLocalStorage) + // would still recognize `helperImporter` here, serving real backend code into what + // should be an ordinary, unrelated SSR resolution of the same helper. + test('Should NOT propagate the suffix onto the same helper importer once no local execution is in flight, even though an earlier execution already traversed it', async () => { + const plugin = getVitePlugin(defaultOptions); + const resolveIdHandler = getResolveIdHandler(plugin); + + // A prior, now-finished local execution traverses entry -> helper. + await localExecutionResolutionContext.run(new Set(), () => + resolveEntryToHelper(resolveIdHandler), + ); + + // Later, unrelated SSR resolution of the same helper importer — outside any local + // execution's own load. + const result = await resolveHelperToBackendFile(resolveIdHandler); + + expect(result).toBeNull(); + }); + + // The importer-suffix branch must be ssr-scoped too, or a client-mode resolution using + // an SSR-only suffixed id as importer would inherit the marker and leak real backend code. + test('Should NOT propagate the suffix through a suffixed importer when the resolution is not SSR', async () => { + const plugin = getVitePlugin(defaultOptions); + const resolveIdHandler = getResolveIdHandler(plugin); + + const result = await resolveIdHandler.call( + { resolve: jest.fn(async () => ({ id: nestedBackendFile })) }, + './otherHandler.backend', + `${entryFile}${LOCAL_EXECUTION_LOAD_SUFFIX}`, + { ssr: false }, + ); + + expect(result).toBeNull(); + }); + }); + test('Should inject the apps runtime', () => { getVitePlugin(defaultOptions); @@ -461,4 +623,20 @@ describe('Backend Functions - getVitePlugin', () => { value: expect.stringMatching(/[/\\]apps-runtime\.mjs$/), }); }); + + test('Should force @datadog/apps-backend and @datadog/action-catalog through the SSR transform pipeline instead of externalizing them', () => { + // These SDKs ship ESM-only, but Vite's dev-server SSR mode externalizes node_modules by + // default (a plain require()), which throws "Cannot use import statement outside a + // module" for them — ssr.noExternal is what server.ssrLoadModule depends on to load them + // correctly. + const plugin = getVitePlugin(defaultOptions); + const configHook = plugin!.config as () => { ssr: { noExternal: string[] } }; + const config = configHook(); + + expect(config).toEqual({ + ssr: { + noExternal: ['@datadog/apps-backend', '@datadog/action-catalog'], + }, + }); + }); }); diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 4b4e26a22..84fd5fb46 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -15,6 +15,8 @@ import { type DoAuthenticatedRequest, } from '../auth'; import { extractExportedFunctions } from '../backend/ast-parsing/extract-backend-functions'; +import { extractConnectionIdsFromModuleGraph } from '../backend/ast-parsing/extract-connection-ids-from-module-graph'; +import { shouldTraverseCollectedModule } from '../backend/ast-parsing/module-graph'; import { analyzeModuleScope } from '../backend/ast-parsing/module-scope'; import { runBackendStaticChecks } from '../backend/ast-parsing/run-backend-static-checks'; import { ensureProgram } from '../backend/ast-parsing/type-guards'; @@ -31,7 +33,9 @@ import type { AppsOptionsWithDefaults } from '../types'; import { buildBackendFunctions } from './build-backend-functions'; import { buildAppPackage } from './build-package'; +import { collectModuleGraphFromServer } from './dev-server-module-graph'; import { createDevServerMiddleware } from './dev-server'; +import { localExecutionResolutionContext } from './local-execution'; export type ViteBundler = { build: typeof build; @@ -130,6 +134,70 @@ export const getVitePlugin = ({ let devServerActive = false; return { + // @datadog/apps-backend and @datadog/action-catalog ship ESM-only, but ssrLoadModule + // externalizes node_modules by default (a plain require()), which throws "Cannot use + // import statement outside a module" — ssr.noExternal forces Vite's SSR transform instead. + config() { + return { + ssr: { + noExternal: ['@datadog/apps-backend', '@datadog/action-catalog'], + }, + }; + }, + // Propagates LOCAL_EXECUTION_LOAD_SUFFIX through the backend-file dependency graph so a + // nested `.backend.ts` import isn't replaced with the frontend proxy stub. Every subgraph + // module gets its own suffixed id, since Vite otherwise shares one cached id across callers. + resolveId: { + // Must run before Vite's built-in resolver ('pre'): a plain relative specifier like + // `./other.backend` is otherwise fully resolved by Vite's own filesystem resolution + // first, short-circuiting the hook chain before this plugin ever sees it. + order: 'pre', + async handler(source, importer, resolveOptions) { + // Top-level guard (not folded into each branch) so any future branch added below + // inherits it automatically: local execution's traversal is always SSR, so without + // this a client-mode resolution could inherit the marker and leak real backend code. + if (resolveOptions.ssr !== true) { + return null; + } + + // The other half of the scoping: the store is only populated while a local + // execution's own loadModule call is in flight (see configureServer), so an + // unrelated SSR resolution never inherits a marker from an earlier execution. + const subgraphImporters = localExecutionResolutionContext.getStore(); + const isPartOfSuffixedSubgraph = + !!importer && + (importer.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX) || + (!!subgraphImporters && subgraphImporters.has(importer))); + if (!isPartOfSuffixedSubgraph) { + return null; + } + + const resolved = await this.resolve(source, importer, { + ...resolveOptions, + skipSelf: true, + }); + if (!resolved || resolved.external) { + return resolved; + } + + if (resolved.id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX)) { + return resolved; + } + + // Only app-local source gets a distinct local-execution identity — an SDK/package + // import must resolve to the same module Vite otherwise caches for it, since an + // unrecognized query on a node_modules id can break Vite's optimizeDeps handling. + if (!shouldTraverseCollectedModule(resolved.id, context.buildRoot)) { + return resolved; + } + + const suffixedId = resolved.id + LOCAL_EXECUTION_LOAD_SUFFIX; + if (!BACKEND_FILE_RE.test(resolved.id)) { + subgraphImporters?.add(suffixedId); + } + return { ...resolved, id: suffixedId }; + }, + }, transform: { filter: { id: { @@ -230,21 +298,35 @@ export const getVitePlugin = ({ throw error; } log.warn( - `No authentication configured. The /__dd/executeAction endpoint will be unavailable. ${AUTH_GUIDANCE}`, + `No authentication configured. Both the /__dd/executeAction and /__dd/executeActionViaCloud endpoints will be unavailable. ${AUTH_GUIDANCE}`, ); } - server.middlewares.use( - createDevServerMiddleware( - bundler.build, - getBackendFunctions, - auth, - doAuthenticatedRequest, - options.longPolling, + const loadModule = server.ssrLoadModule.bind(server); + // Safe to call before `loadModule` runs anything: collectModuleGraphFromServer primes + // each node itself via `transformRequest`, since `moduleParsed` (production's + // mechanism) is Rollup-build-only and never fires on a real dev server. + const getAllowedConnectionIds = async (entryId: string) => { + const moduleGraph = await collectModuleGraphFromServer( + server, + entryId, context.buildRoot, log, - ), + ); + return extractConnectionIdsFromModuleGraph(entryId, moduleGraph, context.buildRoot); + }; + const middleware = createDevServerMiddleware( + bundler.build, + loadModule, + getBackendFunctions, + getAllowedConnectionIds, + auth, + doAuthenticatedRequest, + options.longPolling, + context.buildRoot, + log, ); + server.middlewares.use(middleware); }, }; }; diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index 1be4470d8..c5b9f6b58 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -4,14 +4,18 @@ /* global globalThis, NodeJS */ -import { mockLogFn, mockLogger } from '@dd/tests/_jest/helpers/mocks'; +import { mockLogFn, mockLogger, moduleResolverFor } from '@dd/tests/_jest/helpers/mocks'; import * as shared from '../backend/shared'; import type { BackendFunction } from '../backend/types'; import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; import type { ExecuteAction, LoadModule } from './local-execution'; -import { executeScriptLocally } from './local-execution'; +import { + DEFAULT_LONG_POLLING_CONFIG, + deriveActionTimeouts, + executeScriptLocally, +} from './local-execution'; const func: BackendFunction = { relativePath: 'src/example', @@ -46,14 +50,7 @@ const stubExecuteAction: ExecuteAction = async (fqn) => ({ data: null, stub: tru /** A `loadModule` double that resolves the customer's function from a map and rejects anything else with a module-not-found error, matching the common case where neither optional package is installed. */ function loadModuleReturning(exports: Record): LoadModule { - return async (specifier: string) => { - if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { - return exports; - } - const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); - error.code = 'MODULE_NOT_FOUND'; - throw error; - }; + return moduleResolverFor(func, exports); } const ORDER_MARKER = '__ddLocalExecutionTestOrder'; @@ -545,6 +542,136 @@ describe('local-execution — executeScriptLocally', () => { ); }); + // Stands in for makeExecuteActionRemotely's long-poll, which can legitimately outlast a + // short hang-detection timeout — that's network wait, not a hung function. + test('Should not time out while a real $.Actions call is still legitimately in flight, even past the configured timeout', async () => { + const slowExecuteAction: ExecuteAction = () => + new Promise((resolve) => setTimeout(() => resolve({ ok: true }), 80)); + + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + slowExecuteAction, + loadModuleReturning({ + example: () => + testDollar().Actions.slack.chat.postMessage({ + inputs: { text: 'hi' }, + }), + }), + mockLogger, + // Shorter than slowExecuteAction's own 80ms. + 50, + ); + + expect(result).toEqual({ data: { ok: true } }); + }); + + // Without a bound on the $.Actions call itself, a stalled request would wedge this + // execution and every request queued behind it via `enqueue` indefinitely. + test('Should eventually time out an in-flight $.Actions call that never settles, and not wedge subsequently queued executions', async () => { + jest.useFakeTimers(); + try { + const neverSettlingExecuteAction: ExecuteAction = () => new Promise(() => {}); + + const hungExecution = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + neverSettlingExecuteAction, + loadModuleReturning({ + example: () => + testDollar().Actions.slack.chat.postMessage({ + inputs: { text: 'hi' }, + }), + }), + mockLogger, + 50, + ); + // Enqueued behind hungExecution — if the fix didn't bound the + // stalled $.Actions call, this would never get a turn either. + const queuedNext = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => 'next' }), + mockLogger, + ); + + const { totalExecutionTimeoutMs } = deriveActionTimeouts(DEFAULT_LONG_POLLING_CONFIG); + const hungAssertion = expect(hungExecution).rejects.toThrow( + new RegExp(`exceeded the absolute ${totalExecutionTimeoutMs}ms execution ceiling`), + ); + + await jest.runAllTimersAsync(); + await hungAssertion; + + expect(await queuedNext).toEqual({ data: 'next' }); + } finally { + jest.useRealTimers(); + } + }); + + // A fire-and-forget $.Actions call pauses the per-call hang-detection timer for as long as it + // stays in flight (up to the derived per-call ceiling), even though the customer function has + // moved on — the absolute execution ceiling below must still fire well before that. + test('Should eventually time out via an absolute execution ceiling, independent of any $.Actions call still in flight', async () => { + jest.useFakeTimers(); + try { + const neverSettlingExecuteAction: ExecuteAction = () => new Promise(() => {}); + + const execution = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + neverSettlingExecuteAction, + loadModuleReturning({ + example: () => + testDollar().Actions.slack.chat.postMessage({ + inputs: { text: 'hi' }, + }), + }), + mockLogger, + 50, + ); + + const { totalExecutionTimeoutMs } = deriveActionTimeouts(DEFAULT_LONG_POLLING_CONFIG); + const assertion = expect(execution).rejects.toThrow( + new RegExp(`exceeded the absolute ${totalExecutionTimeoutMs}ms execution ceiling`), + ); + + await jest.advanceTimersByTimeAsync(totalExecutionTimeoutMs); + await assertion; + } finally { + jest.useRealTimers(); + } + }); + + test('Should still time out a function that hangs with no $.Actions call in flight, even after an earlier call in the same run completed', async () => { + const executeAction: ExecuteAction = async () => ({ ok: true }); + + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: async () => { + await testDollar().Actions.slack.chat.postMessage({ + inputs: { text: 'hi' }, + }); + // Hangs with no further $.Actions call — the fresh timeout window from the completed call above must still expire normally. + return new Promise(() => {}); + }, + }), + mockLogger, + 50, + ), + ).rejects.toThrow(/timed out after 50ms/); + }); + // Asserts $'s exact key set, since a token added inside globalThis.$ wouldn't be caught by the weaker top-level check below. test('Should never expose an auth token to the customer module — only backendFunctionArgs, Actions, and Source are visible on globalThis.$', async () => { const result = await executeScriptLocally( @@ -799,23 +926,17 @@ describe('local-execution — executeScriptLocally', () => { expect(registeredImpl).toBeDefined(); }); - // The happy-path counterpart to the "shared loadModule with a never-settling load" test below: proves the plain success case is deduped too, not just the failure/eviction paths. - test('Should load the action-catalog module only once across two successful executions that share the same loadModule', async () => { + test('Should reuse the cached action-catalog registration across executions that share the same loadModule reference, even when each passes a different primedEntry — matching dev-server.ts, which threads one stable loadModule but a fresh per-request primed module', async () => { jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); let actionCatalogLoadCount = 0; const loadModule: LoadModule = async (specifier: string) => { - if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { - return { example: () => 'ok' }; - } if (specifier === '@datadog/action-catalog/action-execution') { actionCatalogLoadCount += 1; return { setExecuteActionImplementation: () => {} }; } - const notFoundError: NodeJS.ErrnoException = new Error( - `Cannot find module '${specifier}'`, - ); - notFoundError.code = 'MODULE_NOT_FOUND'; - throw notFoundError; + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; }; const first = await executeScriptLocally( @@ -825,6 +946,8 @@ describe('local-execution — executeScriptLocally', () => { stubExecuteAction, loadModule, mockLogger, + undefined, + { example: () => 'first' }, ); const second = await executeScriptLocally( func, @@ -833,10 +956,12 @@ describe('local-execution — executeScriptLocally', () => { stubExecuteAction, loadModule, mockLogger, + undefined, + { example: () => 'second' }, ); - expect(first).toEqual({ data: 'ok' }); - expect(second).toEqual({ data: 'ok' }); + expect(first).toEqual({ data: 'first' }); + expect(second).toEqual({ data: 'second' }); expect(actionCatalogLoadCount).toBe(1); }); diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index 7a8dde201..42e7efdd1 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -12,8 +12,10 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import { isActionCatalogInstalled, isDatadogAppsBackendInstalled } from '../backend/shared'; import type { BackendFunction, BackendOutputs } from '../backend/types'; import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; +import type { LongPollingOptions } from '../types'; import { createEpochGuard } from './execution-epoch'; +import { getTotalRetryDelayBudgetMs } from './retry-delay'; type BackendGlobals = { backendFunctionArgs: unknown[]; @@ -33,6 +35,9 @@ const hadPreexistingDollar = Reflect.has(globalThis, '$'); /** Marks the window where a customer module's own top-level code is loading, narrower than "no execution box on the call stack" (also true between executions, where the undefined-returning fallback below is correct). Carries its own mutable box so a top-level `$` write (e.g. `zx/globals`) lands scoped to this module's own load, not the shared `globalDollarOutsideExecution` slot a later, unrelated load would also read from. */ const customerModuleLoadContext = new AsyncLocalStorage<{ assigned: boolean; value: unknown }>(); +/** Scopes vite/index.ts's suffixed-subgraph tracking (see its `resolveId` hook) to one entry's own module-graph traversal, run alongside `customerModuleLoadContext` below — every caller that loads a customer entry, real dev-server request or test harness alike, funnels through `loadCustomerModuleEntry`, so scoping here (rather than wherever a particular `loadModule` happens to be constructed) reaches every path uniformly. Without this, a single process-wide Set would let a helper module reached by one local execution's traversal stay marked for the dev server's whole lifetime, so a later unrelated SSR resolution of the same helper would inherit the marker and serve real backend code instead of the frontend RPC-proxy stub. */ +export const localExecutionResolutionContext = new AsyncLocalStorage>(); + /** Backs `globalThis.$` outside any execution box (e.g. this module's own import-time state); seeded from any `$` already installed before this module loaded so the accessor below doesn't discard a legitimate `zx/globals`-style passthrough. */ let globalDollarOutsideExecution: unknown = Reflect.get(globalThis, '$'); @@ -61,10 +66,8 @@ function dollarGetter(): unknown { if (hadPreexistingDollar) { return globalDollarOutsideExecution; } - // Matches production: $ isn't a global property at all until main() assigns it, so an - // unresolvable `$` reads as undefined rather than throwing (per typeof's spec-defined - // behavior on unresolvable references) — returning undefined here keeps that true even - // though $ is a real accessor property locally, not a genuinely absent one. + // Matches production: $ isn't global until main() assigns it, so it reads as undefined + // rather than throwing, even though it's a real accessor property here, not absent. return undefined; } return globalDollarOutsideExecution; @@ -110,11 +113,58 @@ function isIndexableRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null; } -const DEFAULT_TIMEOUT_MS = 10_000; +export const DEFAULT_TIMEOUT_MS = 10_000; + +type LongPollingConfig = Required; + +// Matches validate.ts's resolveLongPolling defaults, so callers get the same effective ceilings +// a real dev server derives without each passing one in. Exported so tests assert the derived +// value, not a hardcoded copy. +export const DEFAULT_LONG_POLLING_CONFIG: LongPollingConfig = { + maxRetries: 10, + timeoutMs: 40_000, + jitter: true, + exponentialBackoff: true, +}; + +/** + * Both ceilings must exceed `pollQueryExecution`'s worst-case budget: polling time + * (`maxRetries * timeoutMs`) plus the caller-configurable, unbounded retry delays. Derived from + * the real config and the shared retry-delay budget so the two can't drift apart; exported so + * tests compute the expected value instead of hardcoding a copy. + */ +export function deriveActionTimeouts(longPolling: LongPollingConfig): { + actionCallTimeoutMs: number; + totalExecutionTimeoutMs: number; +} { + const worstCaseMs = + longPolling.maxRetries * longPolling.timeoutMs + getTotalRetryDelayBudgetMs(longPolling); + return { + // Bounds a single $.Actions call, exempt from the hang-detection timer — doRequest has + // no deadline of its own, so an unsettled call would wedge the whole serialized queue. + actionCallTimeoutMs: worstCaseMs * 2, + // Absolute wall-clock ceiling, independent of the pause-and-extend mechanism above — + // that can't tell a genuinely slow $.Actions call from a fire-and-forgot one masking a + // real hang, so this bounds the masked case tighter without cutting off legitimate calls. + totalExecutionTimeoutMs: worstCaseMs * 1.2, + }; +} /** Loads a module by specifier, resolved against the customer's own project rather than build-plugins' dependency tree — the dev server passes its Vite instance's `ssrLoadModule` here. */ export type LoadModule = (specifier: string) => Promise>; +/** Loads a customer module under the same top-level-evaluation `$`-scoping `runScriptLocally` uses (see `customerModuleLoadContext`) — for callers like dev-server.ts's priming load that trigger real top-level evaluation ahead of `executeScriptLocally`. */ +export function loadCustomerModuleEntry( + loadModule: LoadModule, + entrySpecifier: string, +): Promise> { + return localExecutionResolutionContext.run(new Set(), () => + customerModuleLoadContext.run({ assigned: false, value: undefined }, () => + loadModule(entrySpecifier), + ), + ); +} + /** Executes a real `$.Actions.foo.bar(...)` call; the dev server supplies the implementation using its own auth, so this module never holds or sees a credential itself. */ export type ExecuteAction = ( fqn: string, @@ -169,7 +219,7 @@ function enqueue(run: () => Promise): Promise { return result; } -// Shared wording for the "no longer current" rejection at every call site that checks execution abandonment (a direct $.Actions call, the action-catalog dispatcher, and the apps-backend accessor) — a concluded scope stays concluded forever, not just "not the latest", so refusing to act under its identity applies uniformly regardless of entry point. +// Shared wording for the "no longer current" rejection at every abandonment call site. function abandonedExecutionError(functionName: string, refusedAction: string): Error { return new Error( `Execution of "${functionName}" already concluded; refusing to ${refusedAction} ` + @@ -177,7 +227,12 @@ function abandonedExecutionError(functionName: string, refusedAction: string): E ); } -/** One shared guard across all executions — `enqueue` only serializes each execution's *start*; a timed-out `fn()` keeps running afterward (see "abandoned, not canceled" below). `isCurrent()`'s cross-scope generation comparison is what rejects that zombie's later `$.Actions` dispatch, once a newer scope has taken over. Each scope's own `concludeIfCurrent()` is a separate, narrower guard: it only clears the shared generation if THIS scope is still the one active, so a scope's delayed cleanup can never clobber a newer scope that has already superseded it. */ +/** + * One shared guard across all executions — `enqueue` only serializes each execution's start, + * so a timed-out `fn()` keeps running (abandoned, not canceled). `isCurrent()` rejects that + * zombie's later dispatch once a newer scope takes over; `concludeIfCurrent()` only clears the + * generation if its own scope is still active, so delayed cleanup can't clobber a newer scope. + */ const executionEpoch = createEpochGuard(); /** Resolves a nested property path (e.g. $.Actions.slack.chat.postMessage) to a callable that invokes `executeAction` directly — no IPC needed since there's no separate process to cross. */ @@ -188,7 +243,8 @@ function makeActionsProxy( ): unknown { return new Proxy(function () {}, { get(_target, prop) { - // An un-invoked $.Actions.foo.bar reference must not be mistaken for a thenable (Promise probes .then()) or serializable (assertJsonSerializable probes .toJSON()) — either probe hitting apply() below would hang or leak a rejection instead of a clear error. + // Must not look thenable/serializable — Promise/assertJsonSerializable probes for + // .then()/.toJSON() would otherwise hit apply() below and hang or leak a rejection. if (prop === 'then' || prop === 'toJSON') { return undefined; } @@ -211,11 +267,11 @@ function makeActionsProxy( }); } -/** Bounds a registration's `loadModule` call so a load that never settles (a broken/circular module graph) rejects instead of leaving its cache entry pending forever — eviction-on-rejection below only fires once a promise settles. Can't cancel the underlying promise, so a load that eventually settles still runs its side effects late; see the registration functions for why that's harmless. */ -function withTimeout(promise: Promise, timeoutMs: number, what: string): Promise { +/** Bounds a promise that could otherwise hang forever — a `loadModule` call against a broken/circular graph, or a `$.Actions` call with no deadline of its own — rejecting instead of leaving the caller waiting indefinitely. Doesn't cancel the underlying promise (not possible for a plain `Promise`), so late side effects can still fire if it eventually settles; see each call site for why that's harmless there. `label` is the full, already-attributed subject of the timeout message (e.g. `` `Loading ${specifier}` ``), not a suffix on a fixed prefix, so it reads naturally for both loads and action calls. */ +export function withTimeout(promise: Promise, timeoutMs: number, label: string): Promise { return new Promise((resolve, reject) => { const timer = setTimeout(() => { - reject(new Error(`Loading ${what} timed out after ${timeoutMs}ms`)); + reject(new Error(`${label} timed out after ${timeoutMs}ms`)); }, timeoutMs); promise.then( (value) => { @@ -278,7 +334,7 @@ async function registerActionCatalogOnce(loadModule: LoadModule, timeoutMs: numb const mod = await withTimeout( loadPromise, timeoutMs, - '@datadog/action-catalog/action-execution', + 'Loading @datadog/action-catalog/action-execution', ); const setExecuteActionImplementation = mod.setExecuteActionImplementation; if (typeof setExecuteActionImplementation !== 'function') { @@ -332,7 +388,7 @@ async function registerBackendRuntimeOnce( const [jsFunctionWithActionsModule, runtimeModule] = await withTimeout( loadPromise, timeoutMs, - '@datadog/apps-backend/runtime', + 'Loading @datadog/apps-backend/runtime', ); const buildRuntimeFromJsFunctionWithActions = jsFunctionWithActionsModule.buildRuntimeFromJsFunctionWithActions; @@ -403,7 +459,9 @@ function assertJsonSerializable(result: unknown, func: BackendFunction): unknown } let serialized: string | undefined; try { - // A replacer visits every key/value pair including the root, so a disallowed value nested arbitrarily deep is caught the same way a top-level one is, instead of JSON.stringify silently flattening/converting/dropping it. The root is excluded from the function/Symbol/undefined check below (handled separately via `serialized === undefined`) and tracked with a one-shot flag, not `key === ''`, since a real property can itself be named `''`. + // A replacer visits every key/value pair including the root, catching a disallowed value + // at any depth. Root is tracked via a one-shot flag, not `key === ''`, since a real + // property can itself be named `''`. let isRootCall = true; serialized = JSON.stringify(result, (key, value) => { const wasRootCall = isRootCall; @@ -450,7 +508,12 @@ function assertJsonSerializable(result: unknown, func: BackendFunction): unknown return JSON.parse(serialized); } -/** `globalThis.$` and the action-catalog/apps-backend registrations above provide the same customer-visible bindings production's generated wrapper module sets up via text injection; serialized via `enqueue`. */ +/** + * Test-only entry point: exercises `runScriptLocally`'s queue/execution behavior with priming + * already done via `primedEntry`. Production always goes through `executeColdActionLocally`, + * which primes inside the same `enqueue()` call instead — kept separate since folding priming + * in here would change what `primedEntry` means for the ~90 tests calling this directly. + */ export async function executeScriptLocally( func: BackendFunction, projectRoot: string, @@ -459,12 +522,73 @@ export async function executeScriptLocally( loadModule: LoadModule, log: Logger, timeoutMs: number = DEFAULT_TIMEOUT_MS, + primedEntry?: Record, + longPolling: LongPollingConfig = DEFAULT_LONG_POLLING_CONFIG, ): Promise { return enqueue(() => - runScriptLocally(func, projectRoot, args, executeAction, loadModule, log, timeoutMs), + runScriptLocally( + func, + projectRoot, + args, + executeAction, + loadModule, + log, + timeoutMs, + primedEntry, + longPolling, + ), ); } +/** + * Cold-function entry point: collects `allowedConnectionIds`, then primes and runs the entry in + * one `enqueue()` call, since priming runs real top-level code and doing it outside the queue + * would let two cold functions run in parallel. Connection IDs are collected first (never + * executes code), rejecting a banned import before the entry runs; `withTimeout` doesn't cancel. + */ +export async function executeColdActionLocally( + func: BackendFunction, + projectRoot: string, + args: unknown[], + executeAction: ExecuteAction, + loadModule: LoadModule, + getAllowedConnectionIds: (entryId: string) => Promise, + log: Logger, + timeoutMs: number = DEFAULT_TIMEOUT_MS, + longPolling: LongPollingConfig = DEFAULT_LONG_POLLING_CONFIG, +): Promise { + const displayName = `${func.relativePath}/${func.name}`; + return enqueue(async () => { + // Each step needs its own timeout bound independent of runScriptLocally's, which only + // starts once its body begins. + const connectionIdsPromise = getAllowedConnectionIds(func.absolutePath); + const allowedConnectionIds = await withTimeout( + connectionIdsPromise, + timeoutMs, + `Resolving allowed connections for "${displayName}"`, + ); + const entrySpecifier = func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX; + const primingPromise = loadCustomerModuleEntry(loadModule, entrySpecifier); + const primedEntry = await withTimeout( + primingPromise, + timeoutMs, + `Loading "${displayName}"`, + ); + // Calls runScriptLocally directly, not executeScriptLocally, to avoid enqueueing twice. + return runScriptLocally( + { ...func, allowedConnectionIds }, + projectRoot, + args, + executeAction, + loadModule, + log, + timeoutMs, + primedEntry, + longPolling, + ); + }); +} + async function runScriptLocally( func: BackendFunction, projectRoot: string, @@ -473,19 +597,55 @@ async function runScriptLocally( loadModule: LoadModule, log: Logger, timeoutMs: number, + primedEntry: Record | undefined, + longPolling: LongPollingConfig, ): Promise { // Never log the args themselves — they may carry secrets/PII, matching dev-server.ts's cloud path. log.debug(`Executing "${func.name}" in-process with args`); - // A timed-out execution is abandoned, not canceled — its fn() may keep running and must not act under a newer execution's identity. isCurrent() gates both this execution's own captured `$.Actions` closure and the shared adapters, which resolve the calling execution's dispatch from AsyncLocalStorage rather than whichever registration is currently live. + const { actionCallTimeoutMs, totalExecutionTimeoutMs } = deriveActionTimeouts(longPolling); + + // A timed-out execution is abandoned, not canceled — its fn() may keep running, so isCurrent() + // gates both its $.Actions closure and the shared adapters against acting under a stale identity. const scope = executionEpoch.start(); - const guardedExecuteAction: ExecuteAction = (fqn, inputs, connectionId) => { + // A long-poll can legitimately outlast timeoutMs (network wait, not a hang) — pausing the + // timer while a call is in flight and restarting it once all settle keeps real progress from + // being penalized while a genuine hang still times out normally. + let timer: ReturnType | undefined; + let rejectTimeout: ((error: Error) => void) | undefined; + let pendingActionCalls = 0; + + const scheduleTimeout = () => { + timer = setTimeout(() => { + concludeExecution(); + rejectTimeout?.( + new Error(`Local execution of "${func.name}" timed out after ${timeoutMs}ms`), + ); + }, timeoutMs); + }; + + const guardedExecuteAction: ExecuteAction = async (fqn, inputs, connectionId) => { if (!scope.isCurrent()) { - // A concluded scope stays concluded forever, not just "not the latest" — the wording stays conclusion-neutral rather than claiming a timeout that may not have happened. - return Promise.reject(abandonedExecutionError(func.name, `run "${fqn}"`)); + // Wording stays conclusion-neutral, not "timed out" — a concluded scope may have + // ended for another reason. + throw abandonedExecutionError(func.name, `run "${fqn}"`); + } + pendingActionCalls += 1; + clearTimeout(timer); + try { + const actionCallPromise = executeAction(fqn, inputs, connectionId); + return await withTimeout( + actionCallPromise, + actionCallTimeoutMs, + `$.Actions call to "${fqn}"`, + ); + } finally { + pendingActionCalls -= 1; + if (pendingActionCalls === 0 && scope.isCurrent()) { + scheduleTimeout(); + } } - return executeAction(fqn, inputs, connectionId); }; const concludeExecution = () => { @@ -507,13 +667,18 @@ async function runScriptLocally( }; const run = async (): Promise => { - // Wraps the whole body, not just the customer-function call below, so a failure while loading/resolving the module (e.g. loadModule rejecting, or the export not being a function) also concludes the scope — otherwise the epoch guard's cross-scope supersession never runs for this execution, leaving activeGeneration pinned to it until the next start() overwrites it. + // Wraps the whole body so a module-load failure also concludes the scope — otherwise + // activeGeneration stays pinned to this execution until the next start() overwrites it. try { - // Loads and evaluates the customer's module BEFORE installing $ and the SDK bridges below, matching production's own ordering (backend/virtual-entry.ts statically imports the customer module before its wrapper installs $ and the SDK bridges) — code that reaches for $ or a typed action during its own top-level evaluation fails the same way locally as it would in Datadog, instead of silently succeeding against bindings production wouldn't have installed yet. - const mod = await customerModuleLoadContext.run( - { assigned: false, value: undefined }, - () => loadModule(func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX), - ); + // Loads the module before installing $ and the SDK bridges, matching production's + // ordering. A caller that already primed this entry passes the resolved module + // directly, keeping loadModule's identity stable for the registration caches below. + const mod = + primedEntry ?? + (await loadCustomerModuleEntry( + loadModule, + func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX, + )); const fn = mod[func.name]; if (typeof fn !== 'function') { throw new Error( @@ -521,7 +686,8 @@ async function runScriptLocally( ); } - // Reinstalls the accessor if a prior execution's customer code deleted globalThis.$, so this execution's box stays reachable. Only closes the gap between executions — a deletion made mid-flight by a still-running concurrent execution can't be recovered, since there's no way to intercept access on a since-deleted global property; that narrower case is accepted as-is. + // Reinstalls the accessor if a prior execution's code deleted globalThis.$ — only + // closes the gap between executions, not a mid-flight deletion by a concurrent one. ensureDollarAccessorInstalled(); // Scopes globalThis.$ and the dispatch info to this call's own async continuation chain. @@ -556,19 +722,29 @@ async function runScriptLocally( } }; - let timer: ReturnType | undefined; const timeout = new Promise((_resolve, reject) => { - timer = setTimeout(() => { - concludeExecution(); - reject(new Error(`Local execution of "${func.name}" timed out after ${timeoutMs}ms`)); - }, timeoutMs); + rejectTimeout = reject; + scheduleTimeout(); }); - // Racing the timeout only stops the caller from waiting — run() keeps executing afterward, so a resumed customer function can still fire real $.Actions side effects; true cancellation would need a Worker thread, not possible in-process. + // Fires regardless of pendingActionCalls, unlike the timeout above — bounds a fire-and-forget + // call masking a hang to totalExecutionTimeoutMs instead of the per-call ceiling. + const absoluteTimeoutTimer = setTimeout(() => { + concludeExecution(); + rejectTimeout?.( + new Error( + `Local execution of "${func.name}" exceeded the absolute ${totalExecutionTimeoutMs}ms execution ceiling, regardless of any $.Actions call in flight.`, + ), + ); + }, totalExecutionTimeoutMs); + + // Racing the timeout only stops the caller from waiting — run() keeps executing afterward, + // since true cancellation would require terminating a Worker thread. const runPromise = run(); - // Set once the race settles, so the handler below can tell an abandoned rejection (caller already gone) from an ordinary one the caller is about to receive normally. + // Lets the handler below tell an abandoned rejection from an ordinary one. let raceSettled = false; - // Nothing awaits runPromise once the timeout wins the race, so a later rejection would otherwise crash the dev server as unhandled — logged instead so a slow real failure stays diagnosable. + // Nothing awaits runPromise once the timeout wins, so a later rejection would otherwise + // crash as unhandled — logged instead so a slow real failure stays diagnosable. runPromise.catch((error: unknown) => { if (!raceSettled) { return; @@ -582,5 +758,6 @@ async function runScriptLocally( } finally { raceSettled = true; clearTimeout(timer); + clearTimeout(absoluteTimeoutTimer); } } diff --git a/packages/plugins/apps/src/vite/retry-delay.ts b/packages/plugins/apps/src/vite/retry-delay.ts new file mode 100644 index 000000000..5b18ac2f7 --- /dev/null +++ b/packages/plugins/apps/src/vite/retry-delay.ts @@ -0,0 +1,28 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import type { LongPollingOptions } from '../types'; + +type LongPollingConfig = Required; + +// Kept small: `done: false` is healthy, so this delay is dead time. It only +// exists to de-synchronize concurrent pollers. +export const RETRY_BASE_DELAY_MS = 250; +export const RETRY_MAX_DELAY_MS = 2_000; + +/** Deterministic upper bound for one retry delay, ignoring jitter's random reduction — dev-server.ts's `getRetryDelay` only ever reduces toward this ceiling, never past it, so it also bounds the worst-case budget below. */ +export function getMaxRetryDelayMs(attempt: number, exponentialBackoff: boolean): number { + return exponentialBackoff + ? Math.min(RETRY_BASE_DELAY_MS * 2 ** (attempt - 1), RETRY_MAX_DELAY_MS) + : RETRY_BASE_DELAY_MS; +} + +/** Worst-case total time `pollQueryExecution` spends waiting between attempts — shared so `deriveActionTimeouts`'s ceilings can't drift from the delays it actually waits out. */ +export function getTotalRetryDelayBudgetMs(config: LongPollingConfig): number { + let totalMs = 0; + for (let attempt = 1; attempt < config.maxRetries; attempt++) { + totalMs += getMaxRetryDelayMs(attempt, config.exponentialBackoff); + } + return totalMs; +} diff --git a/packages/published/esbuild-plugin/package.json b/packages/published/esbuild-plugin/package.json index f6f94bcca..19d645369 100644 --- a/packages/published/esbuild-plugin/package.json +++ b/packages/published/esbuild-plugin/package.json @@ -54,6 +54,7 @@ "@jridgewell/remapping": "2.3.5", "async-retry": "1.3.3", "chalk": "2.3.1", + "esbuild": "0.25.8", "eslint-scope": "7.2.2", "glob": "11.1.0", "json-stream-stringify": "3.1.6", @@ -62,6 +63,7 @@ "outdent": "0.8.0", "p-queue": "6.6.2", "pretty-bytes": "5.6.0", + "rollup": "4.45.1", "simple-git": "3.36.0", "unplugin": "2.3.11" }, diff --git a/packages/published/rollup-plugin/package.json b/packages/published/rollup-plugin/package.json index e05b2da6c..db71afe0e 100644 --- a/packages/published/rollup-plugin/package.json +++ b/packages/published/rollup-plugin/package.json @@ -57,6 +57,7 @@ "@jridgewell/remapping": "2.3.5", "async-retry": "1.3.3", "chalk": "2.3.1", + "esbuild": "0.25.8", "eslint-scope": "7.2.2", "glob": "11.1.0", "json-stream-stringify": "3.1.6", @@ -65,6 +66,7 @@ "outdent": "0.8.0", "p-queue": "6.6.2", "pretty-bytes": "5.6.0", + "rollup": "4.45.1", "simple-git": "3.36.0", "unplugin": "2.3.11" }, diff --git a/packages/published/rspack-plugin/package.json b/packages/published/rspack-plugin/package.json index c28a4a7fd..d4e92ecba 100644 --- a/packages/published/rspack-plugin/package.json +++ b/packages/published/rspack-plugin/package.json @@ -54,6 +54,7 @@ "@jridgewell/remapping": "2.3.5", "async-retry": "1.3.3", "chalk": "2.3.1", + "esbuild": "0.25.8", "eslint-scope": "7.2.2", "glob": "11.1.0", "json-stream-stringify": "3.1.6", @@ -62,6 +63,7 @@ "outdent": "0.8.0", "p-queue": "6.6.2", "pretty-bytes": "5.6.0", + "rollup": "4.45.1", "simple-git": "3.36.0", "unplugin": "2.3.11" }, diff --git a/packages/published/vite-plugin/package.json b/packages/published/vite-plugin/package.json index 140574643..1149aae11 100644 --- a/packages/published/vite-plugin/package.json +++ b/packages/published/vite-plugin/package.json @@ -1,105 +1,107 @@ { - "name": "@datadog/vite-plugin", - "packageManager": "yarn@4.0.2", - "version": "3.2.12", - "license": "MIT", - "author": "Datadog", - "description": "Datadog Vite Plugin", - "keywords": [ - "datadog", - "vite", - "bundler", - "plugin", - "unplugin" - ], - "homepage": "https://github.com/DataDog/build-plugins#readme", - "repository": { - "type": "git", - "url": "https://github.com/DataDog/build-plugins", - "directory": "packages/published/vite-plugin" - }, - "main": "./dist/src/index.js", - "module": "./dist/src/index.mjs", - "types": "./dist/src/index.d.ts", - "exports": { - "./dist/src": "./dist/src/index.js", - "./dist/src/*": "./dist/src/*", - ".": "./src/index.ts" - }, - "publishConfig": { - "access": "public", + "name": "@datadog/vite-plugin", + "packageManager": "yarn@4.0.2", + "version": "3.2.12", + "license": "MIT", + "author": "Datadog", + "description": "Datadog Vite Plugin", + "keywords": [ + "datadog", + "vite", + "bundler", + "plugin", + "unplugin" + ], + "homepage": "https://github.com/DataDog/build-plugins#readme", + "repository": { + "type": "git", + "url": "https://github.com/DataDog/build-plugins", + "directory": "packages/published/vite-plugin" + }, + "main": "./dist/src/index.js", + "module": "./dist/src/index.mjs", "types": "./dist/src/index.d.ts", "exports": { - "./package.json": "./package.json", - ".": { - "import": "./dist/src/index.mjs", - "require": "./dist/src/index.js", - "types": "./dist/src/index.d.ts" - } - } - }, - "files": [ - "dist" - ], - "scripts": { - "buildCmd": "rollup --config rollup.config.mjs", - "build": "yarn clean && yarn buildCmd", - "clean": "rm -rf dist", - "prepack": "yarn build", - "typecheck": "tsc --noEmit", - "watch": "yarn build --watch" - }, - "dependencies": { - "@datadog/js-instrumentation-wasm": "1.0.8", - "@jridgewell/remapping": "2.3.5", - "async-retry": "1.3.3", - "chalk": "2.3.1", - "eslint-scope": "7.2.2", - "glob": "11.1.0", - "json-stream-stringify": "3.1.6", - "jszip": "3.10.1", - "magic-string": "0.30.21", - "outdent": "0.8.0", - "p-queue": "6.6.2", - "pretty-bytes": "5.6.0", - "simple-git": "3.36.0", - "unplugin": "2.3.11" - }, - "devDependencies": { - "@babel/core": "7.24.5", - "@babel/preset-env": "7.24.5", - "@babel/preset-typescript": "7.24.1", - "@dd/factory": "workspace:*", - "@dd/tools": "workspace:*", - "@rollup/plugin-babel": "6.0.4", - "@rollup/plugin-commonjs": "28.0.1", - "@rollup/plugin-esm-shim": "0.1.8", - "@rollup/plugin-json": "6.1.0", - "@rollup/plugin-node-resolve": "15.3.0", - "@rollup/plugin-terser": "0.4.4", - "@types/babel__core": "^7", - "@types/babel__preset-env": "^7", - "dts-bundle-generator": "patch:dts-bundle-generator@npm%3A9.5.1#~/.yarn/patches/dts-bundle-generator-npm-9.5.1-0927b6826f.patch", - "esbuild": "0.25.8", - "rollup": "4.45.1", - "rollup-plugin-esbuild": "6.1.1", - "typescript": "5.4.3" - }, - "peerDependencies": { - "@babel/parser": "^7.24.5", - "@babel/traverse": "^7.24.5", - "@babel/types": "^7.24.5", - "vite": ">= 5.x <= 8.x" - }, - "peerDependenciesMeta": { - "@babel/parser": { - "optional": true + "./dist/src": "./dist/src/index.js", + "./dist/src/*": "./dist/src/*", + ".": "./src/index.ts" + }, + "publishConfig": { + "access": "public", + "types": "./dist/src/index.d.ts", + "exports": { + "./package.json": "./package.json", + ".": { + "import": "./dist/src/index.mjs", + "require": "./dist/src/index.js", + "types": "./dist/src/index.d.ts" + } + } + }, + "files": [ + "dist" + ], + "scripts": { + "buildCmd": "rollup --config rollup.config.mjs", + "build": "yarn clean && yarn buildCmd", + "clean": "rm -rf dist", + "prepack": "yarn build", + "typecheck": "tsc --noEmit", + "watch": "yarn build --watch" + }, + "dependencies": { + "@datadog/js-instrumentation-wasm": "1.0.8", + "@jridgewell/remapping": "2.3.5", + "async-retry": "1.3.3", + "chalk": "2.3.1", + "esbuild": "0.25.8", + "eslint-scope": "7.2.2", + "glob": "11.1.0", + "json-stream-stringify": "3.1.6", + "jszip": "3.10.1", + "magic-string": "0.30.21", + "outdent": "0.8.0", + "p-queue": "6.6.2", + "pretty-bytes": "5.6.0", + "rollup": "4.45.1", + "simple-git": "3.36.0", + "unplugin": "2.3.11" + }, + "devDependencies": { + "@babel/core": "7.24.5", + "@babel/preset-env": "7.24.5", + "@babel/preset-typescript": "7.24.1", + "@dd/factory": "workspace:*", + "@dd/tools": "workspace:*", + "@rollup/plugin-babel": "6.0.4", + "@rollup/plugin-commonjs": "28.0.1", + "@rollup/plugin-esm-shim": "0.1.8", + "@rollup/plugin-json": "6.1.0", + "@rollup/plugin-node-resolve": "15.3.0", + "@rollup/plugin-terser": "0.4.4", + "@types/babel__core": "^7", + "@types/babel__preset-env": "^7", + "dts-bundle-generator": "patch:dts-bundle-generator@npm%3A9.5.1#~/.yarn/patches/dts-bundle-generator-npm-9.5.1-0927b6826f.patch", + "esbuild": "0.25.8", + "rollup": "4.45.1", + "rollup-plugin-esbuild": "6.1.1", + "typescript": "5.4.3" }, - "@babel/traverse": { - "optional": true + "peerDependencies": { + "@babel/parser": "^7.24.5", + "@babel/traverse": "^7.24.5", + "@babel/types": "^7.24.5", + "vite": ">= 5.x <= 8.x" }, - "@babel/types": { - "optional": true + "peerDependenciesMeta": { + "@babel/parser": { + "optional": true + }, + "@babel/traverse": { + "optional": true + }, + "@babel/types": { + "optional": true + } } - } } diff --git a/packages/published/webpack-plugin/package.json b/packages/published/webpack-plugin/package.json index 554365551..8faf9cb9a 100644 --- a/packages/published/webpack-plugin/package.json +++ b/packages/published/webpack-plugin/package.json @@ -54,6 +54,7 @@ "@jridgewell/remapping": "2.3.5", "async-retry": "1.3.3", "chalk": "2.3.1", + "esbuild": "0.25.8", "eslint-scope": "7.2.2", "glob": "11.1.0", "json-stream-stringify": "3.1.6", @@ -62,6 +63,7 @@ "outdent": "0.8.0", "p-queue": "6.6.2", "pretty-bytes": "5.6.0", + "rollup": "4.45.1", "simple-git": "3.36.0", "unplugin": "2.3.11" }, diff --git a/packages/tests/src/_jest/fixtures/action_catalog_project/action-execution.js b/packages/tests/src/_jest/fixtures/action_catalog_project/action-execution.js new file mode 100644 index 000000000..326bafb85 --- /dev/null +++ b/packages/tests/src/_jest/fixtures/action_catalog_project/action-execution.js @@ -0,0 +1,13 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +let implementation; + +export function setExecuteActionImplementation(fn) { + implementation = fn; +} + +export function getExecuteActionImplementation() { + return implementation; +} diff --git a/packages/tests/src/_jest/fixtures/action_catalog_project/index.js b/packages/tests/src/_jest/fixtures/action_catalog_project/index.js new file mode 100644 index 000000000..6521f8c3a --- /dev/null +++ b/packages/tests/src/_jest/fixtures/action_catalog_project/index.js @@ -0,0 +1,15 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import { getExecuteActionImplementation } from './action-execution.js'; + +export async function sendSlackMessage(request) { + const implementation = getExecuteActionImplementation(); + if (!implementation) { + throw new Error( + '@datadog/action-catalog fixture: no execute-action implementation registered', + ); + } + return implementation('com.datadoghq.slack.chat.postMessage', request); +} diff --git a/packages/tests/src/_jest/fixtures/action_catalog_project/package.json b/packages/tests/src/_jest/fixtures/action_catalog_project/package.json new file mode 100644 index 000000000..678e69045 --- /dev/null +++ b/packages/tests/src/_jest/fixtures/action_catalog_project/package.json @@ -0,0 +1,14 @@ +{ + "name": "@datadog/action-catalog", + "version": "0.0.1", + "private": true, + "license": "MIT", + "author": "Datadog", + "type": "module", + "description": "Minimal local fixture standing in for the real @datadog/action-catalog package — only the pieces dev-server.integration.test.ts's connection-ID coverage actually exercises.", + "main": "index.js", + "exports": { + ".": "./index.js", + "./action-execution": "./action-execution.js" + } +} diff --git a/packages/tests/src/_jest/fixtures/apps_backend_project/actionCatalogCall.backend.ts b/packages/tests/src/_jest/fixtures/apps_backend_project/actionCatalogCall.backend.ts new file mode 100644 index 000000000..2528464d9 --- /dev/null +++ b/packages/tests/src/_jest/fixtures/apps_backend_project/actionCatalogCall.backend.ts @@ -0,0 +1,9 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import { sendSlackMessage } from '@datadog/action-catalog'; + +export async function postMessage() { + return sendSlackMessage({ inputs: { text: 'hi' }, connectionId: 'conn-1' }); +} diff --git a/packages/tests/src/_jest/fixtures/apps_backend_project/helper.ts b/packages/tests/src/_jest/fixtures/apps_backend_project/helper.ts new file mode 100644 index 000000000..0159acabd --- /dev/null +++ b/packages/tests/src/_jest/fixtures/apps_backend_project/helper.ts @@ -0,0 +1,9 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import { plainEcho } from './getRuntimeUsers.backend'; + +export async function helperEcho(value: string) { + return plainEcho(value); +} diff --git a/packages/tests/src/_jest/fixtures/apps_backend_project/helperWithBannedImport.ts b/packages/tests/src/_jest/fixtures/apps_backend_project/helperWithBannedImport.ts new file mode 100644 index 000000000..578a219a4 --- /dev/null +++ b/packages/tests/src/_jest/fixtures/apps_backend_project/helperWithBannedImport.ts @@ -0,0 +1,9 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import fs from 'fs'; + +export function readSomething() { + return fs.readFileSync('/etc/hosts', 'utf8'); +} diff --git a/packages/tests/src/_jest/fixtures/apps_backend_project/mixedImports.backend.ts b/packages/tests/src/_jest/fixtures/apps_backend_project/mixedImports.backend.ts new file mode 100644 index 000000000..0976f93f2 --- /dev/null +++ b/packages/tests/src/_jest/fixtures/apps_backend_project/mixedImports.backend.ts @@ -0,0 +1,16 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import { helperEcho } from './helper'; + +// A non-local (npm-package) dynamic import: a local one is already caught by the +// fail-closed unsupportedDependencies check, which would mask the bug this exercises. +await import('chalk'); + +import { sendSlackMessage } from '@datadog/action-catalog'; + +export async function usesMixedImports(value: string) { + await helperEcho(value); + return sendSlackMessage({ inputs: { text: value }, connectionId: 'conn-1' }); +} diff --git a/packages/tests/src/_jest/fixtures/apps_backend_project/nestedImport.backend.ts b/packages/tests/src/_jest/fixtures/apps_backend_project/nestedImport.backend.ts new file mode 100644 index 000000000..8fa36d344 --- /dev/null +++ b/packages/tests/src/_jest/fixtures/apps_backend_project/nestedImport.backend.ts @@ -0,0 +1,9 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import { plainEcho } from './getRuntimeUsers.backend'; + +export async function usesNestedImport(value: string) { + return plainEcho(value); +} diff --git a/packages/tests/src/_jest/fixtures/apps_backend_project/package.json b/packages/tests/src/_jest/fixtures/apps_backend_project/package.json index 988be3f0a..fabae5e0f 100644 --- a/packages/tests/src/_jest/fixtures/apps_backend_project/package.json +++ b/packages/tests/src/_jest/fixtures/apps_backend_project/package.json @@ -5,6 +5,7 @@ "author": "Datadog", "packageManager": "yarn@4.2.1", "dependencies": { + "@datadog/action-catalog": "portal:../action_catalog_project", "@datadog/apps-backend": "0.0.1" } } diff --git a/packages/tests/src/_jest/fixtures/apps_backend_project/viaBannedHelper.backend.ts b/packages/tests/src/_jest/fixtures/apps_backend_project/viaBannedHelper.backend.ts new file mode 100644 index 000000000..01c38f99a --- /dev/null +++ b/packages/tests/src/_jest/fixtures/apps_backend_project/viaBannedHelper.backend.ts @@ -0,0 +1,9 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import { readSomething } from './helperWithBannedImport'; + +export async function usesBannedHelper() { + return readSomething(); +} diff --git a/packages/tests/src/_jest/fixtures/apps_backend_project/viaHelper.backend.ts b/packages/tests/src/_jest/fixtures/apps_backend_project/viaHelper.backend.ts new file mode 100644 index 000000000..7eef9489c --- /dev/null +++ b/packages/tests/src/_jest/fixtures/apps_backend_project/viaHelper.backend.ts @@ -0,0 +1,9 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import { helperEcho } from './helper'; + +export async function usesHelper(value: string) { + return helperEcho(value); +} diff --git a/packages/tests/src/_jest/fixtures/package.json b/packages/tests/src/_jest/fixtures/package.json index df591e7ce..72bf7254e 100644 --- a/packages/tests/src/_jest/fixtures/package.json +++ b/packages/tests/src/_jest/fixtures/package.json @@ -7,6 +7,7 @@ "workspaces": [ "hard_project", "easy_project", - "apps_backend_project" + "apps_backend_project", + "action_catalog_project" ] } diff --git a/packages/tests/src/_jest/fixtures/yarn.lock b/packages/tests/src/_jest/fixtures/yarn.lock index 0a81c424b..79a91d93e 100644 --- a/packages/tests/src/_jest/fixtures/yarn.lock +++ b/packages/tests/src/_jest/fixtures/yarn.lock @@ -5,6 +5,18 @@ __metadata: version: 8 cacheKey: 10 +"@datadog/action-catalog@portal:../action_catalog_project::locator=%40tests%2Fapps_backend_project%40workspace%3Aapps_backend_project": + version: 0.0.0-use.local + resolution: "@datadog/action-catalog@portal:../action_catalog_project::locator=%40tests%2Fapps_backend_project%40workspace%3Aapps_backend_project" + languageName: node + linkType: soft + +"@datadog/action-catalog@workspace:action_catalog_project": + version: 0.0.0-use.local + resolution: "@datadog/action-catalog@workspace:action_catalog_project" + languageName: unknown + linkType: soft + "@datadog/apps-backend@npm:0.0.1": version: 0.0.1 resolution: "@datadog/apps-backend@npm:0.0.1" @@ -23,6 +35,7 @@ __metadata: version: 0.0.0-use.local resolution: "@tests/apps_backend_project@workspace:apps_backend_project" dependencies: + "@datadog/action-catalog": "portal:../action_catalog_project" "@datadog/apps-backend": "npm:0.0.1" languageName: unknown linkType: soft diff --git a/packages/tests/src/_jest/helpers/mocks.ts b/packages/tests/src/_jest/helpers/mocks.ts index 907dfda97..cfa8c30e0 100644 --- a/packages/tests/src/_jest/helpers/mocks.ts +++ b/packages/tests/src/_jest/helpers/mocks.ts @@ -2,6 +2,11 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +/* global NodeJS */ + +import type { BackendFunction } from '@dd/apps-plugin/backend/types'; +import { LOCAL_EXECUTION_LOAD_SUFFIX } from '@dd/apps-plugin/constants'; +import type { LoadModule } from '@dd/apps-plugin/vite/local-execution'; import { DEFAULT_SITE } from '@dd/core/constants'; import { checkFile, @@ -44,7 +49,9 @@ import type { Compilation, Module, MetricsOptions } from '@dd/metrics-plugin/typ import { File } from 'buffer'; import type { PluginBuild, Metafile } from 'esbuild'; import esbuild from 'esbuild'; +import { EventEmitter } from 'events'; import type { PathLike, Stats } from 'fs'; +import type { IncomingMessage, ServerResponse } from 'http'; import path from 'path'; import { getTempWorkingDir } from './env'; @@ -120,6 +127,68 @@ export const getMockTimeLogger = (overrides: Partial = {}): TimeLogg return mockTimer; }; +/** + * Builds a `loadModule`-shaped resolver returning `exports` for `func`'s suffixed absolute + * path (see `LOCAL_EXECUTION_LOAD_SUFFIX`) and rejecting any other specifier, matching a real + * loader when only the target module is resolvable. + */ +export const moduleResolverFor = ( + func: BackendFunction, + exports: Record, +): LoadModule => { + return async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return exports; + } + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }; +}; + +/** + * Create a mock IncomingMessage with a JSON body. + */ +export function createMockRequest(url: string, body: Record): IncomingMessage { + const req = new EventEmitter() as unknown as IncomingMessage; + req.method = 'POST'; + req.url = url; + + // Simulate body stream in next tick. + process.nextTick(() => { + (req as unknown as EventEmitter).emit('data', Buffer.from(JSON.stringify(body))); + (req as unknown as EventEmitter).emit('end'); + }); + + return req; +} + +/** + * Create a mock ServerResponse that captures output. + * Exposes a `done` promise that resolves when `end()` is called. + */ +export function createMockResponse() { + let body = ''; + let resolveDone: () => void; + const done = new Promise((resolve) => { + resolveDone = resolve; + }); + + const res = { + statusCode: 200, + setHeader: jest.fn(), + end: jest.fn((data: string) => { + body = data || ''; + resolveDone(); + }), + getBody() { + return body; + }, + done, + }; + return res as typeof res & ServerResponse; +} + export const mockLogFn = jest.fn((text: any, level: LogLevel) => {}); export const getMockLogger = (overrides: Partial = {}): Logger => ({ getLogger: jest.fn(), diff --git a/packages/tools/src/rollupConfig.mjs b/packages/tools/src/rollupConfig.mjs index fadfd2a5e..d599b6784 100644 --- a/packages/tools/src/rollupConfig.mjs +++ b/packages/tools/src/rollupConfig.mjs @@ -38,7 +38,7 @@ const BUNDLER_NAME_RX = /^@datadog\/(.+)-plugin$/g; * @typedef {import('@dd/core/types').Assign< * import('rollup').RollupOptions, * { - * external?: string[]; + * external?: string[] | ((id: string) => boolean); * plugins?: InputPluginOption[]; * } * >} RollupOptions @@ -50,10 +50,8 @@ const BUNDLER_NAME_RX = /^@datadog\/(.+)-plugin$/g; * @param {RollupOptions} config * @returns {RollupOptions} */ -export const bundle = (packageJson, config) => ({ - input: 'src/index.ts', - ...config, - external: [ +export const bundle = (packageJson, config) => { + const externalPackageNames = [ // All peer dependencies are external dependencies. ...Object.keys(packageJson.peerDependencies), // All dependencies are external dependencies. @@ -61,28 +59,42 @@ export const bundle = (packageJson, config) => ({ // These should be internal only and never be anywhere published. '@dd/tools', '@dd/tests', - // We never want to include Node.js built-in modules in the bundle. - ...modulePackage.builtinModules, - ...(config.external || []), - ], - onwarn(warning, warn) { - // Ignore warnings about undefined `this`. - if (warning.code === 'THIS_IS_UNDEFINED') { - return; - } - warn(warning); - }, - plugins: [ - babel({ - babelHelpers: 'bundled', - include: ['src/**/*'], - }), - json(), - commonjs(), - nodeResolve({ preferBuiltins: true }), - ...(config.plugins || []), - ], -}); + ]; + + return { + input: 'src/index.ts', + ...config, + // Rollup's `external` array only matches an id exactly (not a deep import — `rollup` + // doesn't cover `rollup/parseAst`). Once plugin-node-resolve resolves it to an absolute + // path, the exact-match check can't see the original bare specifier at all. + external: (id) => + // We never want to include Node.js built-in modules in the bundle. + modulePackage.builtinModules.includes(id) || + externalPackageNames.some((name) => id === name || id.startsWith(`${name}/`)) || + // `config.external` can be a matcher function (see the type above) as well as a + // plain array, so it must be invoked rather than always treated as one. + (typeof config.external === 'function' + ? config.external(id) + : (config.external || []).includes(id)), + onwarn(warning, warn) { + // Ignore warnings about undefined `this`. + if (warning.code === 'THIS_IS_UNDEFINED') { + return; + } + warn(warning); + }, + plugins: [ + babel({ + babelHelpers: 'bundled', + include: ['src/**/*'], + }), + json(), + commonjs(), + nodeResolve({ preferBuiltins: true }), + ...(config.plugins || []), + ], + }; +}; /** * Returns the base configuration for the build plugin in the context of this project. diff --git a/packages/tools/src/rollupConfig.test.ts b/packages/tools/src/rollupConfig.test.ts index e01117bd6..b5947c786 100644 --- a/packages/tools/src/rollupConfig.test.ts +++ b/packages/tools/src/rollupConfig.test.ts @@ -449,3 +449,82 @@ describe('Bundling', () => { console.timeEnd(timeId); }); }); + +describe('bundle - external matcher', () => { + // rollupConfig.mjs is a real ES module that ts-jest's CommonJS compilation can't import + // under Jest, so a real `node` subprocess runs the exact same code a real build does. + const runExternalMatcher = (ids: string[], config: { external?: string[] } = {}): boolean[] => { + const script = ` + import { bundle } from ${JSON.stringify(pathToFileURL(path.resolve(__dirname, 'rollupConfig.mjs')).href)}; + const packageJson = { + module: 'dist/src/index.js', + main: 'dist/src/index.cjs', + name: '@datadog/some-plugin', + peerDependencies: { vite: '6.0.0' }, + dependencies: { chalk: '2.3.1', rollup: '4.45.1' }, + }; + const { external } = bundle(packageJson, ${JSON.stringify(config)}); + console.log(JSON.stringify(${JSON.stringify(ids)}.map((id) => external(id)))); + `; + const output = executeSync('node', ['--input-type=module', '-e', script]); + return JSON.parse(output); + }; + + test('Should treat a dependency as external', () => { + expect(runExternalMatcher(['chalk'])).toEqual([true]); + }); + + test('Should treat a peer dependency as external', () => { + expect(runExternalMatcher(['vite'])).toEqual([true]); + }); + + test('Should treat a Node.js built-in as external', () => { + expect(runExternalMatcher(['fs'])).toEqual([true]); + }); + + test('Should treat an id explicitly listed in config.external as external', () => { + expect( + runExternalMatcher(['some-extra-package'], { external: ['some-extra-package'] }), + ).toEqual([true]); + }); + + test('Should treat a subpath import of a dependency as external, not just its exact bare specifier', () => { + // The bug this matcher exists to fix: a plain string in Rollup's own `external` array + // only matches an id exactly, so `rollup/parseAst` would otherwise get bundled despite + // `rollup` itself being declared external. + expect(runExternalMatcher(['rollup/parseAst'])).toEqual([true]); + }); + + test('Should not treat an unrelated package as external', () => { + expect(runExternalMatcher(['left-pad'])).toEqual([false]); + }); + + test('Should not treat a package whose name merely starts with a dependency name as a subpath of it', () => { + // `rollup-plugin-esbuild` is not a subpath of `rollup` — the matcher must check for a + // `/` boundary (`startsWith('rollup/')`), not a bare string-prefix match, or an unrelated + // sibling package sharing a name prefix would be wrongly externalized. + expect(runExternalMatcher(['rollup-plugin-esbuild'])).toEqual([false]); + }); + + test('Should invoke a function-valued config.external instead of throwing', () => { + // config.external is typed as `string[] | ((id: string) => boolean)` — a matcher that + // always called `.includes(id)` on it would throw `TypeError: ...includes is not a + // function` the moment a caller passed a function instead of an array. + const script = ` + import { bundle } from ${JSON.stringify(pathToFileURL(path.resolve(__dirname, 'rollupConfig.mjs')).href)}; + const packageJson = { + module: 'dist/src/index.js', + main: 'dist/src/index.cjs', + name: '@datadog/some-plugin', + peerDependencies: {}, + dependencies: {}, + }; + const { external } = bundle(packageJson, { + external: (id) => id === 'only-this-one', + }); + console.log(JSON.stringify(['only-this-one', 'left-pad'].map((id) => external(id)))); + `; + const output = executeSync('node', ['--input-type=module', '-e', script]); + expect(JSON.parse(output)).toEqual([true, false]); + }); +}); diff --git a/yarn.lock b/yarn.lock index 05e9284d1..03495766f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1950,6 +1950,7 @@ __metadata: "@dd/core": "workspace:*" "@types/eslint-scope": "npm:3.7.7" "@types/estree": "npm:1.0.8" + esbuild: "npm:0.25.8" eslint-scope: "npm:7.2.2" glob: "npm:11.1.0" jszip: "npm:3.10.1" From de961a0a274d02c2957fff39713deb1207db6d75 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Wed, 2 Sep 2026 17:54:05 -0400 Subject: [PATCH 2/3] fix(apps): re-arm the absolute execution ceiling per $.Actions call Also forward Rollup's full external(id, importer, isResolved) signature through rollupConfig.mjs's function-valued matcher wrapper instead of only id. Co-Authored-By: Claude Sonnet 5 --- .../apps/src/vite/local-execution.test.ts | 39 +++++++++++++++++++ .../plugins/apps/src/vite/local-execution.ts | 28 ++++++++----- packages/tools/src/rollupConfig.mjs | 9 +++-- packages/tools/src/rollupConfig.test.ts | 24 ++++++++++++ 4 files changed, 86 insertions(+), 14 deletions(-) diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index c5b9f6b58..05df6530b 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -648,6 +648,45 @@ describe('local-execution — executeScriptLocally', () => { } }); + // Regression test: the absolute ceiling used to be a single fixed window from execution + // start, so two genuinely healthy sequential calls (each individually within bounds) could + // still sum past it. Re-arming the ceiling on each new call fixes that without weakening the + // hang protection above, which relies on the call never re-arming it at all. + test('Should not reject a function whose sequential $.Actions calls each individually stay within the absolute ceiling but sum past it', async () => { + jest.useFakeTimers(); + try { + const { totalExecutionTimeoutMs } = deriveActionTimeouts(DEFAULT_LONG_POLLING_CONFIG); + const delayedExecuteAction: ExecuteAction = () => + new Promise((resolve) => { + setTimeout(() => resolve('ok'), totalExecutionTimeoutMs - 10); + }); + + const execution = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + delayedExecuteAction, + loadModuleReturning({ + example: async () => { + await testDollar().Actions.slack.chat.postMessage({ + inputs: { text: 'first' }, + }); + await testDollar().Actions.slack.chat.postMessage({ + inputs: { text: 'second' }, + }); + return 'done'; + }, + }), + mockLogger, + ); + + await jest.advanceTimersByTimeAsync(totalExecutionTimeoutMs * 2); + await expect(execution).resolves.toEqual({ data: 'done' }); + } finally { + jest.useRealTimers(); + } + }); + test('Should still time out a function that hangs with no $.Actions call in flight, even after an earlier call in the same run completed', async () => { const executeAction: ExecuteAction = async () => ({ ok: true }); diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index 42e7efdd1..e26e3c59d 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -615,6 +615,7 @@ async function runScriptLocally( let timer: ReturnType | undefined; let rejectTimeout: ((error: Error) => void) | undefined; let pendingActionCalls = 0; + let absoluteTimeoutTimer: ReturnType | undefined; const scheduleTimeout = () => { timer = setTimeout(() => { @@ -625,6 +626,21 @@ async function runScriptLocally( }, timeoutMs); }; + // Re-armed (not just set once) from each new $.Actions call below, so a function making + // several sequential calls — each within its own actionCallTimeoutMs — isn't killed for + // exceeding a ceiling sized for only one of them. + const rearmAbsoluteTimeout = () => { + clearTimeout(absoluteTimeoutTimer); + absoluteTimeoutTimer = setTimeout(() => { + concludeExecution(); + rejectTimeout?.( + new Error( + `Local execution of "${func.name}" exceeded the absolute ${totalExecutionTimeoutMs}ms execution ceiling, regardless of any $.Actions call in flight.`, + ), + ); + }, totalExecutionTimeoutMs); + }; + const guardedExecuteAction: ExecuteAction = async (fqn, inputs, connectionId) => { if (!scope.isCurrent()) { // Wording stays conclusion-neutral, not "timed out" — a concluded scope may have @@ -633,6 +649,7 @@ async function runScriptLocally( } pendingActionCalls += 1; clearTimeout(timer); + rearmAbsoluteTimeout(); try { const actionCallPromise = executeAction(fqn, inputs, connectionId); return await withTimeout( @@ -727,16 +744,7 @@ async function runScriptLocally( scheduleTimeout(); }); - // Fires regardless of pendingActionCalls, unlike the timeout above — bounds a fire-and-forget - // call masking a hang to totalExecutionTimeoutMs instead of the per-call ceiling. - const absoluteTimeoutTimer = setTimeout(() => { - concludeExecution(); - rejectTimeout?.( - new Error( - `Local execution of "${func.name}" exceeded the absolute ${totalExecutionTimeoutMs}ms execution ceiling, regardless of any $.Actions call in flight.`, - ), - ); - }, totalExecutionTimeoutMs); + rearmAbsoluteTimeout(); // Racing the timeout only stops the caller from waiting — run() keeps executing afterward, // since true cancellation would require terminating a Worker thread. diff --git a/packages/tools/src/rollupConfig.mjs b/packages/tools/src/rollupConfig.mjs index d599b6784..3d8ea92ac 100644 --- a/packages/tools/src/rollupConfig.mjs +++ b/packages/tools/src/rollupConfig.mjs @@ -38,7 +38,7 @@ const BUNDLER_NAME_RX = /^@datadog\/(.+)-plugin$/g; * @typedef {import('@dd/core/types').Assign< * import('rollup').RollupOptions, * { - * external?: string[] | ((id: string) => boolean); + * external?: string[] | ((id: string, importer: string | undefined, isResolved: boolean) => boolean); * plugins?: InputPluginOption[]; * } * >} RollupOptions @@ -67,14 +67,15 @@ export const bundle = (packageJson, config) => { // Rollup's `external` array only matches an id exactly (not a deep import — `rollup` // doesn't cover `rollup/parseAst`). Once plugin-node-resolve resolves it to an absolute // path, the exact-match check can't see the original bare specifier at all. - external: (id) => + external: (id, importer, isResolved) => // We never want to include Node.js built-in modules in the bundle. modulePackage.builtinModules.includes(id) || externalPackageNames.some((name) => id === name || id.startsWith(`${name}/`)) || // `config.external` can be a matcher function (see the type above) as well as a - // plain array, so it must be invoked rather than always treated as one. + // plain array, so it must be invoked (with Rollup's full callback signature) rather + // than always treated as one. (typeof config.external === 'function' - ? config.external(id) + ? config.external(id, importer, isResolved) : (config.external || []).includes(id)), onwarn(warning, warn) { // Ignore warnings about undefined `this`. diff --git a/packages/tools/src/rollupConfig.test.ts b/packages/tools/src/rollupConfig.test.ts index b5947c786..86bf90df0 100644 --- a/packages/tools/src/rollupConfig.test.ts +++ b/packages/tools/src/rollupConfig.test.ts @@ -527,4 +527,28 @@ describe('bundle - external matcher', () => { const output = executeSync('node', ['--input-type=module', '-e', script]); expect(JSON.parse(output)).toEqual([true, false]); }); + + test('Should forward importer and isResolved to a function-valued config.external, not just id', () => { + // Rollup's real external callback is (source, importer, isResolved) — a wrapper that + // only forwarded id would silently break a matcher that inspects the other two. + const script = ` + import { bundle } from ${JSON.stringify(pathToFileURL(path.resolve(__dirname, 'rollupConfig.mjs')).href)}; + const packageJson = { + module: 'dist/src/index.js', + main: 'dist/src/index.cjs', + name: '@datadog/some-plugin', + peerDependencies: {}, + dependencies: {}, + }; + const { external } = bundle(packageJson, { + external: (id, importer, isResolved) => Boolean(importer) && isResolved === true, + }); + console.log(JSON.stringify([ + external('some-id', '/src/importer.ts', true), + external('some-id', undefined, false), + ])); + `; + const output = executeSync('node', ['--input-type=module', '-e', script]); + expect(JSON.parse(output)).toEqual([true, false]); + }); }); From b7cc3d78ced519d14b6d1daf5d21ac345e572aff Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Wed, 2 Sep 2026 17:59:44 -0400 Subject: [PATCH 3/3] fix(apps): derive the default long-polling config instead of hardcoding a copy DEFAULT_LONG_POLLING_CONFIG now calls validate.ts's resolveLongPolling directly, so the two can't silently drift apart. Co-Authored-By: Claude Sonnet 5 --- packages/plugins/apps/src/validate.ts | 2 +- packages/plugins/apps/src/vite/local-execution.ts | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/plugins/apps/src/validate.ts b/packages/plugins/apps/src/validate.ts index b3a88dd29..6e463063f 100644 --- a/packages/plugins/apps/src/validate.ts +++ b/packages/plugins/apps/src/validate.ts @@ -7,7 +7,7 @@ import type { Options } from '@dd/core/types'; import { CONFIG_KEY } from './constants'; import type { AppsOptions, AppsOptionsWithDefaults } from './types'; -const resolveLongPolling = ( +export const resolveLongPolling = ( longPolling: AppsOptions['longPolling'], ): AppsOptionsWithDefaults['longPolling'] => { const maxRetries = longPolling?.maxRetries ?? 10; diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index e26e3c59d..80b748cf3 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -13,6 +13,7 @@ import { isActionCatalogInstalled, isDatadogAppsBackendInstalled } from '../back import type { BackendFunction, BackendOutputs } from '../backend/types'; import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; import type { LongPollingOptions } from '../types'; +import { resolveLongPolling } from '../validate'; import { createEpochGuard } from './execution-epoch'; import { getTotalRetryDelayBudgetMs } from './retry-delay'; @@ -117,15 +118,9 @@ export const DEFAULT_TIMEOUT_MS = 10_000; type LongPollingConfig = Required; -// Matches validate.ts's resolveLongPolling defaults, so callers get the same effective ceilings -// a real dev server derives without each passing one in. Exported so tests assert the derived -// value, not a hardcoded copy. -export const DEFAULT_LONG_POLLING_CONFIG: LongPollingConfig = { - maxRetries: 10, - timeoutMs: 40_000, - jitter: true, - exponentialBackoff: true, -}; +// Derived from validate.ts's own resolveLongPolling rather than a hardcoded copy, so callers get +// the same effective ceilings a real dev server derives without each passing one in. +export const DEFAULT_LONG_POLLING_CONFIG: LongPollingConfig = resolveLongPolling(undefined); /** * Both ceilings must exceed `pollQueryExecution`'s worst-case budget: polling time