diff --git a/packages/plugins/apps/package.json b/packages/plugins/apps/package.json index a09626633..f648cef15 100644 --- a/packages/plugins/apps/package.json +++ b/packages/plugins/apps/package.json @@ -32,15 +32,16 @@ "dependencies": { "@dd/core": "workspace:*", "chalk": "2.3.1", + "esbuild": "0.25.8", "eslint-scope": "7.2.2", "glob": "11.1.0", "jszip": "3.10.1", - "pretty-bytes": "5.6.0" + "pretty-bytes": "5.6.0", + "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/module-graph.ts b/packages/plugins/apps/src/backend/ast-parsing/module-graph.ts index 171f90066..36c989f14 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,8 @@ function collectStaticModuleDependencies( })); } -function getStaticModuleSources(ast: Program): string[] { +// Exported so callers without build-time Rollup ModuleInfo (the dev server) can resolve each static specifier individually against the exact same list this module zips against, rather than 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/backend/types.ts b/packages/plugins/apps/src/backend/types.ts index 95f228d33..2022e8faa 100644 --- a/packages/plugins/apps/src/backend/types.ts +++ b/packages/plugins/apps/src/backend/types.ts @@ -13,5 +13,5 @@ export interface BackendFunction { allowedConnectionIds: string[]; } -/** Shape of a backend function's result, shared by the remote (dev-server.ts) and in-process (local-execution.ts) execution paths — mirrors the Datadog app-builder query response, which wraps a JS action's return value as `{ data: }`. */ +/** Shape of a backend function's result, shared by the remote (dev-server.ts) and in-process (local-execution.ts) paths — mirrors the app-builder query response's `{ data: }` wrapper. */ export type BackendOutputs = { data: unknown }; diff --git a/packages/plugins/apps/src/constants.ts b/packages/plugins/apps/src/constants.ts index 53732b39c..0c16433d8 100644 --- a/packages/plugins/apps/src/constants.ts +++ b/packages/plugins/apps/src/constants.ts @@ -11,11 +11,11 @@ export const APPS_API_PATH = 'api/unstable/app-builder-code/apps'; export const ARCHIVE_FILENAME = 'datadog-apps-assets.zip'; export const BACKEND_FILE_RE = /\.backend\.(ts|tsx|js|jsx)$/; -/** Query suffix marking a local-execution load, so the transform hook below can skip proxy generation for it instead of matching via the broader `options.ssr` flag. */ +/** Query suffix marking a local-execution load, so the transform hook can target it directly instead of matching on the broader `options.ssr` flag. */ export const LOCAL_EXECUTION_LOAD_SUFFIX = '?dd-local-exec'; -// Derived from BACKEND_FILE_RE plus the escaped suffix (its only regex-special character is the leading `?`), so the two can't drift apart if either the extension list or the suffix ever changes. -export const LOCAL_EXECUTION_LOAD_RE = new RegExp( - `${BACKEND_FILE_RE.source.slice(0, -1)}\\${LOCAL_EXECUTION_LOAD_SUFFIX}$`, +// Matches a backend file with any (or no) trailing query string — scoping only to the exact local-execution suffix would let an unrecognized query slip past this filter and leak the real backend source instead of the safe proxy stub; the handler decides safety per case. +export const BACKEND_FILE_WITH_QUERY_RE = new RegExp( + `${BACKEND_FILE_RE.source.slice(0, -1)}(\\?.*)?$`, ); export const BACKEND_CODE_EXTENSIONS = [ '.ts', 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..00d2fe0d4 --- /dev/null +++ b/packages/plugins/apps/src/vite/dev-server-module-graph.test.ts @@ -0,0 +1,50 @@ +// 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 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; + +function makeFakeServer(resolveId: (specifier: string) => Promise<{ id: string } | null>) { + return { + moduleGraph: { + getModuleById: (id: string) => + id === SUFFIXED_ENTRY_ID + ? { id: SUFFIXED_ENTRY_ID, file: ENTRY_ID, importedModules: new Set() } + : undefined, + }, + pluginContainer: { + resolveId: (specifier: string) => resolveId(specifier), + }, + } 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)).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); + + expect(records.has(ENTRY_ID)).toBe(true); + }); +}); 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..56bb4f72a --- /dev/null +++ b/packages/plugins/apps/src/vite/dev-server-module-graph.ts @@ -0,0 +1,148 @@ +// 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 { transform } from 'esbuild'; +import { readFile } from 'node:fs/promises'; +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 { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; + +/** + * Builds the same `ReadonlyMap` shape + * `createBackendModuleGraphCollector`'s `moduleParsed` hook produces during a real Rollup + * build, but for the dev server, where that hook never fires (Vite's plugin container + * doesn't implement it) — instead walking `server.moduleGraph`, which `ssrLoadModule` + * already populates with the entry's full transitive static-import graph by the time it + * resolves. + * + * Parses each module's source fresh from disk (via `ModuleNode.file`), stripped of TS/JSX + * by `esbuild.transform` in isolation, rather than Vite's own transform results — the + * client transform doesn't run for an SSR-only load, and the SSR transform rewrites + * imports into `__vite_ssr_import__(...)` calls that `collectActionCatalogImports`'s + * plain-`ImportDeclaration` parser can't read. `esbuild.transform` alone only strips + * types/JSX, leaving import specifiers untouched, so this sees the same syntax the + * production build path already trusts. + * + * Call only after `loadModule` has resolved for `bareEntryId + LOCAL_EXECUTION_LOAD_SUFFIX` + * in the same request — the graph is a live side effect of that call. The suffix is + * appended internally, since Vite keys the loaded node by its full resolved id, so callers + * only need to pass the bare id `extractConnectionIdsFromModuleGraph` also uses. + */ +export async function collectModuleGraphFromServer( + server: ViteDevServer, + bareEntryId: string, + buildRoot: string, +): Promise> { + const records = new Map(); + const visited = new Set(); + const pending: ModuleNode[] = []; + + const entryNode = server.moduleGraph.getModuleById(bareEntryId + LOCAL_EXECUTION_LOAD_SUFFIX); + if (entryNode) { + pending.push(entryNode); + } + + while (pending.length > 0) { + const node = pending.shift()!; + const moduleId = node.id ? normalizeViteModuleId(node.id) : undefined; + if (!moduleId || visited.has(moduleId) || !node.file) { + continue; + } + visited.add(moduleId); + + if (!shouldTraverseCollectedModule(moduleId, buildRoot)) { + continue; + } + + let source: string; + try { + source = await readFile(node.file, 'utf-8'); + } 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})`, + ); + } + + // `node.importedModules` mixes static and dynamic imports with no ordering guarantee, + // but `createParsedModuleRecord` zips dependency ids positionally against the AST's + // static import/export declarations, so this list must be static-only, same order. The + // dev server has no Rollup-style `ModuleInfo.importedIds` outside a real build, so each + // static specifier is resolved individually via Vite's own resolution instead — a + // correct 1:1 correspondence by construction, since both sides derive from the same AST. + 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 rather than trusting an incomplete allowlist — falling back to the raw specifier text would let a connectionId-scoped call behind an unresolvable import silently drop out of extractConnectionIdsFromModuleGraph's allowlist instead of the whole request failing loudly. + throw unsupportedModuleGraphDependency( + moduleId, + `unresolvable import specifier "${staticModuleSources[index]}"`, + ); + } + return normalizeViteModuleId(resolved.id); + }); + + const record = createParsedModuleRecord(moduleId, buildRoot, ast, staticDependencyIds); + if (record) { + records.set(record.id, record); + } + + for (const dependencyNode of node.importedModules) { + 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'; +} + +function normalizeViteModuleId(id: string): string { + return id.split('?')[0]; +} 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..0d8b8291a --- /dev/null +++ b/packages/plugins/apps/src/vite/dev-server.integration.test.ts @@ -0,0 +1,453 @@ +// 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 the local-execution path's module resolution — no mocked + * `viteBuild`/`loadModule`, no hand-written stand-in module. Spins up a real Vite dev + * server (`createServer`, middleware mode, no port bound) rooted at the same + * `apps_backend_project` fixture `backend/integration.test.ts` uses, and lets its real + * `ssrLoadModule` import and execute a real `.backend.ts` file via the real + * `/__dd/executeAction` handler, including resolving `@datadog/apps-backend` from the + * fixture's own project root. + * + * The nested-import test below registers the real `getVitePlugin()` hooks on this server + * to exercise `resolveId`'s `LOCAL_EXECUTION_LOAD_SUFFIX` propagation against Vite's actual + * module resolution, which a mocked `this.resolve()` can't reproduce. + * + * `@datadog/apps-backend` and `@datadog/action-catalog` are real, locally-resolvable + * fixture packages (not mocked), so the connection-ID coverage below exercises Vite's + * actual SSR transform output rather than a hand-crafted AST shape a real transform would + * never produce. + */ + +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 { getContextMock, getMockLogger } from '@dd/tests/_jest/helpers/mocks'; +import { EventEmitter } from 'events'; +import type { IncomingMessage, ServerResponse } from 'http'; +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', +); + +const getRuntimeUsersFunc: BackendFunction = { + relativePath: 'getRuntimeUsers', + name: 'getRuntimeUsers', + absolutePath: path.join(FIXTURE_ROOT, 'getRuntimeUsers.backend.ts'), + allowedConnectionIds: [], +}; + +function createMockRequest(url: string, body: Record): IncomingMessage { + const req = new EventEmitter() as unknown as IncomingMessage; + req.method = 'POST'; + req.url = url; + process.nextTick(() => { + (req as unknown as EventEmitter).emit('data', Buffer.from(JSON.stringify(body))); + (req as unknown as EventEmitter).emit('end'); + }); + return req; +} + +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; +} + +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 () => { + const appsPlugin: Plugin = { + name: 'dd-apps-test', + ...getVitePlugin({ + bundler: { build }, + context: getContextMock({ buildRoot: FIXTURE_ROOT }), + options: { + authOverrides: { method: 'apiKey' }, + include: [], + dryRun: true, + }, + }), + }; + + server = await createServer({ + configFile: false, + root: FIXTURE_ROOT, + logLevel: 'silent', + server: { middlewareMode: true, hmr: false }, + plugins: [appsPlugin], + // Local execution only ever goes through ssrLoadModule, never the client bundle, so + // Vite's auto-crawl-and-pre-bundle step (for the browser path) is disabled — a + // fixture-only dependency like the fake @datadog/action-catalog package below can + // otherwise trip it up in ways unrelated to what this suite tests. + 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, + getAuthenticatedRequest('apiKey', auth, getMockLogger()), + 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); + + // Real coverage for resolveId's LOCAL_EXECUTION_LOAD_SUFFIX propagation: nestedImport.backend.ts + // statically imports plainEcho from getRuntimeUsers.backend.ts. Without propagating the suffix + // onto that nested import, Vite would resolve it unsuffixed, the transform hook would swap in + // the frontend RPC-proxy stub (calling the server-nonexistent globalThis.DD_APPS_RUNTIME), and + // this would throw instead of returning the real value. + 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, + getAuthenticatedRequest('apiKey', auth, getMockLogger()), + 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); + + // Real coverage for the multi-hop case the single-hop test above doesn't exercise: + // viaHelper.backend.ts imports helper.ts (plain, non-backend), which imports plainEcho from + // getRuntimeUsers.backend.ts. Suffix propagation must follow the chain through helper.ts + // (reached via a suffixed importer, but never itself suffixed) — otherwise helper.ts becomes + // an unsuffixed importer one hop past the direct case, and getRuntimeUsers.backend.ts + // resolves to its frontend RPC-proxy stub instead of its real code. + 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, + getAuthenticatedRequest('apiKey', auth, getMockLogger()), + 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); + + // Regression coverage for the real configureServer-installed middleware, not the hand-built + // one every other test in this file uses via createDevServerMiddleware(..., () => [], ...), + // which bypasses getAllowedConnectionIds' real wiring entirely. Sending the request through + // server.middlewares — the real Connect stack getVitePlugin's configureServer hook installs — + // is what actually exercises getAllowedConnectionIds resolving the entry's record without + // moduleParsed, a Rollup-build-only hook that never fires on a real dev server. + 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); + + // Regression coverage for a real getAllowedConnectionIds, wired exactly as vite/index.ts's + // configureServer builds it, on the very first request for an entry — no priming import + // beforehand. Uses noSdkFunc since any other function here would already have a warm + // moduleGraph node from an earlier test against this shared beforeAll server, masking the + // invariant under test: on a cold entry, Vite only registers the node under the fully-resolved + // (suffixed) id, so collectModuleGraphFromServer must look it up by that 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), + FIXTURE_ROOT, + ); + + const auth: AuthOptionsWithDefaults = { + apiKey: 'test-api-key', + appKey: 'test-app-key', + site: 'datadoghq.com', + }; + const middleware = createDevServerMiddleware( + build, + loadModule, + () => [noSdkFunc], + getAllowedConnectionIds, + auth, + getAuthenticatedRequest('apiKey', auth, getMockLogger()), + 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); + + // Regression coverage for the connection-ID collector against a REAL Vite SSR transform, not a + // hand-crafted AST fixture. Vite's SSR transform rewrites imports into + // `__vite_ssr_import__(...)` calls the plain-`ImportDeclaration` search in + // collectActionCatalogImports can't parse — before collectModuleGraphFromServer started + // reading each module's original source from disk instead, this silently returned an empty + // allowlist for any function importing a typed action-catalog function, rejecting real calls + // with "not in this function's allowed connections: []" for lack of visibility, not a real + // access violation. + 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), + FIXTURE_ROOT, + ); + + const auth: AuthOptionsWithDefaults = { + apiKey: 'test-api-key', + appKey: 'test-app-key', + site: 'datadoghq.com', + }; + const middleware = createDevServerMiddleware( + build, + loadModule, + () => [actionCatalogCallFunc], + getAllowedConnectionIds, + auth, + getAuthenticatedRequest('apiKey', auth, getMockLogger()), + 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); + + // Coverage for a module mixing a static import with a top-level dynamic import textually + // between two static ones — exercises dev-server-module-graph.ts resolving each static + // specifier individually against the AST, independent of node.importedModules's undocumented + // ordering for mixed static/dynamic imports, so the resolution stays correct by construction + // regardless of what that ordering happens to be for a given module. + 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), + FIXTURE_ROOT, + ); + + const auth: AuthOptionsWithDefaults = { + apiKey: 'test-api-key', + appKey: 'test-app-key', + site: 'datadoghq.com', + }; + const middleware = createDevServerMiddleware( + build, + loadModule, + () => [mixedImportsFunc], + getAllowedConnectionIds, + auth, + getAuthenticatedRequest('apiKey', auth, getMockLogger()), + 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 263df3e89..88b634c35 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -2,10 +2,12 @@ // 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 } from '@dd/apps-plugin/vite/dev-server'; import type { AuthOptionsWithDefaults } from '@dd/core/types'; -import { getMockLogger } from '@dd/tests/_jest/helpers/mocks'; +import { getMockLogger, mockLogFn, moduleResolverFor } from '@dd/tests/_jest/helpers/mocks'; import { EventEmitter } from 'events'; import type { IncomingMessage, ServerResponse } from 'http'; import nock from 'nock'; @@ -13,6 +15,7 @@ import { parseAst } from 'rollup/parseAst'; import { encodeQueryName } from '../backend/encodeQueryName'; import type { BackendFunction } from '../backend/types'; +import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; jest.mock('@dd/core/helpers/oauth-request', () => ({ doOAuthRequest: jest.fn(async (opts) => { @@ -26,8 +29,14 @@ jest.mock('@dd/core/helpers/oauth-request', () => ({ }), })); +/** 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); + 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[] = [ @@ -147,20 +156,70 @@ function mockBuildWithParsedBackend(code = '// code') { }); } +/** + * 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(() => { nock.cleanAll(); }); + describe('startup auth warning', () => { + test('Should warn that both executeAction endpoints will be unavailable when auth is not configured', () => { + createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => mockFunctions, + async () => [], + mockAuth, + undefined, + '/project', + mockLog, + ); + + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining( + 'Both the /__dd/executeAction and /__dd/executeActionViaCloud endpoints will be unavailable', + ), + 'warn', + ); + }); + + test('Should not warn when auth is configured', () => { + createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => mockFunctions, + async () => [], + mockAuth, + getApiKeyRequest(), + '/project', + mockLog, + ); + + expect(mockLogFn).not.toHaveBeenCalledWith(expect.anything(), 'warn'); + }); + }); + describe('createDevServerMiddleware routing', () => { const middleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, getApiKeyRequest(), '/project', @@ -208,7 +267,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. @@ -225,7 +305,7 @@ describe('Dev Server Middleware', () => { }, }); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: ['world'], }); @@ -248,7 +328,9 @@ describe('Dev Server Middleware', () => { describe('debugBundle handler', () => { const middleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, getApiKeyRequest(), '/project', @@ -321,10 +403,12 @@ describe('Dev Server Middleware', () => { }); }); - describe('executeAction handler', () => { + describe('executeActionViaCloud handler', () => { const middleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockAuth, getApiKeyRequest(), '/project', @@ -332,7 +416,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()); @@ -342,7 +426,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(); @@ -369,7 +453,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: [], }); @@ -421,7 +505,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], }); @@ -449,7 +533,9 @@ describe('Dev Server Middleware', () => { const oauthMiddleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockOauthOnlyAuth, getOAuthRequest(), '/project', @@ -469,7 +555,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: [], }); @@ -488,14 +574,16 @@ describe('Dev Server Middleware', () => { test('Should return 400 with auth guidance when explicit API-key auth is missing keys', async () => { const noKeyMiddleware = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => mockFunctions, + async () => [], mockOauthOnlyAuth, undefined, '/project', mockLog, ); - const req = createMockRequest('/__dd/executeAction', { + const req = createMockRequest('/__dd/executeActionViaCloud', { functionName: encodeQueryName(mockFunctions[0]), args: [], }); @@ -546,7 +634,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, }); @@ -575,7 +663,9 @@ describe('Dev Server Middleware', () => { ]; const middlewareWithAllowlist = createDevServerMiddleware( mockViteBuild, + mockLoadModule, () => functionsWithAllowlist, + async () => [], mockAuth, getApiKeyRequest(), '/project', @@ -605,7 +695,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: [], }); @@ -660,7 +750,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: [], }); @@ -687,7 +777,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: [], }); @@ -715,7 +805,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: [], }); @@ -732,12 +822,344 @@ describe('Dev Server Middleware', () => { }); }); + describe('executeAction handler (local)', () => { + const middleware = createDevServerMiddleware( + mockViteBuild, + mockLoadModule, + () => mockFunctions, + async () => [], + mockAuth, + getApiKeyRequest(), + '/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 () => [], + mockOauthOnlyAuth, + undefined, + '/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, + getApiKeyRequest(), + '/project', + mockLog, + ); + mockLoadModuleReturning(funcWithConnection, () => + ( + globalThis as typeof globalThis & { $: { Actions: ActionsProxy } } + ).$.Actions.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', + }); + }); + + test("Should surface a successful $.Actions call's result to the local console", async () => { + mockLoadModuleReturning(mockFunctions[0], () => + ( + globalThis as typeof globalThis & { $: { Actions: ActionsProxy } } + ).$.Actions.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], () => + ( + globalThis as typeof globalThis & { $: { Actions: ActionsProxy } } + ).$.Actions.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, even though the module-graph priming load runs before executeScriptLocally installs its own scoping', 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(); + }); + + // Guards the priming loadModule call (see handleExecuteAction) — it evaluates the entry's + // real top-level code before executeScriptLocally's hang-detection timeout is installed, + // so a hanging top-level await would otherwise wedge this request 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(); + } + }); + + // 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 handleExecuteAction. + 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, + getApiKeyRequest(), + '/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, getApiKeyRequest(), '/project', diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index f982275e9..97c971d3c 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -15,9 +15,17 @@ import { encodeQueryName } from '../backend/encodeQueryName'; import type { ExecuteActionRequest, ExecuteActionResponse } from '../backend/protocol'; import type { BackendFunction, BackendOutputs } from '../backend/types'; import { generateDevVirtualEntryContent } from '../backend/virtual-entry'; +import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; import { createBackendConnectionIdCollector } from './backend-connection-id-collector'; import { getBaseBackendBuildConfig } from './build-config'; +import type { ExecuteAction, LoadModule } from './local-execution'; +import { + DEFAULT_TIMEOUT_MS, + executeScriptLocally, + loadCustomerModuleEntry, + withTimeout, +} from './local-execution'; interface BundleResult { func: BackendFunction; @@ -120,18 +128,19 @@ async function bundleBackendFunction( } /** - * Execute a script via Datadog's app-builder queries API. + * Submits a query to Datadog's app-builder `preview-async` endpoint and returns its + * receipt ID. `querySpec` is the query's own `spec` — either the `jsFunctionWithActions` + * wrapper or a single action's `{fqn, inputs}` (see `makeExecuteActionRemotely`) — + * `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, 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}`); @@ -144,14 +153,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, }, }, @@ -178,36 +180,94 @@ async function executeScriptViaDatadog( log.debug(`Query execution started with receipt: ${receiptId}`); - return pollQueryExecution(receiptId, auth, doAuthenticatedRequest, log); + return receiptId; +} + +/** 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, + log: Logger, +): Promise { + const displayName = formatRef(func); + + const receiptId = await submitQuery( + { + fqn: 'com.datadoghq.datatransformation.jsFunctionWithActions', + inputs: { + script: scriptBody, + allowedConnectionIds: func.allowedConnectionIds, + context: { backendFunctionArgs: args }, + }, + }, + displayName, + auth, + doAuthenticatedRequest, + log, + ); + + const outputs = await pollQueryExecution(receiptId, auth, doAuthenticatedRequest, 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, + log: Logger, +): ExecuteAction { + return async ( + fqn: string, + inputs: unknown, + connectionId: string | undefined, + ): Promise => { + try { + const receiptId = await submitQuery( + connectionId ? { fqn, inputs, connectionId } : { fqn, inputs }, + fqn, + auth, + doAuthenticatedRequest, + log, + ); + const result = await pollQueryExecution(receiptId, auth, doAuthenticatedRequest, 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 Datadog API until a submitted query's execution completes or times out, + * returning the raw `outputs` value — shape varies by query type (`jsFunctionWithActions` + * wraps as `{data: }`; a single-action query's `outputs` is that action's own schema), + * so callers interpret it accordingly. The server holds each poll open ~30s and responds + * `done: false` on timeout; this loop only handles that application-level re-polling, since + * `doRequest` already retries transient HTTP failures internally. + */ async function pollQueryExecution( receiptId: string, auth: AuthConfig, doAuthenticatedRequest: DoAuthenticatedRequest, log: Logger, -): Promise { +): Promise { const endpoint = `https://api.${auth.site}/api/v2/app-builder/queries/execution-long-polling/${receiptId}`; const maxRetries = 10; - /* - * Long-poll Datadog API until the query execution completes or times out. - * - * Executing an action works in two phases: - * 1. executeScriptViaDatadog sends a POST to preview-async, which starts the - * query and returns a receipt ID immediately. - * 2. This function polls the execution-long-polling endpoint with that receipt ID. - * The server holds the connection open (~30s) and responds with done: true when - * the result is ready, or done: false when its long-poll window expires. - * - * This loop handles application-level re-polling (done: false), not HTTP retries. - * doRequest already retries transient HTTP failures (5xx, network errors) internally. - */ for (let attempt = 0; attempt < maxRetries; attempt++) { log.debug(`Long-poll attempt ${attempt + 1}/${maxRetries}...`); @@ -226,7 +286,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; @@ -257,14 +317,13 @@ class HttpError extends Error { } /** - * Shared request pipeline: parse body, validate functionName, look up - * the backend function by encoded query name, and bundle it. + * Parse the request body and look up the backend function by encoded query + * name. */ -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') { @@ -276,6 +335,19 @@ async function validateAndBundle( throw new HttpError(404, `Backend function "${functionName}" not found`); } + return { func, args }; +} + +/** + * Shared request pipeline: parse body, validate functionName, look up + * the backend function by encoded query name, and bundle it. + */ +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 }; } @@ -303,9 +375,89 @@ async function handleDebugBundle( } /** - * 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, + 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`); + + // The bundling collector is what normally populates func.allowedConnectionIds, but this + // path skips bundling — priming the entry through Vite's moduleGraph first (see + // collectModuleGraphFromServer) is what makes the allowlist reflect the function's real + // imports instead of staying empty. Runs before executeScriptLocally's hang-detection + // timeout, and evaluates the entry's real top-level code (not a parse-only step) — needs + // its own bound, or a hanging top-level await wedges the request forever. Also the only + // place top-level code runs (Vite caches the module), so it must use + // loadCustomerModuleEntry's $-scoping, not a raw loadModule call, or a customer module's + // top-level $ access would silently resolve stale instead of throwing. + const entrySpecifier = func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX; + const primingLoadPromise = loadCustomerModuleEntry(loadModule, entrySpecifier); + const primedModule = await withTimeout( + primingLoadPromise, + DEFAULT_TIMEOUT_MS, + `Loading "${displayName}"`, + ); + // Same reasoning as the priming load above: this reads every reachable module from disk and + // transforms it, with no bound of its own — a stalled file read or a hung esbuild.transform + // call would otherwise wedge this request forever. + const allowedConnectionIdsPromise = getAllowedConnectionIds(func.absolutePath); + const funcWithConnectionIds: BackendFunction = { + ...func, + allowedConnectionIds: await withTimeout( + allowedConnectionIdsPromise, + DEFAULT_TIMEOUT_MS, + `Resolving allowed connections for "${displayName}"`, + ), + }; + + // executeScriptLocally loads this same entry specifier again + // internally; reuse the module already resolved above instead of + // making Vite re-run ssrLoadModule for it a second time. + const loadModuleReusingPrimedEntry: LoadModule = (specifier) => + specifier === entrySpecifier ? Promise.resolve(primedModule) : loadModule(specifier); + + const executeAction = makeExecuteActionRemotely(auth, doAuthenticatedRequest, log); + const result = await executeScriptLocally( + funcWithConnectionIds, + projectRoot, + args, + executeAction, + loadModuleReusingPrimedEntry, + log, + ); + + res.statusCode = 200; + res.setHeader('Content-Type', 'application/json'); + res.end(JSON.stringify({ success: true, result } satisfies ExecuteActionResponse)); + } 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); + } +} + +/** + * 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, @@ -318,7 +470,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, @@ -335,7 +487,7 @@ async function handleExecuteAction( } 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}`); + log.debug(`Error handling executeActionViaCloud: ${message}`); sendError(res, statusCode, message); } } @@ -357,7 +509,9 @@ function buildFunctionMap(backendFunctions: BackendFunction[]): Map BackendFunction[], + getAllowedConnectionIds: (entryId: string) => Promise, auth: AuthConfig, doAuthenticatedRequest: DoAuthenticatedRequest | undefined, projectRoot: string, @@ -375,7 +529,7 @@ export function createDevServerMiddleware( if (!doAuthenticatedRequest) { log.warn( - `Auth credentials not configured. The /__dd/executeAction endpoint will be unavailable. ${AUTH_GUIDANCE}`, + `Auth credentials not configured. Both the /__dd/executeAction and /__dd/executeActionViaCloud endpoints will be unavailable. ${AUTH_GUIDANCE}`, ); } @@ -392,11 +546,30 @@ export function createDevServerMiddleware( sendError(res, 500, 'Unexpected error'); }); } else if (req.url === '/__dd/executeAction') { + // Matches production's auth-before-execution ordering (PreviewAsyncQueryHandler checks the user first) — checked upfront here, not lazily inside a $.Actions call, so a function that never calls $.Actions isn't a loophole. Just a local presence check, not a credential-validation network call, so it adds no latency to the dev loop. if (!doAuthenticatedRequest) { sendError(res, 400, `Auth credentials not configured. ${AUTH_GUIDANCE}`); return; } handleExecuteAction( + req, + res, + functionsByName, + auth, + doAuthenticatedRequest, + loadModule, + getAllowedConnectionIds, + projectRoot, + log, + ).catch(() => { + sendError(res, 500, 'Unexpected error'); + }); + } else if (req.url === '/__dd/executeActionViaCloud') { + if (!doAuthenticatedRequest) { + sendError(res, 400, `Auth credentials not configured. ${AUTH_GUIDANCE}`); + return; + } + handleExecuteActionViaCloud( req, res, functionsByName, diff --git a/packages/plugins/apps/src/vite/execution-epoch.test.ts b/packages/plugins/apps/src/vite/execution-epoch.test.ts new file mode 100644 index 000000000..14935b46d --- /dev/null +++ b/packages/plugins/apps/src/vite/execution-epoch.test.ts @@ -0,0 +1,89 @@ +// 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 { createEpochGuard } from '@dd/apps-plugin/vite/execution-epoch'; + +describe('execution-epoch — createEpochGuard', () => { + test('Should report a fresh scope as current and report no active scope before any start()', () => { + const guard = createEpochGuard(); + expect(guard.hasActiveScope()).toBe(false); + + const scope = guard.start(); + expect(scope.isCurrent()).toBe(true); + expect(guard.hasActiveScope()).toBe(true); + }); + + test('Should invalidate an older scope once a newer one starts', () => { + const guard = createEpochGuard(); + const older = guard.start(); + expect(older.isCurrent()).toBe(true); + + const newer = guard.start(); + expect(older.isCurrent()).toBe(false); + expect(newer.isCurrent()).toBe(true); + expect(guard.hasActiveScope()).toBe(true); + }); + + test('Should make concludeIfCurrent a no-op returning false for an already-superseded scope', () => { + const guard = createEpochGuard(); + const older = guard.start(); + guard.start(); + + expect(older.concludeIfCurrent()).toBe(false); + // The newer scope must be unaffected by the older one's no-op conclude. + expect(guard.hasActiveScope()).toBe(true); + }); + + test('Should conclude a still-current scope, clearing hasActiveScope', () => { + const guard = createEpochGuard(); + const scope = guard.start(); + + expect(scope.concludeIfCurrent()).toBe(true); + expect(scope.isCurrent()).toBe(false); + expect(guard.hasActiveScope()).toBe(false); + }); + + test('Should make a second concludeIfCurrent call on the same scope a no-op', () => { + const guard = createEpochGuard(); + const scope = guard.start(); + + expect(scope.concludeIfCurrent()).toBe(true); + expect(scope.concludeIfCurrent()).toBe(false); + }); + + test('Should invalidate the active scope and clear hasActiveScope on forceInvalidate, without starting a new one', () => { + const guard = createEpochGuard(); + const scope = guard.start(); + + guard.forceInvalidate(); + + expect(scope.isCurrent()).toBe(false); + expect(guard.hasActiveScope()).toBe(false); + }); + + test('Should make forceInvalidate followed by a fresh start() behave like an ordinary new scope', () => { + const guard = createEpochGuard(); + const abandoned = guard.start(); + guard.forceInvalidate(); + + const current = guard.start(); + + expect(abandoned.isCurrent()).toBe(false); + expect(current.isCurrent()).toBe(true); + expect(guard.hasActiveScope()).toBe(true); + + // The abandoned scope's late conclude must not corrupt the new one. + expect(abandoned.concludeIfCurrent()).toBe(false); + expect(current.isCurrent()).toBe(true); + }); + + test('Should keep independently-created guards from sharing any state', () => { + const guardA = createEpochGuard(); + const guardB = createEpochGuard(); + + const scopeA = guardA.start(); + expect(guardB.hasActiveScope()).toBe(false); + expect(scopeA.isCurrent()).toBe(true); + }); +}); diff --git a/packages/plugins/apps/src/vite/execution-epoch.ts b/packages/plugins/apps/src/vite/execution-epoch.ts new file mode 100644 index 000000000..82d87de2b --- /dev/null +++ b/packages/plugins/apps/src/vite/execution-epoch.ts @@ -0,0 +1,49 @@ +// 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. + +/** Generation-counter guard so an abandoned scope's late cleanup can't touch a shared resource a newer scope now owns (used by `local-execution.ts`). */ +export interface EpochScope { + /** True until a newer scope starts, or this scope is concluded or invalidated. */ + isCurrent(): boolean; + /** Marks no scope active and returns true if still current, otherwise a no-op returning false — call in a `finally` to gate cleanup on still owning the resource. */ + concludeIfCurrent(): boolean; +} + +export interface EpochGuard { + /** Starts a new scope, superseding whichever one was previously active. */ + start(): EpochScope; + /** True if some started scope hasn't yet been concluded or superseded. */ + hasActiveScope(): boolean; + /** Unconditionally invalidates the active scope without starting a new one — the backstop for a scope whose own `fn` never settles. */ + forceInvalidate(): void; +} + +export function createEpochGuard(): EpochGuard { + let currentGeneration = 0; + let activeGeneration: number | null = null; + + return { + start() { + const myGeneration = ++currentGeneration; + activeGeneration = myGeneration; + return { + isCurrent: () => activeGeneration === myGeneration, + concludeIfCurrent: () => { + if (activeGeneration === myGeneration) { + activeGeneration = null; + return true; + } + return false; + }, + }; + }, + hasActiveScope() { + return activeGeneration !== null; + }, + forceInvalidate() { + currentGeneration += 1; + activeGeneration = null; + }, + }; +} diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index f4bff1099..96bc0c922 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -173,7 +173,7 @@ describe('Backend Functions - getVitePlugin', () => { expect(assets.collectAssets).toHaveBeenCalledWith(['dist/**/*'], '/build'); }); - // Regression test: without the suffix check, ssrLoadModule() would get the RPC-proxy stub instead of the real function body. + // Regression test: without the suffix check, ssrLoadModule() would get the proxy stub instead of the real function body. test('Should skip proxy generation for a suffixed local-execution load made from SSR context, returning the real source untouched', async () => { const plugin = getVitePlugin(defaultOptions); const transformHandler = getTransformHandler(plugin); @@ -194,10 +194,7 @@ describe('Backend Functions - getVitePlugin', () => { expect(result).toBeNull(); }); - // Regression test: the suffix alone must not bypass proxy generation — only real local-execution - // loads (via ssrLoadModule, always SSR context) get the real source; a spoofed client-side import - // using the same suffix (e.g. `./secrets.backend.ts?dd-local-exec`) still gets the safe RPC-proxy - // stub, never the real backend module body. + // Regression test: the suffix alone must not bypass proxy generation — a spoofed client-side import reusing it still gets the safe proxy stub, never the real backend module body. test('Should still generate the frontend RPC-proxy for a suffixed import made outside SSR context', async () => { const plugin = getVitePlugin(defaultOptions); const transformHandler = getTransformHandler(plugin); @@ -234,6 +231,43 @@ describe('Backend Functions - getVitePlugin', () => { expect(result?.code).toEqual(expect.stringContaining('executeBackendFunction')); }); + // Regression test: an unrecognized query string must still be caught by the transform filter, or Vite falls back to its default loader and leaks the real backend source. + test('Transform filter should match a backend file carrying an unrecognized query string', () => { + const plugin = getVitePlugin(defaultOptions); + const filter = (plugin!.transform as { filter?: { id?: { include?: RegExp[] } } }).filter; + const includePatterns = filter?.id?.include ?? []; + + const idsThatMustMatch = [ + '/build/src/backend/myHandler.backend.ts', + `/build/src/backend/myHandler.backend.ts${LOCAL_EXECUTION_LOAD_SUFFIX}`, + '/build/src/backend/myHandler.backend.ts?x', + `/build/src/backend/myHandler.backend.ts${LOCAL_EXECUTION_LOAD_SUFFIX}&x`, + ]; + + for (const id of idsThatMustMatch) { + expect(includePatterns.some((pattern) => pattern.test(id))).toBe(true); + } + }); + + // Regression test: an unrecognized query must still default to the safe proxy stub, not the real backend source. + test('Should still generate the frontend RPC-proxy for an import with an unrecognized query string', async () => { + const plugin = getVitePlugin(defaultOptions); + const transformHandler = getTransformHandler(plugin); + + const result = (await transformHandler.call( + { + parse: parseAst, + resolve: jest.fn(async () => null), + load: jest.fn(async () => null), + addWatchFile: jest.fn(), + }, + 'export function myHandler() { return 42; }', + '/build/src/backend/myHandler.backend.ts?x', + )) as { code: string } | null; + + expect(result?.code).toEqual(expect.stringContaining('executeBackendFunction')); + }); + test('Should inject the apps runtime', () => { getVitePlugin(defaultOptions); @@ -243,4 +277,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 eaa6aeb7c..54a92dd5f 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -14,18 +14,20 @@ 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 { encodeQueryName } from '../backend/encodeQueryName'; import { generateProxyModule } from '../backend/proxy-codegen'; import type { BackendFunction } from '../backend/types'; import { BACKEND_FILE_RE, - LOCAL_EXECUTION_LOAD_RE, + BACKEND_FILE_WITH_QUERY_RE, LOCAL_EXECUTION_LOAD_SUFFIX, PLUGIN_NAME, } from '../constants'; import type { AppsOptionsWithDefaults } from '../types'; import { buildBackendFunctions } from './build-backend-functions'; +import { collectModuleGraphFromServer } from './dev-server-module-graph'; import { createDevServerMiddleware } from './dev-server'; import { handleUpload } from './handle-upload'; @@ -122,11 +124,78 @@ export const getVitePlugin = ({ const { setBackendFunctions, getBackendFunctions } = createBackendFunctionRegistry(); + // Non-backend module ids reached transitively from a suffixed backend entry point. A plain + // helper module's id never carries LOCAL_EXECUTION_LOAD_SUFFIX itself (it's never ambiguous + // between a proxy and real code), but must still be recognized as part of the suffixed + // subgraph — otherwise a *.backend.ts file reached through it would lose the marker one hop + // later than a direct backend-to-backend import does. + const suffixedSubgraphImporters = new Set(); + return { + // @datadog/apps-backend and @datadog/action-catalog ship ESM-only, but server.ssrLoadModule + // externalizes node_modules by default (a plain require(), for speed), which throws + // "Cannot use import statement outside a module" for them. ssr.noExternal forces Vite's + // SSR transform pipeline to handle them instead, matching how production bundling already + // inlines every dependency. + config() { + return { + ssr: { + noExternal: ['@datadog/apps-backend', '@datadog/action-catalog'], + }, + }; + }, + // Propagates the LOCAL_EXECUTION_LOAD_SUFFIX marker through the backend-file dependency + // graph: ssrLoadModule only tags the one entry module it's called with, so without this + // hook a `.backend.ts` file importing another `.backend.ts` file would resolve that nested + // import unsuffixed and get replaced with the frontend RPC-proxy stub, breaking local + // execution for a multi-backend-file graph. Propagates whenever the importer was itself + // suffixed, or is a previously-seen non-backend module reached from within the suffixed + // subgraph (see `suffixedSubgraphImporters`) — and only appends the suffix onto another + // `.backend.ts` file, since a plain helper module never hits the proxy-vs-real-code branch + // this marker disambiguates. + resolveId: { + // Must run before Vite's built-in resolver: a plain relative specifier like + // `./other.backend` is fully resolvable by Vite's own filesystem resolution, which at + // its default order would short-circuit the hook chain before this plugin ever saw it. + // `pre` guarantees this hook gets first look at every id. + order: 'pre', + async handler(source, importer, resolveOptions) { + // Scoped to `resolveOptions.ssr` since local execution's traversal is always an + // SSR resolution — otherwise the same helper reached later from the ordinary + // client graph would inherit the marker and serve real backend code to the + // browser instead of the frontend RPC-proxy stub. + const isPartOfSuffixedSubgraph = + !!importer && + (importer.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX) || + (resolveOptions.ssr === true && suffixedSubgraphImporters.has(importer))); + if (!isPartOfSuffixedSubgraph) { + return null; + } + + const resolved = await this.resolve(source, importer, { + ...resolveOptions, + skipSelf: true, + }); + if (!resolved || resolved.external) { + return resolved; + } + + if (!BACKEND_FILE_RE.test(resolved.id)) { + suffixedSubgraphImporters.add(resolved.id); + return resolved; + } + + if (!resolved.id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX)) { + return { ...resolved, id: resolved.id + LOCAL_EXECUTION_LOAD_SUFFIX }; + } + + return resolved; + }, + }, transform: { filter: { id: { - include: [BACKEND_FILE_RE, LOCAL_EXECUTION_LOAD_RE], + include: [BACKEND_FILE_WITH_QUERY_RE], exclude: [/node_modules/, /[/\\]dist[/\\]/], }, }, @@ -134,15 +203,13 @@ export const getVitePlugin = ({ // them as backend functions, and replace the module with a // frontend proxy that calls executeBackendFunction at runtime. handler(code, id, transformOptions) { - let normalizedId = id; - if (id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX)) { - if (transformOptions?.ssr) { - // Local execution needs the real function body, not the RPC-proxy stub generated below. - return null; - } - // A spoofed client-side import like `./secrets.backend.ts?dd-local-exec` falls through to the same safe proxy-stub generation as any other backend file instead — real local-execution loads always go through ssrLoadModule, which runs in SSR context. Strips the suffix first so this registers under the same relativePath/query-name as the file's real (unsuffixed) import, not a second, corrupted entry. - normalizedId = id.slice(0, -LOCAL_EXECUTION_LOAD_SUFFIX.length); + if (id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX) && transformOptions?.ssr) { + // Local execution needs the real function body, not the proxy stub below — real loads always go through ssrLoadModule, which runs in SSR, so this only fires for that legitimate path. + return null; } + // Any other case (no query, a spoofed client-side import reusing the suffix, or an unrecognized query) falls through to the safe proxy-stub generation below. Strip the query first so it registers under the file's real (unsuffixed) relativePath/query-name, not a duplicate. + const queryIndex = id.indexOf('?'); + const normalizedId = queryIndex === -1 ? id : id.slice(0, queryIndex); const ast = this.parse(code); const exportNames = extractExportedFunctions(ast, normalizedId); @@ -215,16 +282,29 @@ export const getVitePlugin = ({ } } - server.middlewares.use( - createDevServerMiddleware( - bundler.build, - getBackendFunctions, - auth, - doAuthenticatedRequest, + const loadModule = server.ssrLoadModule.bind(server); + // Call only after `loadModule` has resolved for this same entryId — the graph read + // below is a live side effect of that call, not independently maintained state + // (moduleParsed, production's mechanism, is a Rollup-build-only hook that never + // fires on a real dev server). collectModuleGraphFromServer appends + // LOCAL_EXECUTION_LOAD_SUFFIX internally, so this closure only handles the bare path. + const getAllowedConnectionIds = async (entryId: string) => + extractConnectionIdsFromModuleGraph( + entryId, + await collectModuleGraphFromServer(server, entryId, context.buildRoot), context.buildRoot, - log, - ), + ); + const middleware = createDevServerMiddleware( + bundler.build, + loadModule, + getBackendFunctions, + getAllowedConnectionIds, + auth, + doAuthenticatedRequest, + context.buildRoot, + log, ); + server.middlewares.use(middleware); }, }; }; diff --git a/packages/plugins/apps/src/vite/local-execution.resilience.test.ts b/packages/plugins/apps/src/vite/local-execution.resilience.test.ts new file mode 100644 index 000000000..288267adf --- /dev/null +++ b/packages/plugins/apps/src/vite/local-execution.resilience.test.ts @@ -0,0 +1,88 @@ +// 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. + +/** Two targeted checks that empirically confirm real failure modes of running backend functions in-process rather than in an isolated child process/thread — accepted v1 limitations, not bugs this file fixes. */ + +import { mockLogger, moduleResolverFor } from '@dd/tests/_jest/helpers/mocks'; +import { spawnSync } from 'child_process'; + +import type { BackendFunction } from '../backend/types'; + +import type { ExecuteAction } from './local-execution'; +import { executeScriptLocally } from './local-execution'; + +const func: BackendFunction = { + relativePath: 'src/example', + name: 'example', + absolutePath: '/src/example.backend.ts', + allowedConnectionIds: [], +}; + +const stubExecuteAction: ExecuteAction = async (fqn) => ({ data: null, stub: true, fqn }); + +describe('local-execution resilience (Milestone 7)', () => { + // A real `while (true) {}` would hang this test (and the whole Jest + // worker) forever, since nothing — including the timeout's own + // setTimeout callback — can run while the event loop is synchronously + // blocked. A bounded, time-boxed busy-wait demonstrates the exact same + // mechanism without actually hanging: if the 20ms timeout could + // interrupt a synchronous loop, this would settle around 20ms with a + // timeout rejection; instead it can only settle once the loop itself + // finishes on its own, ~200ms later, with the loop's real result. + test('Should NOT interrupt a synchronous CPU-bound loop with the current timeout — known, accepted v1 limitation', async () => { + const start = Date.now(); + + const result = await executeScriptLocally( + func, + '/project', + [], + stubExecuteAction, + moduleResolverFor(func, { + example: () => { + const deadline = Date.now() + 200; + // eslint-disable-next-line no-empty + while (Date.now() < deadline) {} + return 'loop finished on its own'; + }, + }), + mockLogger, + 20, + ); + + const elapsedMs = Date.now() - start; + + expect(result).toEqual({ data: 'loop finished on its own' }); + expect(elapsedMs).toBeGreaterThanOrEqual(150); + }); + + // process.exit() can't be run inside this same Jest process — it would + // actually terminate the test runner. Spawning a real child process is + // the only safe way to observe what it does, and it directly tests the + // relevant claim: does try/finally around the customer's function call + // (the same shape runScriptLocally uses to run cleanup unconditionally) + // offer any protection against it? It doesn't — process.exit() is + // immediate and unconditional at the OS level, so no JS-level exception + // handling in this in-process design can intercept it. A customer + // function calling process.exit() takes the whole dev server down with + // it, not just its own execution. + test('Should confirm process.exit() inside the customer function crashes the whole process, bypassing try/finally cleanup — known, real risk, not a safely-contained failure', () => { + const script = ` + async function customerFunction() { + process.exit(7); + } + (async () => { + try { + await customerFunction(); + } finally { + console.log('CLEANUP_RAN'); + } + })(); + `; + + const result = spawnSync(process.execPath, ['-e', script], { timeout: 5000 }); + + expect(result.status).toBe(7); + expect(result.stdout.toString()).not.toContain('CLEANUP_RAN'); + }); +}); diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index fac1fce34..3e1a37690 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -4,7 +4,7 @@ /* 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'; @@ -31,7 +31,7 @@ interface TestGlobalDollar { Source: { initiator: { id: string; orgId: string }; runAsUser: { id: string; orgId: string } }; } -/** Reads the `$` this module installs onto `globalThis` during an execution, from the customer-code perspective these tests simulate — genuinely untyped from TypeScript's static perspective since it's a runtime-only property (see local-execution.ts's `setGlobalDollar`). Centralized here instead of repeating the same cast at each call site. */ +/** Reads the `$` local-execution.ts installs onto `globalThis` via `Object.defineProperty` — genuinely untyped, so the cast is centralized here instead of repeated at each call site. */ function testDollar(): TestGlobalDollar { return (globalThis as unknown as { $: TestGlobalDollar }).$; } @@ -42,20 +42,18 @@ beforeEach(() => { jest.spyOn(shared, 'isDatadogAppsBackendInstalled').mockReturnValue(false); }); +/** 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); + const stubExecuteAction: ExecuteAction = async (fqn) => ({ data: null, stub: true, fqn }); /** 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'; + describe('local-execution — executeScriptLocally', () => { test('Should run a simple function in-process and return its result', async () => { const result = await executeScriptLocally( @@ -103,11 +101,13 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow(`"example" is not a function exported from ${func.absolutePath}`); }); - test('Should load and evaluate the customer module before installing globalThis.$, matching production module-evaluation order', async () => { + test('Should read $ as undefined when a customer module reaches for it during its own top-level evaluation, matching production module-evaluation order', async () => { let dollarDuringModuleLoad: unknown = 'not captured'; const loadModule: LoadModule = async (specifier) => { if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { - // Captures globalThis.$ at module-evaluation time — production's static customer-module import runs before its wrapper installs $, so a customer module reaching for $ during its own top-level evaluation must see the same absence locally, not this execution's own $ installed early. + // Production's static import also runs before its wrapper installs $, so $ isn't a + // global property yet — reading it must resolve to undefined the same way locally, + // not throw (typeof $ never throws on an unresolvable reference in production). dollarDuringModuleLoad = (globalThis as Record).$; return { example: () => 'done' }; } @@ -131,6 +131,128 @@ describe('local-execution — executeScriptLocally', () => { expect(dollarDuringModuleLoad).toBeUndefined(); }); + test("Should return a pre-existing globalThis.$ during a customer module's top-level evaluation when something (e.g. zx/globals) seeded it before this module loaded", async () => { + const originalDescriptor = Object.getOwnPropertyDescriptor(globalThis, '$'); + const preExisting = { fromZxGlobals: true }; + (globalThis as Record).$ = preExisting; + let isolatedExecuteScriptLocally!: typeof executeScriptLocally; + try { + jest.isolateModules(() => { + // A fresh module instance re-runs its Reflect.has check with preExisting already set; the outer instance was imported too early to exercise this path. + isolatedExecuteScriptLocally = require('./local-execution').executeScriptLocally; + }); + + let dollarDuringModuleLoad: unknown = 'not captured'; + const loadModule: LoadModule = async (specifier) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + dollarDuringModuleLoad = (globalThis as Record).$; + return { example: () => 'done' }; + } + throw new Error(`Cannot find module '${specifier}'`); + }; + + const result = await isolatedExecuteScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + ); + expect(result).toEqual({ data: 'done' }); + expect(dollarDuringModuleLoad).toBe(preExisting); + } finally { + if (originalDescriptor) { + Object.defineProperty(globalThis, '$', originalDescriptor); + } else { + delete (globalThis as Record).$; + } + } + }); + + test('Should reinstall the $ accessor if a customer execution deleted globalThis.$, so a later execution can still use it', async () => { + await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + delete (globalThis as Record).$; + return 'first'; + }, + }), + mockLogger, + ); + + const second = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => testDollar().backendFunctionArgs }), + mockLogger, + ); + expect(second).toEqual({ data: [] }); + }); + + test("Should not leak one execution's top-level zx/globals-style $ write into a later execution's own top-level load", async () => { + const firstLoadModule: LoadModule = async (specifier) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + // Simulates a top-level side effect (e.g. `import 'zx/globals'`) writing $ before this execution's box exists. + (globalThis as Record).$ = { + fromFirstExecutionTopLevel: true, + }; + return { example: () => 'first' }; + } + throw new Error(`Cannot find module '${specifier}'`); + }; + await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + firstLoadModule, + mockLogger, + ); + + let dollarDuringSecondLoad: unknown = 'not captured'; + const secondLoadModule: LoadModule = async (specifier) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + dollarDuringSecondLoad = (globalThis as Record).$; + return { example: () => 'second' }; + } + throw new Error(`Cannot find module '${specifier}'`); + }; + await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + secondLoadModule, + mockLogger, + ); + + expect(dollarDuringSecondLoad).toBeUndefined(); + }); + + test('Should reject when loadModule itself rejects, same as a native-module load failure would', async () => { + // Simulates a native addon failing to load at import time — not a customer function throwing. + const loadModule: LoadModule = async () => { + throw new Error('cannot find native module'); + }; + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + ), + ).rejects.toThrow('cannot find native module'); + }); + test('Should resolve a $.Actions.foo.bar(...) call through the injected executeAction, including connectionId', async () => { const executeAction = jest.fn().mockResolvedValue({ ok: true }); const result = await executeScriptLocally( @@ -155,20 +277,21 @@ describe('local-execution — executeScriptLocally', () => { ); }); - test('Should not hang when a customer function returns an un-invoked $.Actions reference instead of calling it', async () => { - // $.Actions.slack.chat is itself a callable Proxy; forgetting the trailing .postMessage(...) call and just returning it must not make `await fn(...args)` treat it as a thenable and hang until the timeout. - const result = await executeScriptLocally( - func, - TEST_PROJECT_ROOT, - [], - stubExecuteAction, - loadModuleReturning({ - example: () => testDollar().Actions.slack.chat, - }), - mockLogger, - 20, - ); - expect(result.data).toBeDefined(); + test('Should reject with a clear error, not hang, when a customer function returns an un-invoked $.Actions reference instead of calling it', async () => { + // Returning $.Actions.slack.chat un-invoked must not be mistaken for a thenable (hang) or leak an unhandled rejection — just the ordinary "can't be serialized" error. + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => testDollar().Actions.slack.chat, + }), + mockLogger, + 20, + ), + ).rejects.toThrow(/JSON\.stringify silently drops/); }); test("Should reject a $.Actions call whose connectionId isn't in the function's allowedConnectionIds", async () => { @@ -242,9 +365,7 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow('boom'); }); - // Regression test: the "late failure" log is meant for an execution abandoned after the caller's own - // await already gave up (see the test below), not every rejection — this one's caller is still waiting - // and receives the same error normally via its own `rejects.toThrow` above. + // Regression test: the "late failure" log fires only for an execution abandoned after the caller stopped waiting (see the test below) — here the caller is still waiting and gets the error via `rejects.toThrow` above. test('Should not log a "caller had already stopped waiting" message for an ordinary, timely rejection', async () => { await expect( executeScriptLocally( @@ -323,6 +444,144 @@ describe('local-execution — executeScriptLocally', () => { ); }); + // executeAction stands in for dev-server.ts's makeExecuteActionRemotely, whose long-poll 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: () => + ( + globalThis as typeof globalThis & { $: { Actions: ActionsProxy } } + ).$.Actions.slack.chat.postMessage({ + inputs: { text: 'hi' }, + }), + }), + mockLogger, + // Shorter than slowExecuteAction's own 80ms. + 50, + ); + + expect(result).toEqual({ data: { ok: true } }); + }); + + // Guards against the hang-detection timer staying paused forever: without a bound on the + // $.Actions call itself, a stalled request would wedge this execution, and every request + // serialized behind it via `enqueue`, indefinitely. The absolute execution ceiling (6 minutes) + // always fires before the per-call bound (10 minutes) could, so that's the mechanism this test + // observes — the per-call bound remains a backstop for any path the ceiling doesn't cover. + 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: () => + ( + globalThis as typeof globalThis & { $: { Actions: ActionsProxy } } + ).$.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 hungAssertion = expect(hungExecution).rejects.toThrow( + /exceeded the absolute 360000ms 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 10 minutes), 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: () => + ( + globalThis as typeof globalThis & { $: { Actions: ActionsProxy } } + ).$.Actions.slack.chat.postMessage({ + inputs: { text: 'hi' }, + }), + }), + mockLogger, + 50, + ); + + const assertion = expect(execution).rejects.toThrow( + /exceeded the absolute 360000ms execution ceiling/, + ); + + await jest.advanceTimersByTimeAsync(6 * 60_000); + 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 ( + globalThis as typeof globalThis & { $: { Actions: ActionsProxy } } + ).$.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( @@ -415,7 +674,24 @@ describe('local-execution — executeScriptLocally', () => { }); }); - test('Should restore a pre-existing globalThis.$ (e.g. from zx/globals) once the execution completes, not leave the execution context in place permanently', async () => { + test('Should allow a customer module to assign to globalThis.$ (e.g. importing zx/globals) without throwing', async () => { + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + (globalThis as Record).$ = { notOurs: true }; + return 'done'; + }, + }), + mockLogger, + ); + expect(result).toEqual({ data: 'done' }); + }); + + test('Should restore a pre-existing globalThis.$ (e.g. from zx/globals) once the execution completes, even if the customer function reassigned it', async () => { const preExisting = { notOurs: true }; (globalThis as Record).$ = preExisting; try { @@ -425,44 +701,39 @@ describe('local-execution — executeScriptLocally', () => { [], stubExecuteAction, loadModuleReturning({ - example: () => testDollar().backendFunctionArgs, + example: () => { + (globalThis as Record).$ = { reassigned: true }; + return 'done'; + }, }), mockLogger, ); - expect(result).toEqual({ data: [] }); - // Compares via a plain boolean, not a direct .toBe() on the value — $.Actions is a Proxy whose get trap returns another Proxy for every property (including well-known symbols), which crashes Jest's diff formatting if this assertion ever fails and needs to pretty-print it. + expect(result).toEqual({ data: 'done' }); expect(Object.is((globalThis as Record).$, preExisting)).toBe(true); } finally { - delete (globalThis as Record).$; + (globalThis as Record).$ = undefined; } }); - test("Should restore a pre-existing globalThis.$ even when the customer function throws, not leave the execution's context behind", async () => { - const preExisting = { notOurs: true }; + test('Should seed the outside-execution slot from a globalThis.$ that already existed before this module was first loaded', () => { + const originalDescriptor = Object.getOwnPropertyDescriptor(globalThis, '$'); + const preExisting = { fromZxGlobals: true }; (globalThis as Record).$ = preExisting; try { - await expect( - executeScriptLocally( - func, - TEST_PROJECT_ROOT, - [], - stubExecuteAction, - loadModuleReturning({ - example: () => { - throw new Error('customer function failed'); - }, - }), - mockLogger, - ), - ).rejects.toThrow('customer function failed'); - expect(Object.is((globalThis as Record).$, preExisting)).toBe(true); + jest.isolateModules(() => { + // A fresh module instance re-runs its top-level Object.defineProperty and must read the current $ (still `preExisting`) rather than start from an empty slot. + require('./local-execution'); + }); + expect((globalThis as Record).$).toBe(preExisting); } finally { - delete (globalThis as Record).$; + if (originalDescriptor) { + Object.defineProperty(globalThis, '$', originalDescriptor); + } } }); - test('Should remove globalThis.$ once the execution completes when nothing was previously defined there', async () => { - delete (globalThis as Record).$; + test('Should read globalThis.$ as undefined once the execution completes when nothing was defined before it started', async () => { + (globalThis as Record).$ = undefined; await executeScriptLocally( func, TEST_PROJECT_ROOT, @@ -471,7 +742,35 @@ describe('local-execution — executeScriptLocally', () => { loadModuleReturning({ example: () => 'done' }), mockLogger, ); - expect(Object.prototype.hasOwnProperty.call(globalThis, '$')).toBe(false); + expect((globalThis as Record).$).toBeUndefined(); + }); + + test("Should not leak one execution's globalThis.$ override into a later, separately-queued execution", async () => { + await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + (globalThis as Record).$ = { fromFirstExecution: true }; + return 'first'; + }, + }), + mockLogger, + ); + + const second = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => Object.keys(testDollar()).sort(), + }), + mockLogger, + ); + expect(second).toEqual({ data: ['Actions', 'Source', 'backendFunctionArgs'] }); }); describe('action-catalog / apps-backend registration', () => { @@ -488,33 +787,131 @@ describe('local-execution — executeScriptLocally', () => { expect(result).toEqual({ data: 'fine' }); }); - test('Should propagate a real load failure from an installed action-catalog package, not treat it as absent', async () => { - jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + test('Should pick up action-catalog on the very next execution after it becomes installed mid-session, not stay permanently skipped', async () => { + const isInstalledSpy = jest + .spyOn(shared, 'isActionCatalogInstalled') + .mockReturnValue(false); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; const loadModule: LoadModule = async (specifier: string) => { if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { - return { example: () => 'unreachable' }; + return { example: () => 'fine' }; } if (specifier === '@datadog/action-catalog/action-execution') { - // A real transform/evaluation failure, not a module-not-found error — must not be swallowed as "not installed". - throw new Error('Unexpected token in action-catalog/action-execution'); + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; } const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); error.code = 'MODULE_NOT_FOUND'; throw error; }; - await expect( - executeScriptLocally( - func, - TEST_PROJECT_ROOT, - [], - stubExecuteAction, - loadModule, - mockLogger, + // Not installed yet — registration is skipped, same as the "neither package installed" case. + await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + ); + expect(registeredImpl).toBeUndefined(); + + // Simulates a mid-session install — the very next execution must register it, not stay skipped from the earlier uncached check. + isInstalledSpy.mockReturnValue(true); + await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + ); + expect(registeredImpl).toBeDefined(); + }); + + test('Should propagate a real load failure from an installed action-catalog package, not treat it as absent', async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { example: () => 'unreachable' }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + // A real transform/evaluation failure, not a module-not-found error — must not be swallowed as "not installed". + throw new Error('Unexpected token in action-catalog/action-execution'); + } + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }; + + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, ), ).rejects.toThrow('Unexpected token in action-catalog/action-execution'); }); + // The sibling registration failing doesn't affect this adapter — it's stable and execution-agnostic, so it rejects on its own once no execution is active. + test('Should still reject a typed-wrapper call through a successfully-registered action-catalog implementation after the sibling apps-backend registration genuinely fails and the execution concludes', async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + jest.spyOn(shared, 'isDatadogAppsBackendInstalled').mockReturnValue(true); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { example: () => 'unreachable' }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + if (specifier === '@datadog/apps-backend/runtime/jsFunctionWithActions') { + // A real transform/evaluation failure, not module-not-found — must not be swallowed as "package isn't installed". + throw new Error( + 'Unexpected token in apps-backend/runtime/jsFunctionWithActions', + ); + } + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }; + + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + ), + ).rejects.toThrow('Unexpected token in apps-backend/runtime/jsFunctionWithActions'); + + expect(registeredImpl).toBeDefined(); + await expect( + registeredImpl?.('com.datadoghq.slack.chat.postMessage', { inputs: {} }), + ).rejects.toThrow(/no active local execution/i); + }); + test('Should route an action-catalog typed-wrapper call through the same injected executeAction', async () => { jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); const executeAction = jest.fn().mockResolvedValue({ ok: true }); @@ -648,65 +1045,1028 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow(/must have an inputs field/); expect(executeAction).not.toHaveBeenCalled(); }); + + // Mirrors the action-catalog abandonment test — apps-backend's setBackend has the same shared-module-level-setter hazard. + test("Should reject an abandoned execution's apps-backend accessor call once concluded", async () => { + jest.spyOn(shared, 'isDatadogAppsBackendInstalled').mockReturnValue(true); + let abandonedCallOutcome: 'pending' | 'resolved' | { rejected: string } = 'pending'; + let registeredBackend: { get: () => unknown } | undefined; + + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { + example: async () => { + await new Promise((resolve) => setTimeout(resolve, 100)); + try { + registeredBackend?.get(); + abandonedCallOutcome = 'resolved'; + } catch (err) { + abandonedCallOutcome = { + rejected: err instanceof Error ? err.message : String(err), + }; + } + return { data: 'abandoned' }; + }, + }; + } + if (specifier === '@datadog/apps-backend/runtime/jsFunctionWithActions') { + return { + // Mirrors the real package's synchronous $.Source validation, so a poisoned proxy passed through here fails the same way. + buildRuntimeFromJsFunctionWithActions: ($: unknown) => { + const source = ($ as Record).Source as + | { initiator?: unknown } + | undefined; + if (!source || typeof source.initiator !== 'object') { + throw new Error( + 'Invalid $.Source supplied to buildRuntimeFromJsFunctionWithActions', + ); + } + return { get: () => source }; + }, + }; + } + if (specifier === '@datadog/apps-backend/runtime') { + return { + setBackend: (runtime: { get: () => unknown }) => { + registeredBackend = runtime; + }, + }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + + const abandoned = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(abandonedCallOutcome).toEqual({ + rejected: expect.stringContaining('already concluded'), + }); + }); + + // A flat method reading its own internal state via `this` (a real, common accessor + // pattern) must still work when called through the backend-runtime proxy — not just + // arrow-function methods that close over data instead, which every other test here uses. + test('Should preserve `this` when a flat apps-backend runtime method reads its own internal state', async () => { + jest.spyOn(shared, 'isDatadogAppsBackendInstalled').mockReturnValue(true); + let registeredBackend: { getUserId(): string } | undefined; + let capturedUserId: unknown; + + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { + example: () => { + capturedUserId = registeredBackend?.getUserId(); + return 'done'; + }, + }; + } + if (specifier === '@datadog/apps-backend/runtime/jsFunctionWithActions') { + return { + buildRuntimeFromJsFunctionWithActions: () => ({ + userId: 'real-user-id', + // A real accessor pattern: reads its own instance state via `this`, + // not a closure — throws if called unbound. + getUserId() { + if ( + !this || + typeof (this as { userId?: unknown }).userId !== 'string' + ) { + throw new Error('getUserId called with no `this`'); + } + return (this as { userId: string }).userId; + }, + }), + }; + } + if (specifier === '@datadog/apps-backend/runtime') { + return { + setBackend: (runtime: { getUserId(): string }) => { + registeredBackend = runtime; + }, + }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + ); + + expect(result).toEqual({ data: 'done' }); + expect(capturedUserId).toBe('real-user-id'); + }); }); - describe('serialization of concurrent executions', () => { - function delayedResult(label: T, delayMs: number): () => Promise { - return () => new Promise((resolve) => setTimeout(() => resolve(label), delayMs)); - } + describe('non-serializable results', () => { + test('Should reject with a clear, attributed error when the result has a circular reference', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + const o: Record = {}; + o.self = o; + return o; + }, + }), + mockLogger, + ), + ).rejects.toThrow(/example.*can't be serialized to JSON/); + }); - test("Should allow two independent calls to run without cross-contaminating each other's result", async () => { - const [resultA, resultB] = await Promise.all([ + test('Should reject with a clear, attributed error when the result contains a BigInt', async () => { + await expect( executeScriptLocally( func, TEST_PROJECT_ROOT, [], stubExecuteAction, - loadModuleReturning({ example: delayedResult('A', 20) }), + loadModuleReturning({ example: () => BigInt(10) }), mockLogger, ), + ).rejects.toThrow(/example.*can't be serialized to JSON/); + }); + + test('Should reject with a clear, attributed error when the result is a bare function (silently dropped by JSON.stringify)', async () => { + await expect( executeScriptLocally( func, TEST_PROJECT_ROOT, [], stubExecuteAction, - loadModuleReturning({ example: delayedResult('B', 0) }), + loadModuleReturning({ example: () => function notSerializable() {} }), mockLogger, ), - ]); - expect([resultA, resultB]).toEqual([{ data: 'A' }, { data: 'B' }]); + ).rejects.toThrow(/example.*JSON.stringify silently drops/); }); - // Reads $.backendFunctionArgs after a delay, which is what would surface cross-contamination between concurrent calls' globalThis.$. - function readOwnArgsAfterDelay(delayMs: number): () => Promise { - return () => - new Promise((resolve) => - setTimeout(() => resolve(testDollar().backendFunctionArgs), delayMs), - ); - } + test('Should reject with a clear, attributed error when the result is a Map (silently flattened to "{}" by JSON.stringify)', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => new Map([['a', 1]]) }), + mockLogger, + ), + ).rejects.toThrow(/example.*silently flattens/); + }); - // Known race: two concurrent calls both write globalThis.$ synchronously, so the second write wins for both calls' duration. Skip until calls are serialized through an execution queue. - test.skip("Should let each concurrent call see its OWN backendFunctionArgs via globalThis.$, not the other call's", async () => { - const [resultA, resultB] = await Promise.all([ + test('Should reject with a clear, attributed error when the result is a Set (silently flattened to "{}" by JSON.stringify)', async () => { + await expect( executeScriptLocally( func, TEST_PROJECT_ROOT, - ['A-arg'], + [], stubExecuteAction, - loadModuleReturning({ example: readOwnArgsAfterDelay(20) }), + loadModuleReturning({ example: () => new Set([1, 2, 3]) }), mockLogger, ), + ).rejects.toThrow(/example.*silently flattens/); + }); + + test('Should reject with a clear, attributed error when the result is NaN (silently converted to "null" by JSON.stringify)', async () => { + await expect( executeScriptLocally( func, TEST_PROJECT_ROOT, - ['B-arg'], + [], stubExecuteAction, - loadModuleReturning({ example: readOwnArgsAfterDelay(0) }), + loadModuleReturning({ example: () => NaN }), mockLogger, ), - ]); - expect(resultA).toEqual({ data: ['A-arg'] }); - expect(resultB).toEqual({ data: ['B-arg'] }); + ).rejects.toThrow(/example.*silently converts to "null"/); + }); + + test('Should reject with a clear, attributed error when the result is Infinity (silently converted to "null" by JSON.stringify)', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => Infinity }), + mockLogger, + ), + ).rejects.toThrow(/example.*silently converts to "null"/); + }); + + test('Should reject a Map nested inside a plain object, not just at the top level', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => ({ data: new Map([['a', 1]]) }) }), + mockLogger, + ), + ).rejects.toThrow(/example.*silently flattens/); + }); + + test('Should reject a Set nested inside an array, not just at the top level', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => [1, new Set([1, 2, 3])] }), + mockLogger, + ), + ).rejects.toThrow(/example.*silently flattens/); + }); + + test('Should reject a NaN nested inside a plain object, not just at the top level', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => ({ score: NaN }) }), + mockLogger, + ), + ).rejects.toThrow(/example.*silently converts to "null"/); + }); + + test('Should reject a function nested inside a plain object, not just at the top level', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => ({ status: 'ok', callback: () => {} }) }), + mockLogger, + ), + ).rejects.toThrow(/example.*JSON.stringify silently drops/); + }); + + test('Should reject a Symbol-keyed property, which JSON.stringify silently omits with no replacer call at all', async () => { + const secretSymbol = Symbol('secret'); + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => ({ status: 'ok', [secretSymbol]: 'leaked' }), + }), + mockLogger, + ), + ).rejects.toThrow(/example.*Symbol-keyed property/); + }); + + test('Should reject a Symbol-keyed property nested inside an array, not just at the top level', async () => { + const secretSymbol = Symbol('secret'); + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => [{ [secretSymbol]: 'leaked' }] }), + mockLogger, + ), + ).rejects.toThrow(/example.*Symbol-keyed property/); + }); + + test('Should reject an explicit undefined nested inside a plain object, not just at the top level', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => ({ status: 'ok', extra: undefined }) }), + mockLogger, + ), + ).rejects.toThrow(/example.*JSON.stringify silently drops/); + }); + + test('Should reject an explicit undefined at a property literally named the empty string, not mistake it for the JSON root', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => ({ '': undefined, other: 'ok' }) }), + mockLogger, + ), + ).rejects.toThrow(/example.*JSON.stringify silently drops/); + }); + + test('Should reject a function at a property literally named the empty string, not mistake it for the JSON root', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => ({ '': () => {}, other: 'ok' }) }), + mockLogger, + ), + ).rejects.toThrow(/example.*JSON.stringify silently drops/); + }); + + test('Should reject a Symbol nested inside an array, not just at the top level', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => [1, Symbol('unsupported')] }), + mockLogger, + ), + ).rejects.toThrow(/example.*JSON.stringify silently drops/); + }); + + test('Should allow an explicit undefined result through unchanged', async () => { + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => undefined }), + mockLogger, + ); + expect(result).toEqual({ data: undefined }); + }); + + // dev-server.ts serializes the result again for the HTTP response — returning the original (not the parsed round-trip) would invoke a custom toJSON() twice. + test('Should return the JSON-round-tripped value, not the original, so a custom toJSON() is only invoked once', async () => { + let toJsonCallCount = 0; + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => ({ + toJSON() { + toJsonCallCount += 1; + return { callNumber: toJsonCallCount }; + }, + }), + }), + mockLogger, + ); + expect(result).toEqual({ data: { callNumber: 1 } }); + expect(toJsonCallCount).toBe(1); + }); + }); + + describe('serialization of concurrent executions', () => { + beforeEach(() => { + delete (globalThis as Record)[ORDER_MARKER]; + }); + + function recordingOrder(label: string, delayMs: number): () => Promise { + return async () => { + const marker = + ((globalThis as Record)[ORDER_MARKER] as string[]) ?? []; + (globalThis as Record)[ORDER_MARKER] = marker; + marker.push(`start-${label}`); + await new Promise((r) => setTimeout(r, delayMs)); + marker.push(`end-${label}`); + return label; + }; + } + + test('Should never interleave two concurrent executions — the second never starts until the first fully finishes', async () => { + const [resultA, resultB] = await Promise.all([ + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: recordingOrder('A', 20) }), + mockLogger, + ), + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: recordingOrder('B', 0) }), + mockLogger, + ), + ]); + + expect([resultA, resultB]).toEqual([{ data: 'A' }, { data: 'B' }]); + const order = (globalThis as Record)[ORDER_MARKER] as string[]; + // Whichever call runs first, its start/end pair must be adjacent — a real race would interleave as [start-A, start-B, end-B, end-A]. + expect(order).toEqual([ + expect.stringMatching(/^start-/), + expect.stringMatching(/^end-/), + expect.stringMatching(/^start-/), + expect.stringMatching(/^end-/), + ]); + expect(order[0].slice('start-'.length)).toEqual(order[1].slice('end-'.length)); + expect(order[2].slice('start-'.length)).toEqual(order[3].slice('end-'.length)); + }); + + function readOwnArgsAfterDelay(delayMs: number): () => Promise { + return () => + new Promise((resolve) => + setTimeout(() => resolve(testDollar().backendFunctionArgs), delayMs), + ); + } + + // globalThis.$ is scoped per call via AsyncLocalStorage, independent of the enqueue queue (which exists for the action-catalog/apps-backend module-singleton race). + test("Should let each concurrent call see its OWN backendFunctionArgs via globalThis.$, not the other call's", async () => { + const [resultA, resultB] = await Promise.all([ + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + ['A-arg'], + stubExecuteAction, + loadModuleReturning({ example: readOwnArgsAfterDelay(20) }), + mockLogger, + ), + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + ['B-arg'], + stubExecuteAction, + loadModuleReturning({ example: readOwnArgsAfterDelay(0) }), + mockLogger, + ), + ]); + expect(resultA).toEqual({ data: ['A-arg'] }); + expect(resultB).toEqual({ data: ['B-arg'] }); + }); + + test('Should still run the next queued execution after an earlier one rejects', async () => { + const first = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + throw new Error('first fails'); + }, + }), + mockLogger, + ); + const second = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => 2 }), + mockLogger, + ); + + await expect(first).rejects.toThrow('first fails'); + await expect(second).resolves.toEqual({ data: 2 }); + }); + + // Covers the raw-$.Actions path: a captured Actions reference must reject once abandoned, even after globalThis.$ is overwritten by a newer execution. + test('Should reject a captured $.Actions reference once its own execution is abandoned, even after a newer execution has taken over', async () => { + let abandonedCallOutcome: 'pending' | 'resolved' | { rejected: string } = 'pending'; + + const abandoned = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: async () => { + // Captured BEFORE the timeout fires — this execution's own Actions proxy, not whatever globalThis.$ points to later. + const { Actions } = testDollar(); + // Outlives the 20ms timeout below, so the caller already sees a rejection by the time this line runs. + await new Promise((resolve) => setTimeout(resolve, 100)); + try { + await Actions.foo.bar({ inputs: {} }); + abandonedCallOutcome = 'resolved'; + } catch (err) { + abandonedCallOutcome = { + rejected: err instanceof Error ? err.message : String(err), + }; + } + return { data: 'abandoned' }; + }, + }), + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + // The queue is free as soon as the timeout wins — the second execution starts and completes normally, becoming "current". + const second = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => 'second' }), + mockLogger, + ); + expect(second).toEqual({ data: 'second' }); + + // Give the abandoned execution's background timer room to fire its action call before asserting on the outcome. + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(abandonedCallOutcome).toEqual({ + rejected: expect.stringContaining('already concluded'), + }); + }); + + test("Should resolve a zombie execution's FRESH read of globalThis.$ to its OWN identity, never a newer execution's — even while that newer execution is still in flight", async () => { + const funcA: BackendFunction = { ...func, allowedConnectionIds: ['conn-A'] }; + const funcB: BackendFunction = { ...func, allowedConnectionIds: ['conn-B'] }; + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + + let zombieOutcome: 'pending' | 'resolved' | { rejected: string } = 'pending'; + + const abandoned = executeScriptLocally( + funcA, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: async () => { + // Fires ~60ms in, inside funcB's in-flight window — a fresh $ read here needs AsyncLocalStorage or it would resolve to funcB's $. + await new Promise((resolve) => setTimeout(resolve, 60)); + const $ = testDollar(); + try { + // funcB's own connectionId, not funcA's — only valid if this call incorrectly runs under funcB's still-live identity. + await $.Actions.foo.bar({ inputs: {}, connectionId: 'conn-B' }); + zombieOutcome = 'resolved'; + } catch (err) { + zombieOutcome = { + rejected: err instanceof Error ? err.message : String(err), + }; + } + return 'zombie-done'; + }, + }), + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + // Stays "current" for 80ms, overlapping the zombie's 60ms wakeup; never calls $.Actions itself, so any observed call must be the zombie's. + const second = executeScriptLocally( + funcB, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: async () => { + await new Promise((resolve) => setTimeout(resolve, 80)); + return 'second'; + }, + }), + mockLogger, + ); + await expect(second).resolves.toEqual({ data: 'second' }); + + // The zombie's fresh read resolved to its own $ (funcA's allowedConnectionIds), so funcB's connectionId is rejected before reaching executeAction. + expect(zombieOutcome).toEqual({ + rejected: expect.stringContaining("not in this function's allowed connections"), + }); + expect(executeAction).not.toHaveBeenCalled(); + }); + + // The dispatcher resolves the calling execution's dispatch from AsyncLocalStorage at call time — a per-closure guard alone would be bypassed once a newer execution re-registers. + test("Should reject an abandoned execution's action-catalog typed-wrapper call, not silently run it under a newer registration", async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + let abandonedCallOutcome: 'pending' | 'resolved' | { rejected: string } = 'pending'; + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { + example: async () => { + await new Promise((resolve) => setTimeout(resolve, 100)); + try { + await registeredImpl?.('com.datadoghq.foo.bar', { inputs: {} }); + abandonedCallOutcome = 'resolved'; + } catch (err) { + abandonedCallOutcome = { + rejected: err instanceof Error ? err.message : String(err), + }; + } + return { data: 'abandoned' }; + }, + }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + + const abandoned = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + // No second execution registers here — the call is rejected because the dispatcher resolves this execution's own dispatch, already concluded by the 20ms timeout. + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(abandonedCallOutcome).toEqual({ + rejected: expect.stringContaining('already concluded'), + }); + }); + + // registeredImpl points at funcB's registration once it registers, but a call from within funcA's own continuation must still resolve funcA's concluded dispatch via AsyncLocalStorage and be rejected, not routed through funcB's identity. + test("Should reject a zombie execution's action-catalog typed-wrapper call even after a newer execution has legitimately re-registered its own implementation", async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + const funcA: BackendFunction = { ...func, allowedConnectionIds: ['conn-A'] }; + const funcB: BackendFunction = { ...func, allowedConnectionIds: ['conn-B'] }; + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + let zombieOutcome: 'pending' | 'resolved' | { rejected: string } = 'pending'; + + const makeLoadModule = (exampleImpl: () => Promise): LoadModule => { + return async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { example: exampleImpl }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + }; + + // Times out at 20ms, then calls the typed wrapper ~60ms in — inside funcB's in-flight window — using conn-B, a connection funcA is never allowed to use. + const abandoned = executeScriptLocally( + funcA, + TEST_PROJECT_ROOT, + [], + executeAction, + makeLoadModule(async () => { + await new Promise((resolve) => setTimeout(resolve, 60)); + try { + await registeredImpl?.('com.datadoghq.foo.bar', { + inputs: {}, + connectionId: 'conn-B', + }); + zombieOutcome = 'resolved'; + } catch (err) { + zombieOutcome = { + rejected: err instanceof Error ? err.message : String(err), + }; + } + return 'zombie-done'; + }), + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + // Starts as soon as the queue frees, registers immediately, but doesn't conclude until 80ms — overlapping funcA's 60ms zombie wakeup. + const second = executeScriptLocally( + funcB, + TEST_PROJECT_ROOT, + [], + executeAction, + makeLoadModule(() => new Promise((resolve) => setTimeout(() => resolve('B'), 80))), + mockLogger, + ); + await expect(second).resolves.toEqual({ data: 'B' }); + + // funcB's own registration checks conn-B against funcB's allowedConnectionIds, which passes — the zombie call must not be allowed to reach that registration at all. + expect(zombieOutcome).toEqual({ + rejected: expect.stringContaining('already concluded'), + }); + expect(executeAction).not.toHaveBeenCalled(); + }); + + // The apps-backend loadModule hangs forever, so a post-Promise.all destructuring would never run — publishing each handle via its own .then() is what lets the completed action-catalog registration still take effect. + test('Should still register the action-catalog adapter even when the sibling apps-backend registration never settles, and reject a call once no execution is active', async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + jest.spyOn(shared, 'isDatadogAppsBackendInstalled').mockReturnValue(true); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { example: () => 'unused' }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + if ( + specifier === '@datadog/apps-backend/runtime/jsFunctionWithActions' || + specifier === '@datadog/apps-backend/runtime' + ) { + return new Promise(() => {}); + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + + const abandoned = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + expect(registeredImpl).toBeDefined(); + await expect(registeredImpl?.('com.datadoghq.foo.bar', { inputs: {} })).rejects.toThrow( + /no active local execution/i, + ); + }); + + // Deliberately reuses one loadModule across both calls (not the usual per-call closure) — a real dev server does the same, so a load that never settles must not permanently poison later executions sharing it. + test('Should let a later execution register and run after an earlier one shared the same loadModule with a registration load that never settles', 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; + if (actionCatalogLoadCount === 1) { + // Simulates a genuinely broken/circular module graph, not just a slow one. + return new Promise(() => {}); + } + return { setExecuteActionImplementation: () => {} }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + + const first = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + 20, + ); + await expect(first).rejects.toThrow(/timed out after 20ms/); + + // Gives the first attempt's own registration timeout (also ~20ms, started microseconds after + // the execution's own timeout above) room to fire and evict its cache entry, the same way a + // real dev server's next request would naturally arrive well after that — not racing the two. + await new Promise((resolve) => setTimeout(resolve, 30)); + + // Without evicting the first attempt's still-pending registration, this would hang until it also times out — never actually invoking its own function. + const second = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + 50, + ); + expect(second).toEqual({ data: 'ok' }); + }); + + // An abandoned execution's fn() can settle normally later — its finally block's conclude step must not disturb whatever a newer execution's own registration already put in place. + test("Should not let a late-settling abandoned execution's own conclusion clobber a newer execution's already-registered action-catalog implementation", async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + + const makeLoadModule = (exampleImpl: () => Promise): LoadModule => { + return async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { example: exampleImpl }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + }; + + // Times out at 20ms, but its own fn() resolves normally ~100ms later, well after being abandoned. + const abandoned = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + makeLoadModule( + () => new Promise((resolve) => setTimeout(() => resolve('A-late'), 100)), + ), + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + // The queue is free as soon as the timeout wins — the second execution registers and finishes well before the abandoned one's 100ms sleep is up. + const second = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + makeLoadModule(() => Promise.resolve('B')), + mockLogger, + ); + expect(second).toEqual({ data: 'B' }); + + // Captures whatever B's own conclusion left registered — B's own registration staying in place after it concludes is fine; nothing else must overwrite it. + const registeredAfterB = registeredImpl; + + // Give the abandoned execution's late-settling fn() and its finally block room to run. + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(registeredImpl).toBe(registeredAfterB); + }); + + // A's slow-to-resolve registration re-installs the same stable dispatcher B already put in place — harmless, since either closure resolves a call against whichever execution is on the AsyncLocalStorage call stack, not against whichever registered it. + test("Should still dispatch correctly after a stale execution's slow-to-resolve registration re-installs the adapter following a newer execution's own registration", async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + + const makeLoadModule = (actionCatalogDelayMs: number): LoadModule => { + return async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { example: () => 'result' }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + if (actionCatalogDelayMs > 0) { + await new Promise((resolve) => + setTimeout(resolve, actionCatalogDelayMs), + ); + } + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + }; + + // Times out at 20ms, well before its own 100ms-delayed action-catalog module load resolves. + const abandoned = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + makeLoadModule(100), + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + // The queue is free as soon as the timeout wins — the second execution registers with no artificial delay, well before A's slow load resolves. + const second = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + makeLoadModule(0), + mockLogger, + ); + expect(second).toEqual({ data: 'result' }); + + // Give A's slow action-catalog load room to finally resolve and re-install the adapter. + await new Promise((resolve) => setTimeout(resolve, 150)); + + // No execution is active at this point — either closure instance correctly rejects the same way. + await expect(registeredImpl?.('com.datadoghq.foo.bar', { inputs: {} })).rejects.toThrow( + /no active local execution/i, + ); + }); + + // An abandoned execution's loadModule/registration steps might still resolve after timeout — proves the customer function is never invoked once already known-stale. + test('Should never invoke the customer function once already known to be abandoned before it starts', async () => { + let callCount = 0; + const slowLoadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + // Slower than the 20ms timeout below — by the time this resolves, the execution is already known-abandoned. + await new Promise((resolve) => setTimeout(resolve, 100)); + return { + example: () => { + callCount += 1; + return 'should never run'; + }, + }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + + const abandoned = executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + slowLoadModule, + mockLogger, + 20, + ); + await expect(abandoned).rejects.toThrow(/timed out after 20ms/); + + // Give the slow loadModule call room to actually resolve. + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(callCount).toBe(0); }); }); }); diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index d23580de7..db083ffda 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -7,11 +7,99 @@ /** Executes a backend function's file directly in-process inside the Vite dev server, mirroring executeScriptViaDatadog's `BackendOutputs` contract in dev-server.ts as a drop-in alternate implementation. */ import type { Logger } from '@dd/core/types'; +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 { createEpochGuard } from './execution-epoch'; + +type BackendGlobals = { + backendFunctionArgs: unknown[]; + Actions: unknown; + Source: ReturnType; +}; + +/** Boxed so a customer module assigning to `globalThis.$` (e.g. `zx/globals`) mutates only its own execution's box, never a concurrent or zombie execution's. */ +type BackendGlobalsBox = { value: unknown }; + +/** Scopes `globalThis.$` per execution via AsyncLocalStorage so a zombie execution's late "fresh" read resolves to its own `$`, never a newer execution's identity. */ +const backendGlobalsContext = new AsyncLocalStorage(); + +/** Whether `$` was installed (e.g. by `zx/globals`) before this module's own accessor below — distinguishes that legitimate passthrough from a customer module reaching for `$` with no prior value, which should fail like production does. */ +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 }>(); + +/** 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, '$'); + +function ensureDollarAccessorInstalled(): void { + if (Object.getOwnPropertyDescriptor(globalThis, '$')?.get === dollarGetter) { + return; + } + Object.defineProperty(globalThis, '$', { + configurable: true, + enumerable: true, + get: dollarGetter, + set: dollarSetter, + }); +} + +function dollarGetter(): unknown { + const box = backendGlobalsContext.getStore(); + if (box) { + return box.value; + } + const loadBox = customerModuleLoadContext.getStore(); + if (loadBox) { + if (loadBox.assigned) { + return loadBox.value; + } + 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. + return undefined; + } + return globalDollarOutsideExecution; +} + +function dollarSetter(value: unknown): void { + const box = backendGlobalsContext.getStore(); + if (box) { + box.value = value; + return; + } + const loadBox = customerModuleLoadContext.getStore(); + if (loadBox) { + // Scoped to this module load, not the shared globalDollarOutsideExecution slot — otherwise a top-level write (e.g. zx/globals) would leak into every later, unrelated load. + loadBox.assigned = true; + loadBox.value = value; + return; + } + globalDollarOutsideExecution = value; +} + +ensureDollarAccessorInstalled(); + +/** What the stable, once-ever-registered adapters below need to dispatch a call to whichever execution is on the AsyncLocalStorage call stack — kept out of `BackendGlobals` since that object is also `globalThis.$`, visible to customer code. */ +type ExecutionDispatch = { + executeAction: ExecuteAction; + allowedConnectionIds: string[]; + isAbandoned: () => boolean; + functionName: string; + $: BackendGlobals; +}; + +/** Distinct from `backendGlobalsContext` so dispatch-only fields (the real `executeAction`, `allowedConnectionIds`) never leak onto `globalThis.$`. */ +const executionDispatchContext = new AsyncLocalStorage(); + interface ActionCallArgs { inputs: Record; connectionId?: string; @@ -22,25 +110,27 @@ function isIndexableRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null; } -/** `globalThis.$` is a runtime-only property TypeScript's built-in `typeof globalThis` has no way to know about — `Reflect.get` reads it without a type assertion, the same way `deleteGlobalDollar` below already avoids one for deletion. */ -function getGlobalDollar(): unknown { - return Reflect.get(globalThis, '$'); -} - -/** `Object.assign`'s signature doesn't require its source object's keys to already exist on the target, so this installs `$` without asserting `globalThis`'s type. */ -function setGlobalDollar(value: unknown): void { - Object.assign(globalThis, { $: value }); -} +export const DEFAULT_TIMEOUT_MS = 10_000; -function deleteGlobalDollar(): void { - Reflect.deleteProperty(globalThis, '$'); -} +/** Bounds a single `$.Actions` call while it's exempt from the hang-detection timer (see `guardedExecuteAction`) — `doRequest` has no deadline of its own, so an unsettled call would wedge this execution, and every serialized request queued behind it, forever. Set generously past `pollQueryExecution`'s worst-case long-poll budget (10 retries × ~30s) so a legitimately slow action is never cut off. */ +const MAX_ACTION_CALL_TIMEOUT_MS = 10 * 60_000; -const DEFAULT_TIMEOUT_MS = 10_000; +/** Absolute ceiling on one execution's wall-clock time, independent of `pendingActionCalls`'s pause-and-extend mechanism (see `guardedExecuteAction`) — that mechanism can't tell a function genuinely awaiting a slow `$.Actions` call from one that fired-and-forgot a call and then hung on something else, so an unawaited call can mask a real hang for up to `MAX_ACTION_CALL_TIMEOUT_MS`. Set just above `pollQueryExecution`'s worst case (~300s) so one legitimate slow call still finishes, bounding the masked-hang case to ~6 minutes rather than the full 10 — going lower would start killing real in-progress calls instead of just hangs. */ +const MAX_TOTAL_EXECUTION_TIMEOUT_MS = 6 * 60_000; /** 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 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, @@ -69,7 +159,7 @@ function assertConnectionIdAllowed( } } -/** Shared validation for both $.Actions entry points (the raw proxy and the action-catalog typed-wrapper dispatcher) — extracted so a future change to this contract can't be applied to one and missed on the other, the exact gap that let the action-catalog path silently forward `inputs: undefined`. */ +/** Shared validation for both $.Actions entry points (raw proxy and action-catalog typed wrapper) — extracted so a contract change can't be applied to one and missed on the other, as happened when the action-catalog path silently forwarded `inputs: undefined`. */ function validateActionCall( call: Partial, allowedConnectionIds: string[], @@ -83,6 +173,21 @@ function validateActionCall( return { inputs, connectionId }; } +/** Serializes local executions — a customer function deleting `globalThis.$` mid-flight would otherwise break `$` access for any other execution concurrently in progress (see `ensureDollarAccessorInstalled`). */ +let queueTail: Promise = Promise.resolve(); + +function enqueue(run: () => Promise): Promise { + const result = queueTail.then(run); + queueTail = result.then( + () => undefined, + () => undefined, + ); + return result; +} + +/** 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), so this guard's generation counter is what rejects that zombie's late dispatch during the overlap, not a redundant backstop. */ +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. */ function makeActionsProxy( executeAction: ExecuteAction, @@ -91,8 +196,8 @@ function makeActionsProxy( ): unknown { return new Proxy(function () {}, { get(_target, prop) { - // A customer function that returns an un-invoked reference (e.g. $.Actions.foo.bar without the trailing call) must not be treated as a thenable — Promise's resolution protocol would call .then() on it and hang until the timeout, since apply() below never settles it. - if (prop === 'then') { + // 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. + if (prop === 'then' || prop === 'toJSON') { return undefined; } return makeActionsProxy( @@ -117,45 +222,118 @@ function makeActionsProxy( }); } -/** No-ops if @datadog/action-catalog isn't installed; checks `isActionCatalogInstalled` up front rather than catching a load failure, since `loadModule` doesn't guarantee an error code for a missing bare specifier. */ -async function registerActionCatalogIfInstalled( +/** 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(`${label} timed out after ${timeoutMs}ms`)); + }, timeoutMs); + promise.then( + (value) => { + clearTimeout(timer); + resolve(value); + }, + (err: unknown) => { + clearTimeout(timer); + reject(err); + }, + ); + }); +} + +/** Keyed by `loadModule` identity, not a module-level flag, so a real dev server's reused `ssrLoadModule` gets true once-ever registration while each test's own closure stays isolated. A rejection (including a load `withTimeout` turns into one) is evicted so the next execution retries instead of staying permanently poisoned. */ +const actionCatalogRegistrations = new WeakMap>(); + +/** No-ops if @datadog/action-catalog isn't installed — the check is re-run uncached on every call, so a mid-session install is picked up on the very next execution. Once installed, registers ONE stable dispatcher that reads `executionDispatchContext.getStore()` at call time, so a zombie's typed-wrapper call can never dispatch under a newer execution's identity just because that execution's registration is the one currently live. */ +function registerActionCatalogIfInstalled( loadModule: LoadModule, projectRoot: string, - executeAction: ExecuteAction, - allowedConnectionIds: string[], + timeoutMs: number, ): Promise { if (!isActionCatalogInstalled(projectRoot)) { - return; + return Promise.resolve(); + } + const existing = actionCatalogRegistrations.get(loadModule); + if (existing) { + return existing; } - const mod = await loadModule('@datadog/action-catalog/action-execution'); + const registration = registerActionCatalogOnce(loadModule, timeoutMs).catch((err) => { + actionCatalogRegistrations.delete(loadModule); + throw err; + }); + actionCatalogRegistrations.set(loadModule, registration); + return registration; +} + +async function registerActionCatalogOnce(loadModule: LoadModule, timeoutMs: number): Promise { + const loadPromise = loadModule('@datadog/action-catalog/action-execution'); + const mod = await withTimeout( + loadPromise, + timeoutMs, + 'Loading @datadog/action-catalog/action-execution', + ); const setExecuteActionImplementation = mod.setExecuteActionImplementation; if (typeof setExecuteActionImplementation !== 'function') { return; } setExecuteActionImplementation(async (actionId: string, request: unknown) => { + const dispatch = executionDispatchContext.getStore(); + if (!dispatch) { + throw new Error(`No active local execution to run "${actionId}" under.`); + } + if (dispatch.isAbandoned()) { + throw new Error( + `Execution of "${dispatch.functionName}" already concluded; refusing to run ` + + `"${actionId}" as this stale execution to avoid using a newer execution's identity.`, + ); + } const call: Partial = isIndexableRecord(request) ? request : {}; const { inputs, connectionId } = validateActionCall( call, - allowedConnectionIds, + dispatch.allowedConnectionIds, `"${actionId}"`, ); - return executeAction(actionId, inputs, connectionId); + return dispatch.executeAction(actionId, inputs, connectionId); }); } -/** No-ops if @datadog/apps-backend isn't installed; see `registerActionCatalogIfInstalled` for why this checks installedness up front rather than catching a load failure. */ -async function registerBackendRuntimeIfInstalled( +/** Mirrors `actionCatalogRegistrations` — same keying and timeout-eviction rationale. */ +const backendRuntimeRegistrations = new WeakMap>(); + +/** Mirrors `registerActionCatalogIfInstalled`'s no-op/re-check/once-ever-registration behavior for @datadog/apps-backend; the registered runtime Proxy resolves whichever execution's `$` is live on the AsyncLocalStorage call stack, rather than binding to one execution's `$` at registration time. */ +function registerBackendRuntimeIfInstalled( loadModule: LoadModule, projectRoot: string, - $: unknown, + timeoutMs: number, ): Promise { if (!isDatadogAppsBackendInstalled(projectRoot)) { - return; + return Promise.resolve(); } - const [jsFunctionWithActionsModule, runtimeModule] = await Promise.all([ + const existing = backendRuntimeRegistrations.get(loadModule); + if (existing) { + return existing; + } + const registration = registerBackendRuntimeOnce(loadModule, timeoutMs).catch((err) => { + backendRuntimeRegistrations.delete(loadModule); + throw err; + }); + backendRuntimeRegistrations.set(loadModule, registration); + return registration; +} + +async function registerBackendRuntimeOnce( + loadModule: LoadModule, + timeoutMs: number, +): Promise { + const loadPromise = Promise.all([ loadModule('@datadog/apps-backend/runtime/jsFunctionWithActions'), loadModule('@datadog/apps-backend/runtime'), ]); + const [jsFunctionWithActionsModule, runtimeModule] = await withTimeout( + loadPromise, + timeoutMs, + 'Loading @datadog/apps-backend/runtime', + ); const buildRuntimeFromJsFunctionWithActions = jsFunctionWithActionsModule.buildRuntimeFromJsFunctionWithActions; const setBackend = runtimeModule.setBackend; @@ -165,10 +343,114 @@ async function registerBackendRuntimeIfInstalled( ) { return; } - setBackend(buildRuntimeFromJsFunctionWithActions($)); + // Cached by dispatch identity, not rebuilt per accessor call — dispatch.$ is fixed for the whole execution. + const runtimeByDispatch = new WeakMap(); + // Forwards whatever shape the real runtime's property has (nested namespace or flat method) rather than assuming every property is callable. + const backendRuntimeProxy = new Proxy( + {}, + { + get(_target, prop) { + const dispatch = executionDispatchContext.getStore(); + if (!dispatch) { + throw new Error( + `No active local execution to resolve an apps-backend accessor under.`, + ); + } + if (dispatch.isAbandoned()) { + throw new Error( + `Execution of "${dispatch.functionName}" already concluded; ` + + `refusing to resolve a further apps-backend accessor under its identity.`, + ); + } + let runtime = runtimeByDispatch.get(dispatch); + if (runtime === undefined) { + runtime = buildRuntimeFromJsFunctionWithActions(dispatch.$); + runtimeByDispatch.set(dispatch, runtime); + } + if (!isIndexableRecord(runtime)) { + return undefined; + } + const value = runtime[String(prop)]; + // A flat method must be bound to the real runtime object, not this Proxy's empty target; a nested namespace is returned as-is since its own methods already bind correctly. + return typeof value === 'function' ? value.bind(runtime) : value; + }, + }, + ); + setBackend(backendRuntimeProxy); +} + +/** Rejects a non-JSON-serializable result (circular reference, `BigInt`, a dropped function/`Symbol`, a `Map`/`Set` flattened to `{}`) here with a clear error, instead of failing downstream when serialized for the HTTP response. */ +// Lets the replacer's already-specific message pass through the outer catch below unwrapped, instead of being replaced by its generic fallback. +class UnsupportedJsonValueError extends Error {} + +/** `JSON.stringify`'s replacer never runs for a symbol-KEYED property (only symbol-valued ones under a string key) — it silently omits them with no callback at all, so they need their own recursive check. */ +function findSymbolKeyedObject(value: unknown, visited: Set): boolean { + if (typeof value !== 'object' || value === null || visited.has(value)) { + return false; + } + if (Object.getOwnPropertySymbols(value).length > 0) { + return true; + } + visited.add(value); + return Object.values(value).some((child) => findSymbolKeyedObject(child, visited)); +} + +function assertJsonSerializable(result: unknown, func: BackendFunction): unknown { + if (findSymbolKeyedObject(result, new Set())) { + throw new Error( + `Local execution of "${func.name}" returned a value with a Symbol-keyed property, which JSON.stringify silently drops instead of serializing — return a plain JSON-compatible value instead.`, + ); + } + 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 `''`. + let isRootCall = true; + serialized = JSON.stringify(result, (key, value) => { + const wasRootCall = isRootCall; + isRootCall = false; + if (value instanceof Map || value instanceof Set) { + throw new UnsupportedJsonValueError( + `Local execution of "${func.name}" returned a ${value.constructor.name}${key ? ` (at "${key}")` : ''}, which JSON.stringify silently flattens to "{}" instead of serializing its entries — return a plain array or object instead.`, + ); + } + if (typeof value === 'number' && !Number.isFinite(value)) { + throw new UnsupportedJsonValueError( + `Local execution of "${func.name}" returned ${value}${key ? ` (at "${key}")` : ''}, which JSON.stringify silently converts to "null" instead of throwing — return a finite number instead.`, + ); + } + if ( + !wasRootCall && + (typeof value === 'function' || typeof value === 'symbol' || value === undefined) + ) { + throw new UnsupportedJsonValueError( + `Local execution of "${func.name}" returned a ${typeof value} (at "${key}"), which JSON.stringify silently drops instead of serializing — return a plain JSON-compatible value instead.`, + ); + } + return value; + }); + } catch (err) { + if (err instanceof UnsupportedJsonValueError) { + throw err; + } + throw new Error( + `Local execution of "${func.name}" returned a value that can't be serialized to JSON: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } + if (serialized === undefined) { + if (result !== undefined) { + throw new Error( + `Local execution of "${func.name}" returned a ${typeof result} value, which JSON.stringify silently drops instead of serializing — return a plain JSON-compatible value instead.`, + ); + } + return undefined; + } + // Return the parsed-and-reserialized value, not the original — the caller serializes again for the HTTP response, and the original would invoke a custom toJSON() a second time. + return JSON.parse(serialized); } -/** `globalThis.$` and the registrations above provide the same customer-visible bindings production's generated wrapper module sets up via text injection. */ +/** `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`. */ export async function executeScriptLocally( func: BackendFunction, projectRoot: string, @@ -177,62 +459,148 @@ export async function executeScriptLocally( loadModule: LoadModule, log: Logger, timeoutMs: number = DEFAULT_TIMEOUT_MS, +): Promise { + return enqueue(() => + runScriptLocally(func, projectRoot, args, executeAction, loadModule, log, timeoutMs), + ); +} + +async function runScriptLocally( + func: BackendFunction, + projectRoot: string, + args: unknown[], + executeAction: ExecuteAction, + loadModule: LoadModule, + log: Logger, + timeoutMs: number, ): 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 scope = executionEpoch.start(); + + // `executeAction`'s long-poll (dev-server.ts's pollQueryExecution) can legitimately outlast + // `timeoutMs` — that's network wait, not a hung customer function. Pausing the hang-detection + // timer while a call is in flight, and giving a fresh `timeoutMs` window once all in-flight + // calls settle, means real `$.Actions` progress is never penalized, while a genuine hang (no + // call in flight) still times out at the usual `timeoutMs`. + 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. + throw new Error( + `Execution of "${func.name}" already concluded; refusing to run ` + + `"${fqn}" as this stale execution to avoid using a newer execution's identity.`, + ); + } + pendingActionCalls += 1; + clearTimeout(timer); + try { + const actionCallPromise = executeAction(fqn, inputs, connectionId); + return await withTimeout( + actionCallPromise, + MAX_ACTION_CALL_TIMEOUT_MS, + `$.Actions call to "${fqn}"`, + ); + } finally { + pendingActionCalls -= 1; + if (pendingActionCalls === 0 && scope.isCurrent()) { + scheduleTimeout(); + } + } + }; + + const concludeExecution = () => { + scope.concludeIfCurrent(); + }; + const $ = { backendFunctionArgs: args, - Actions: makeActionsProxy(executeAction, func.allowedConnectionIds), + Actions: makeActionsProxy(guardedExecuteAction, func.allowedConnectionIds), Source: makeLocalDevSource(), }; + const dispatch: ExecutionDispatch = { + executeAction: guardedExecuteAction, + allowedConnectionIds: func.allowedConnectionIds, + isAbandoned: () => !scope.isCurrent(), + functionName: func.name, + $, + }; + const run = async (): Promise => { // 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 loadModule(func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX); + const mod = await loadCustomerModuleEntry( + loadModule, + func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX, + ); const fn = mod[func.name]; if (typeof fn !== 'function') { throw new Error(`"${func.name}" is not a function exported from ${func.absolutePath}`); } - // Restores whatever globalThis.$ held before this call (or removes it entirely if nothing did) once the execution settles, so a pre-existing global (e.g. from zx/globals) isn't permanently clobbered and a completed execution's own context isn't left reachable by unrelated process code. - const hadPreviousDollar = Object.prototype.hasOwnProperty.call(globalThis, '$'); - const previousDollar = getGlobalDollar(); - setGlobalDollar($); - try { - await Promise.all([ - registerActionCatalogIfInstalled( - loadModule, - projectRoot, - executeAction, - func.allowedConnectionIds, - ), - registerBackendRuntimeIfInstalled(loadModule, projectRoot, $), - ]); - - const result = await fn(...args); - return { data: result }; - } finally { - if (hadPreviousDollar) { - setGlobalDollar(previousDollar); - } else { - deleteGlobalDollar(); - } - } + // 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. + ensureDollarAccessorInstalled(); + + // Scopes globalThis.$ and the dispatch info to this call's own async continuation chain. + return backendGlobalsContext.run({ value: $ }, () => + executionDispatchContext.run(dispatch, async () => { + try { + // Both adapters are stable and idempotent to re-register, so no coordination is needed between them or across executions. + await Promise.all([ + registerActionCatalogIfInstalled(loadModule, projectRoot, timeoutMs), + registerBackendRuntimeIfInstalled(loadModule, projectRoot, timeoutMs), + ]); + + if (!scope.isCurrent()) { + // Already known-abandoned before the customer function was reached — no point invoking it now. + throw new Error( + `Execution of "${func.name}" was abandoned after timing out before it could start.`, + ); + } + const result = await fn(...args); + return { data: assertJsonSerializable(result, func) }; + } finally { + // However this execution ends, mark it concluded so any further dispatch through it — direct or via the shared adapters — is rejected. + concludeExecution(); + } + }), + ); }; - let timer: ReturnType | undefined; const timeout = new Promise((_resolve, reject) => { - timer = setTimeout(() => { - reject(new Error(`Local execution of "${func.name}" timed out after ${timeoutMs}ms`)); - }, timeoutMs); + rejectTimeout = reject; + scheduleTimeout(); }); + // Fires regardless of pendingActionCalls, unlike the pause-and-extend timeout above — bounds the worst case of a fire-and-forget $.Actions call masking an unrelated hang to MAX_TOTAL_EXECUTION_TIMEOUT_MS instead of the per-call MAX_ACTION_CALL_TIMEOUT_MS. + const absoluteTimeoutTimer = setTimeout(() => { + concludeExecution(); + rejectTimeout?.( + new Error( + `Local execution of "${func.name}" exceeded the absolute ${MAX_TOTAL_EXECUTION_TIMEOUT_MS}ms execution ceiling, regardless of any $.Actions call in flight.`, + ), + ); + }, MAX_TOTAL_EXECUTION_TIMEOUT_MS); + // Racing against the timeout only stops the caller from waiting — run() keeps executing in-process afterward, so a customer function that resumes post-timeout can still fire real $.Actions side effects. True cancellation requires terminating a Worker thread, not possible for in-process execution. const runPromise = run(); - // Set once the race below has settled, so the handler right after can tell a genuinely abandoned rejection (caller already gone) from an ordinary one the caller's own `await Promise.race` is about to receive normally. + // 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. let raceSettled = false; - // Nothing awaits runPromise once the timeout has already settled the race — an unhandled rejection from it later would otherwise crash the whole dev server process. Logged (not swallowed silently) so a slow real failure is still diagnosable after the caller has already moved on. + // 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. runPromise.catch((error: unknown) => { if (!raceSettled) { return; @@ -246,5 +614,6 @@ export async function executeScriptLocally( } finally { raceSettled = true; clearTimeout(timer); + clearTimeout(absoluteTimeoutTimer); } } diff --git a/packages/published/esbuild-plugin/package.json b/packages/published/esbuild-plugin/package.json index 3047f1f7c..ae6a471dc 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", @@ -63,6 +64,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 d05a63ae0..14dc73e79 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", @@ -66,6 +67,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 85a14619a..f8888e587 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", @@ -63,6 +64,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 05c805e65..3b11402a6 100644 --- a/packages/published/vite-plugin/package.json +++ b/packages/published/vite-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", @@ -63,6 +64,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/webpack-plugin/package.json b/packages/published/webpack-plugin/package.json index 148e40ffe..0455d4dc2 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", @@ -63,6 +64,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..6476cbc32 --- /dev/null +++ b/packages/tests/src/_jest/fixtures/action_catalog_project/index.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. + +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..46a9b5daa --- /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..260ef5b6e --- /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/mixedImports.backend.ts b/packages/tests/src/_jest/fixtures/apps_backend_project/mixedImports.backend.ts new file mode 100644 index 000000000..450f8810f --- /dev/null +++ b/packages/tests/src/_jest/fixtures/apps_backend_project/mixedImports.backend.ts @@ -0,0 +1,19 @@ +// 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 specifically, not a local one — a +// local dynamic import is already caught by the fail-closed +// unsupportedDependencies check, which would mask the bug this fixture +// exists to exercise (silent misattribution of the static import textually +// after it, not a thrown error). +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..bdeb98437 --- /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/viaHelper.backend.ts b/packages/tests/src/_jest/fixtures/apps_backend_project/viaHelper.backend.ts new file mode 100644 index 000000000..68e728635 --- /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..b92a13683 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, @@ -120,6 +125,27 @@ export const getMockTimeLogger = (overrides: Partial = {}): TimeLogg return mockTimer; }; +/** + * Builds a `loadModule`-shaped resolver that returns `exports` for `func`'s + * absolute path (as requested by local execution's own suffixed specifier — + * see `LOCAL_EXECUTION_LOAD_SUFFIX`) and rejects any other specifier — + * matching what a real module loader returns when only the target module is + * actually 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; + }; +}; + 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..04db8dca5 100644 --- a/packages/tools/src/rollupConfig.mjs +++ b/packages/tools/src/rollupConfig.mjs @@ -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,44 @@ 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, + // A plain string in Rollup's `external` array only matches an id + // exactly (see Rollup's own `getIdMatcher`) — it does not also match + // a deep import of that package (e.g. declaring `rollup` external + // doesn't cover `rollup/parseAst`). Once `@rollup/plugin-node-resolve` + // resolves a deep import to its real absolute file path, the + // exact-match check no longer sees the original bare specifier at + // all, so a package's own subpath export would otherwise get inlined + // despite being declared as a dependency specifically so it stays + // external. + 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 || []).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/yarn.lock b/yarn.lock index 3d9228437..44fed7c02 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1976,6 +1976,7 @@ __metadata: "@types/eslint-scope": "npm:3.7.7" "@types/estree": "npm:1.0.8" chalk: "npm:2.3.1" + esbuild: "npm:0.25.8" eslint-scope: "npm:7.2.2" glob: "npm:11.1.0" jszip: "npm:3.10.1"