diff --git a/packages/plugins/apps/README.md b/packages/plugins/apps/README.md index 977d57251..da16a417c 100644 --- a/packages/plugins/apps/README.md +++ b/packages/plugins/apps/README.md @@ -11,6 +11,7 @@ A Vite plugin that builds a deployable Datadog Apps package. Publishing is owned - [Configuration](#configuration) - [Development server authentication](#development-server-authentication) - [Development server requests](#development-server-requests) +- [Cloud execution](#cloud-execution) - [Package output](#package-output) - [apps.enable](#appsenable) - [apps.include](#appsinclude) @@ -58,6 +59,12 @@ The dev server's backend function endpoints (`/__dd/executeAction`, `/__dd/execu To call one by hand, send that content type, for example `curl -X POST -H 'Content-Type: application/json' -d '{"functionName":"..."}' http://localhost:5173/__dd/debugBundle`. +## Cloud execution + +Cloud execution (`dev:verify`, `/__dd/executeActionViaCloud`) bundles a backend function with the production settings a `vite build` upload gets, though the dev server runs under `NODE_ENV=development`, so it sends the bytes an upload would; `/__dd/debugBundle` returns that bundle, minified with names kept. Two cases still follow the dev server's `NODE_ENV`: package export conditions on Vite 5, which doesn't export its default server conditions, and Vite's stub for an optional peer dependency the app hasn't installed. + +When Datadog rejects a bundle as longer than the `jsFunctionWithActions` `script` input accepts, the dev server answers with a 413 naming the function, its size and Datadog's answer, and warns once per function until the dev server restarts. + ## Package output A production `vite build` writes `datadog-app-assets.zip` beside the Vite output. The ZIP contains `frontend/`, `backend/`, and `manifest.json`. The app's identity is resolved by `@datadog/apps-cli` at deploy time. diff --git a/packages/plugins/apps/src/vite/build-backend-functions.ts b/packages/plugins/apps/src/vite/build-backend-functions.ts index a11818400..0bf9da2c3 100644 --- a/packages/plugins/apps/src/vite/build-backend-functions.ts +++ b/packages/plugins/apps/src/vite/build-backend-functions.ts @@ -20,10 +20,6 @@ import { getBaseBackendBuildConfig } from './build-config'; const VIRTUAL_PREFIX = '\0dd-backend:'; export const BACKEND_OUT_DIR_PREFIX = 'dd-apps-backend-'; -// Rolldown (Vite 8) output option that keeps function and class names when minifying. Rollup's -// types don't declare it, and Rollup-based Vite ignores it, so it is spread in rather than typed. -const ROLLDOWN_KEEP_NAMES = { keepNames: true } satisfies Record; - /** * Build all backend functions using a separate vite.build() call. * Produces one standalone JS file per function in a temp directory. @@ -74,17 +70,8 @@ export async function buildBackendFunctions( // eslint-disable-next-line no-await-in-loop const result = await viteBuild({ ...baseConfig, - // Production only: dev-server bundles stay unminified so local traces keep line - // numbers. Function and class names are kept for function-log stack frames and - // code reading `.name`; frames still point at `1:`. - // - // `minify: true` selects each Vite major's own minifier: esbuild before Vite 8, - // Oxc in Rolldown-based Vite 8, which only loads esbuild if the app installs it. - // Each reads its own keepNames option and ignores the other's. - esbuild: { keepNames: true }, build: { ...baseConfig.build, - minify: true, write: true, outDir, emptyOutDir: false, @@ -93,7 +80,6 @@ export async function buildBackendFunctions( input: { [bundleName]: virtualId }, output: { ...baseConfig.build.rollupOptions.output, - ...ROLLDOWN_KEEP_NAMES, entryFileNames: '[name].js', }, }, diff --git a/packages/plugins/apps/src/vite/build-config.test.ts b/packages/plugins/apps/src/vite/build-config.test.ts index 947584f98..1c3d61fa8 100644 --- a/packages/plugins/apps/src/vite/build-config.test.ts +++ b/packages/plugins/apps/src/vite/build-config.test.ts @@ -7,9 +7,13 @@ import { outputFileSync, rmSync } from '@dd/core/helpers/fs'; import { getTempWorkingDir } from '@dd/tests/_jest/helpers/env'; -import { build } from 'vite'; +import { build, defaultServerConditions } from 'vite'; -import { getBaseBackendBuildConfig } from './build-config'; +import { + DEV_PROD_CONDITION, + getBaseBackendBuildConfig, + withProductionSettings, +} from './build-config'; describe('getBaseBackendBuildConfig', () => { test('bundles a backend function that imports a real Node builtin module with a working import, not a browser-external stub', async () => { @@ -58,7 +62,7 @@ describe('getBaseBackendBuildConfig', () => { const code = chunk.type === 'chunk' ? chunk.code : ''; // The browser-external stub has no real exports and rewrites away the import specifier. - expect(code).toContain("from 'node:crypto'"); + expect(code).toMatch(/from\s*["']node:crypto["']/); expect(code).not.toContain('__vite-browser-external'); } finally { rmSync(workingDir); @@ -108,6 +112,8 @@ describe('getBaseBackendBuildConfig', () => { ...baseConfig, build: { ...baseConfig.build, + // The minifier's own dead-code removal would otherwise hide a tree-shaking regression. + minify: false, write: false, rollupOptions: { ...baseConfig.build.rollupOptions, input: virtualId }, }, @@ -243,3 +249,33 @@ describe('getBaseBackendBuildConfig', () => { } }); }); + +describe('withProductionSettings', () => { + const baseConfig = getBaseBackendBuildConfig('/project', {}); + + test("Should resolve Vite's default server conditions to production", () => { + const config = withProductionSettings(baseConfig, defaultServerConditions); + + const { resolve, ...ssr } = config.ssr ?? {}; + const conditions = resolve?.conditions; + const otherConditions = defaultServerConditions.filter((c) => c !== DEV_PROD_CONDITION); + expect(defaultServerConditions).toContain(DEV_PROD_CONDITION); + expect(ssr).toEqual(baseConfig.ssr); + const expectedConditions = expect.arrayContaining(['production', ...otherConditions]); + expect(conditions).toEqual(expectedConditions); + expect(conditions).toHaveLength(defaultServerConditions.length); + expect(config.define).toEqual({ + ...baseConfig.define, + 'import.meta.env.DEV': 'false', + 'import.meta.env.PROD': 'true', + }); + expect(config.esbuild).toEqual({ ...baseConfig.esbuild, jsxDev: false }); + }); + + test("Should keep Vite's own conditions when it exports no defaults", () => { + const config = withProductionSettings(baseConfig, undefined); + + expect(config.ssr).toEqual(baseConfig.ssr); + expect(config.esbuild).toEqual({ ...baseConfig.esbuild, jsxDev: false }); + }); +}); diff --git a/packages/plugins/apps/src/vite/build-config.ts b/packages/plugins/apps/src/vite/build-config.ts index 589cd3d3e..841227cc6 100644 --- a/packages/plugins/apps/src/vite/build-config.ts +++ b/packages/plugins/apps/src/vite/build-config.ts @@ -6,6 +6,41 @@ import type { BuildOptions, InlineConfig, Plugin } from 'vite'; import { BACKEND_CODE_EXTENSIONS } from '../constants'; +// Rolldown (Vite 8) output option that keeps function and class names when minifying. Rollup's +// types don't declare it, and Rollup-based Vite ignores it, so it is spread in rather than typed. +const ROLLDOWN_KEEP_NAMES = { keepNames: true } satisfies Record; + +export const DEV_PROD_CONDITION = 'development|production'; + +type BaseBackendBuildConfig = ReturnType; + +/** + * Vite takes export conditions, `import.meta.env.DEV`/`PROD` and the JSX dev transform from + * `process.env.NODE_ENV`, which a dev server sets to development. Pinning them makes a bundle built + * there match a `vite build` upload, which runs under production. `serverConditions` is Vite's + * `defaultServerConditions`, which Vite 5 doesn't export; no option pins its missing-peer stub. + */ +export function withProductionSettings( + config: BaseBackendBuildConfig, + serverConditions: readonly string[] | undefined, +): BaseBackendBuildConfig { + const conditions = serverConditions?.map((condition) => + condition === DEV_PROD_CONDITION ? 'production' : condition, + ); + const resolve = conditions && { resolve: { ...config.ssr?.resolve, conditions } }; + return { + ...config, + define: { + ...config.define, + 'import.meta.env.DEV': 'false', + 'import.meta.env.PROD': 'true', + }, + // Vite 8 converts esbuild options to Oxc's, jsxDev included. + esbuild: { ...config.esbuild, jsxDev: false }, + ssr: { ...config.ssr, ...resolve }, + }; +} + /** * Create the virtual module resolver plugin used by both production and dev builds. * Maps virtual IDs to their generated source content. @@ -30,9 +65,9 @@ export function createVirtualPlugin(name: string, virtualEntries: Record { getMockLogger(), 'development', isHostAllowed, + defaultServerConditions, ); const req = createMockRequest('/__dd/executeAction', { @@ -254,6 +263,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { getMockLogger(), 'development', isHostAllowed, + defaultServerConditions, ); const req = createMockRequest('/__dd/executeAction', { @@ -293,6 +303,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { getMockLogger(), 'development', isHostAllowed, + defaultServerConditions, ); const req = createMockRequest('/__dd/executeAction', { @@ -371,6 +382,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { getMockLogger(), 'development', isHostAllowed, + defaultServerConditions, ); const req = createMockRequest('/__dd/executeAction', { @@ -423,6 +435,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { getMockLogger(), 'development', isHostAllowed, + defaultServerConditions, ); // The connection-ID collector is under test here, not the preview-async round trip @@ -484,6 +497,7 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { getMockLogger(), 'development', isHostAllowed, + defaultServerConditions, ); const apiScope = nock('https://api.datadoghq.com') @@ -509,6 +523,207 @@ describe('Dev Server Middleware — real end-to-end local execution', () => { }, 30000); }); +describe('Dev Server Middleware — dev:verify bundles', () => { + const originalNodeEnv = process.env.NODE_ENV; + + afterEach(() => { + nock.cleanAll(); + if (originalNodeEnv === undefined) { + delete process.env.NODE_ENV; + } else { + process.env.NODE_ENV = originalNodeEnv; + } + }); + + // Each build runs under the NODE_ENV its real process has: `vite build` sets production, a dev + // server development. + async function buildUploadAndVerify(workingDir: string, func: BackendFunction) { + const log = getMockLogger(); + process.env.NODE_ENV = 'production'; + const upload = await buildBackendFunctions(build, [func], workingDir, log); + let uploadCode: string; + try { + const [uploadPath] = [...upload.outputs.values()]; + uploadCode = readFileSync(uploadPath); + } finally { + rmSync(upload.outDir); + } + + process.env.NODE_ENV = 'development'; + const submitted: { script?: unknown } = {}; + const apiScope = nock('https://api.datadoghq.com') + .post('/api/v2/app-builder/queries/preview-async', (body) => { + submitted.script = body.data.attributes.query.properties.spec.inputs.script; + return true; + }) + .reply(200, { data: { id: 'receipt-verify-bundle' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-verify-bundle') + .reply(200, { data: { attributes: { done: true, outputs: { data: { ok: true } } } } }); + const isHostAllowed = createViteHostCheck({ server: { allowedHosts: [] }, preview: {} }); + const middleware = createDevServerMiddleware( + build, + async () => ({}), + () => [func], + async () => [], + { site: 'datadoghq.com' } satisfies AuthOptionsWithDefaults, + testApiKeyRequest, + mockLongPolling, + workingDir, + log, + DEV_VERIFY_MODE, + isHostAllowed, + defaultServerConditions, + ); + const functionName = encodeQueryName(func); + const req = createMockRequest(EXECUTE_ACTION_ROUTE, { functionName, args: ['QA'] }); + const res = createMockResponse(); + const next = jest.fn(); + middleware(req, res, next); + await res.done; + + const apiDone = apiScope.isDone(); + return { + uploadCode, + submittedScript: submitted.script, + statusCode: res.statusCode, + apiDone, + }; + } + + test('Should submit the minified bytes an upload writes', async () => { + const seed = `dev-verify-minify-${Date.now()}`; + const workingDir = getTempWorkingDir(seed); + try { + const absolutePath = `${workingDir}/src/names.backend.ts`; + outputFileSync( + absolutePath, + ` + class BackendValidationError extends Error {} + const readableHelper = (value: string) => { + if (!value) throw new BackendValidationError('missing value'); + return value; + }; + export async function readableBackendFunction(value: string) { + return readableHelper(value); + } + `, + ); + const namesFunc: BackendFunction = { + relativePath: 'src/names', + name: 'readableBackendFunction', + absolutePath, + allowedConnectionIds: [], + }; + + const result = await buildUploadAndVerify(workingDir, namesFunc); + + // Minified, with names kept: the unminified output spans many lines. + const uploadLineCount = result.uploadCode.trim().split('\n').length; + expect(result.statusCode).toBe(200); + expect(result.apiDone).toBe(true); + expect(result.submittedScript).toBe(result.uploadCode); + expect(uploadLineCount).toBeLessThan(5); + expect(result.uploadCode).toContain('"BackendValidationError"'); + } finally { + rmSync(workingDir); + } + }, 30000); + + test('Should submit the production bytes an upload writes, under the dev server NODE_ENV', async () => { + const seed = `dev-verify-production-${Date.now()}`; + const workingDir = getTempWorkingDir(seed); + try { + const packageDir = `${workingDir}/node_modules/dd-conditional-flavor`; + const packageJson = { + name: 'dd-conditional-flavor', + type: 'module', + exports: { + '.': { + development: './development.js', + production: './production.js', + default: './production.js', + }, + }, + }; + const packageJsonText = JSON.stringify(packageJson); + outputFileSync(`${packageDir}/package.json`, packageJsonText); + outputFileSync(`${packageDir}/development.js`, "export const flavor = 'dev-export';"); + outputFileSync(`${packageDir}/production.js`, "export const flavor = 'prod-export';"); + const absolutePath = `${workingDir}/src/flavors.backend.ts`; + outputFileSync( + absolutePath, + ` + import { flavor } from 'dd-conditional-flavor'; + export async function flavors() { + const viteFlavor = import.meta.env.DEV ? 'vite-dev' : 'vite-prod'; + const prodFlavor = import.meta.env.PROD ? 'prod-true' : 'prod-false'; + const nodeFlavor = process.env.NODE_ENV === 'production' ? 'node-prod' : 'node-dev'; + return [flavor, viteFlavor, prodFlavor, nodeFlavor, import.meta.env.MODE]; + } + `, + ); + const flavorsFunc: BackendFunction = { + relativePath: 'src/flavors', + name: 'flavors', + absolutePath, + allowedConnectionIds: [], + }; + + const result = await buildUploadAndVerify(workingDir, flavorsFunc); + + expect(result.statusCode).toBe(200); + expect(result.apiDone).toBe(true); + expect(result.uploadCode).toContain('prod-export'); + expect(result.uploadCode).not.toContain('dev-export'); + expect(result.submittedScript).toBe(result.uploadCode); + } finally { + rmSync(workingDir); + } + }, 30000); + + test('Should submit the production JSX transform an upload writes, under the dev server NODE_ENV', async () => { + const seed = `dev-verify-jsx-${Date.now()}`; + const workingDir = getTempWorkingDir(seed); + try { + const reactDir = `${workingDir}/node_modules/react`; + const reactPackage = { + name: 'react', + type: 'module', + exports: { + './jsx-runtime': './jsx-runtime.js', + './jsx-dev-runtime': './jsx-dev-runtime.js', + }, + }; + const reactPackageText = JSON.stringify(reactPackage); + outputFileSync(`${reactDir}/package.json`, reactPackageText); + outputFileSync(`${reactDir}/jsx-runtime.js`, 'export const jsx = (type) => type;'); + outputFileSync( + `${reactDir}/jsx-dev-runtime.js`, + 'export const jsxDEV = (type) => type;', + ); + const tsconfig = { compilerOptions: { jsx: 'react-jsx' } }; + const tsconfigText = JSON.stringify(tsconfig); + outputFileSync(`${workingDir}/tsconfig.json`, tsconfigText); + const absolutePath = `${workingDir}/src/markup.backend.tsx`; + outputFileSync(absolutePath, 'export async function markup() { return
; }'); + const markupFunc: BackendFunction = { + relativePath: 'src/markup', + name: 'markup', + absolutePath, + allowedConnectionIds: [], + }; + + const result = await buildUploadAndVerify(workingDir, markupFunc); + + expect(result.statusCode).toBe(200); + expect(result.uploadCode).not.toContain('jsxDEV'); + expect(result.submittedScript).toBe(result.uploadCode); + } finally { + rmSync(workingDir); + } + }, 30000); +}); + // Uses its own dev server rooted in a temp dir, since writing files under the shared fixtures tree // would race other test files that copy that whole tree in parallel. const FAKE_SDK_NAME = 'dd-fake-stateful-sdk'; diff --git a/packages/plugins/apps/src/vite/dev-server.test.ts b/packages/plugins/apps/src/vite/dev-server.test.ts index 0acc186cf..cbd9c34ab 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -5,6 +5,7 @@ /* global globalThis */ import { getAuthenticatedRequest } from '@dd/apps-plugin/auth'; +import { buildBackendFunctions } from '@dd/apps-plugin/vite/build-backend-functions'; import { createViteHostCheck } from '@dd/apps-plugin/vite/dev-server-request-guard'; import { createDevServerMiddleware, @@ -12,8 +13,10 @@ import { DEV_SERVER_ROUTES, formatRef, getRetryDelay, + oversizedBundleMessage, } from '@dd/apps-plugin/vite/dev-server'; import { withTimeout } from '@dd/apps-plugin/vite/local-execution'; +import { rmSync } from '@dd/core/helpers/fs'; import type { AuthOptionsWithDefaults, RequestOpts } from '@dd/core/types'; import { createMockRequest, @@ -25,6 +28,7 @@ import { import { emitModuleParsed } from '@dd/tests/_jest/helpers/moduleParsed'; import type { IncomingMessage } from 'http'; import nock from 'nock'; +import type { InlineConfig } from 'vite'; import { encodeQueryName } from '../backend/encodeQueryName'; import type { BackendFunction } from '../backend/types'; @@ -286,6 +290,7 @@ function createTestMiddleware( log?: CreateDevServerMiddlewareArgs[8]; mode?: CreateDevServerMiddlewareArgs[9]; isHostAllowed?: CreateDevServerMiddlewareArgs[10]; + serverConditions?: CreateDevServerMiddlewareArgs[11]; } = {}, ): ReturnType { return createDevServerMiddleware( @@ -302,6 +307,7 @@ function createTestMiddleware( overrides.log ?? mockLog, overrides.mode ?? 'development', overrides.isHostAllowed ?? localOnlyHostCheck, + overrides.serverConditions, ); } @@ -647,11 +653,50 @@ describe('Dev Server Middleware', () => { logLevel: 'silent', build: expect.objectContaining({ write: false, - minify: false, }), }), ); }); + + test('Should bundle with the minify and keepNames settings an upload uses', async () => { + const [func] = mockFunctions; + const uploadBuild = jest.fn(); + uploadBuild.mockImplementation(async (config) => { + const source = `export function ${func.name}() { return null; }`; + await emitModuleParsed(config, func.absolutePath, source); + return { output: [] }; + }); + const upload = await buildBackendFunctions(uploadBuild, [func], '/project', mockLog); + rmSync(upload.outDir); + mockBuildWithParsedBackend(); + const functionName = encodeQueryName(func); + const req = createMockRequest('/__dd/debugBundle', { functionName }); + const res = createMockResponse(); + const next = jest.fn(); + + middleware(req, res, next); + await res.done; + + const [[uploadConfig]] = uploadBuild.mock.calls; + const [[devConfig]] = mockViteBuild.mock.calls; + const minifySettings = (config: InlineConfig) => { + const output = config.build?.rollupOptions?.output; + const hasKeepNames = output && !Array.isArray(output) && 'keepNames' in output; + return { + esbuildKeepNames: config.esbuild ? config.esbuild.keepNames : undefined, + minify: config.build?.minify, + outputKeepNames: hasKeepNames ? output.keepNames : undefined, + }; + }; + const devSettings = minifySettings(devConfig); + const uploadSettings = minifySettings(uploadConfig); + expect(devSettings).toEqual(uploadSettings); + expect(devSettings).toEqual({ + esbuildKeepNames: true, + minify: true, + outputKeepNames: true, + }); + }); }); describe('executeActionViaCloud handler', () => { @@ -1148,6 +1193,156 @@ describe('Dev Server Middleware', () => { }); }); + describe('oversized bundle rejection', () => { + const BUNDLED_CODE = 'export async function main($) { return "😀"; }'; + const BUNDLE_LENGTH = Array.from(BUNDLED_CODE).length; + const SCRIPT_LIMIT = BUNDLE_LENGTH - 1; + const SCRIPT_REJECTION = `.script: length must be <= ${SCRIPT_LIMIT}, but got ${BUNDLE_LENGTH}`; + + // Staging's answer to a jsFunctionWithActions script over the limit, with its numbers swapped in. + function inputRejection(fieldError: string) { + return { + errors: [ + { + status: '400', + id: 'c391b376-bfec-4dd4-b415-cc9cd0fc4aa7', + code: 'INVALID_INPUT_PARAMETER', + title: 'error running query', + detail: `input parameter validation error: 1 error occurred:\n\t* ${fieldError}\n\n`, + }, + ], + }; + } + + function rejectSubmit(body: ReturnType, times = 1) { + return nock(DD_API_ORIGIN) + .post('/api/v2/app-builder/queries/preview-async') + .times(times) + .reply(400, body); + } + + async function callViaCloud( + middleware: ReturnType, + func = mockFunctions[0], + url = '/__dd/executeActionViaCloud', + ) { + const functionName = encodeQueryName(func); + const req = createMockRequest(url, { functionName, args: [] }); + const res = createMockResponse(); + const next = jest.fn(); + middleware(req, res, next); + await res.done; + const responseBody = res.getBody(); + const body = JSON.parse(responseBody); + return { statusCode: res.statusCode, body }; + } + + function getWarnings() { + return mockLogFn.mock.calls + .filter(([, level]) => level === 'warn') + .map(([text]) => text); + } + + test('Should name the function, its size, Datadog answer and how to shrink it', () => { + const message = oversizedBundleMessage('src/a/run', 12, SCRIPT_REJECTION); + + expect(message).toContain('Backend function "src/a/run" is larger than production'); + expect(message).toContain('its minified bundle is 12 characters'); + expect(message).toContain(`"${SCRIPT_REJECTION}"`); + expect(message).toContain('splitting the function or moving large data out of it'); + }); + + const routeCases = [ + { route: '/__dd/executeActionViaCloud', mode: 'development' }, + { route: '/__dd/executeAction', mode: DEV_VERIFY_MODE }, + ]; + test.each(routeCases)( + 'Should answer $route in $mode mode with a 413 that explains the size', + async ({ route, mode }) => { + mockBuildWithParsedBackend(BUNDLED_CODE); + const rejection = inputRejection(SCRIPT_REJECTION); + const scope = rejectSubmit(rejection); + const middleware = createTestMiddleware({ mode }); + + const { statusCode, body } = await callViaCloud( + middleware, + mockFunctions[0], + route, + ); + const isDone = scope.isDone(); + + const displayName = formatRef(mockFunctions[0]); + const expected = oversizedBundleMessage( + displayName, + BUNDLE_LENGTH, + SCRIPT_REJECTION, + ); + expect(isDone).toBe(true); + expect(statusCode).toBe(413); + const warnings = getWarnings(); + expect(body).toEqual({ success: false, error: expected }); + expect(warnings).toEqual([expected]); + }, + ); + + test('Should warn once per function, however often it is rejected', async () => { + mockBuildWithParsedBackend(BUNDLED_CODE); + const rejection = inputRejection(SCRIPT_REJECTION); + const scope = rejectSubmit(rejection, 3); + const middleware = createTestMiddleware(); + + const first = await callViaCloud(middleware, mockFunctions[0]); + const second = await callViaCloud(middleware, mockFunctions[0]); + const other = await callViaCloud(middleware, mockFunctions[1]); + const isDone = scope.isDone(); + const warnings = getWarnings(); + + const [firstName, otherName] = mockFunctions.map(formatRef); + const firstWarning = oversizedBundleMessage(firstName, BUNDLE_LENGTH, SCRIPT_REJECTION); + const otherWarning = oversizedBundleMessage(otherName, BUNDLE_LENGTH, SCRIPT_REJECTION); + expect(isDone).toBe(true); + expect([first.statusCode, second.statusCode, other.statusCode]).toEqual([ + 413, 413, 413, + ]); + expect(warnings).toEqual([firstWarning, otherWarning]); + }); + + const passthroughCases = [ + { + scenario: 'another input field is rejected', + body: inputRejection('.allowedConnectionIds: expected array'), + }, + { + scenario: 'a nested field named script is rejected', + body: inputRejection('.context.script: expected string'), + }, + { + scenario: 'the error code is not an input-parameter rejection', + body: { + errors: [{ ...inputRejection(SCRIPT_REJECTION).errors[0], code: 'OTHER' }], + }, + }, + ]; + test.each(passthroughCases)( + 'Should pass the error on unchanged when $scenario', + async ({ body: rejection }) => { + mockBuildWithParsedBackend(BUNDLED_CODE); + const scope = rejectSubmit(rejection); + const middleware = createTestMiddleware(); + + const { statusCode, body } = await callViaCloud(middleware); + const isDone = scope.isDone(); + const warnings = getWarnings(); + + expect(isDone).toBe(true); + expect(statusCode).toBe(500); + expect(body.error).toContain('HTTP 400'); + expect(body.error).not.toContain('larger than production'); + expect(warnings).toEqual([]); + }, + ); + }); + describe('executeAction handler (local)', () => { beforeEach(() => { mockRuntimeContextHydration(); @@ -1274,6 +1469,7 @@ describe('Dev Server Middleware', () => { mockLog, 'development', localOnlyHostCheck, + undefined, ); const req = createMockRequest('/__dd/executeAction', { @@ -1307,6 +1503,7 @@ describe('Dev Server Middleware', () => { mockLog, 'development', localOnlyHostCheck, + undefined, ); mockLoadModuleReturning(mockFunctions[0], () => 'recovered'); const recoveringReq = createMockRequest('/__dd/executeAction', { @@ -1608,6 +1805,7 @@ describe('Dev Server Middleware', () => { mockLog, 'development', localOnlyHostCheck, + undefined, ); const req = createMockRequest('/__dd/executeAction', { diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index c809ffa6b..e5d63156e 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -21,7 +21,7 @@ import type { LongPollingOptions } from '../types'; import { createBackendConnectionIdCollector } from './backend-connection-id-collector'; import { createBackendStaticChecksPlugin } from './backend-static-checks-plugin'; -import { getBaseBackendBuildConfig } from './build-config'; +import { getBaseBackendBuildConfig, withProductionSettings } from './build-config'; import type { HostCheck } from './dev-server-request-guard'; import { checkDevServerRequest } from './dev-server-request-guard'; import type { ExecuteAction, GetRuntimeContext, LoadModule } from './local-execution'; @@ -135,6 +135,7 @@ function parseRequestBody(req: IncomingMessage): Promise { */ async function bundleBackendFunction( viteBuild: typeof build, + serverConditions: readonly string[] | undefined, func: BackendFunction, projectRoot: string, log: Logger, @@ -158,10 +159,11 @@ async function bundleBackendFunction( log, connectionIdCollector.getModuleRecords, ); - const baseConfig = getBaseBackendBuildConfig(projectRoot, { [virtualId]: virtualContent }, [ + const devBaseConfig = getBaseBackendBuildConfig(projectRoot, { [virtualId]: virtualContent }, [ connectionIdCollector.plugin, staticChecksPlugin, ]); + const baseConfig = withProductionSettings(devBaseConfig, serverConditions); // Dev: build a single function in-memory per request so we can send the // bundled script to the Datadog API without writing temp files. @@ -255,6 +257,35 @@ async function submitQuery( return pollQueryExecution(receiptId, auth, doAuthenticatedRequest, longPolling, log, signal); } +export function oversizedBundleMessage( + displayName: string, + length: number, + rejection: string, +): string { + return `Backend function "${displayName}" is larger than production accepts: its minified bundle is ${length} characters, and Datadog answered "${rejection}". Shrink the bundle, for example by splitting the function or moving large data out of it.`; +} + +// Datadog rejects a script over the action's maxLength as an invalid `script` input, the only +// constraint on that field a string can break. doRequest's message lists each error's fields. +function asOversizedBundleError( + error: unknown, + displayName: string, + script: string, +): OversizedBundleError | undefined { + if (!(error instanceof Error) || !error.message.startsWith('HTTP 400 ')) { + return undefined; + } + const isInputRejection = error.message.includes('code: INVALID_INPUT_PARAMETER'); + const rejection = /(?:^|\s)(\.script: [^\n]*)/.exec(error.message)?.[1]; + if (!isInputRejection || !rejection) { + return undefined; + } + // Datadog counts code points, as JSON Schema's maxLength does. + const length = Array.from(script).length; + const message = oversizedBundleMessage(displayName, length, rejection); + return new OversizedBundleError(message, displayName); +} + /** 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, @@ -281,7 +312,9 @@ async function executeScriptViaDatadog( doAuthenticatedRequest, longPolling, log, - ); + ).catch((error: unknown) => { + throw asOversizedBundleError(error, displayName, scriptBody) ?? error; + }); if (typeof outputs !== 'object' || outputs === null || !('data' in outputs)) { throw new Error('Query execution completed without a "data" field in its outputs'); @@ -526,6 +559,15 @@ class HttpError extends Error { } } +class OversizedBundleError extends HttpError { + constructor( + message: string, + readonly displayName: string, + ) { + super(413, message); + } +} + /** * Split out from `validateAndBundle` so `handleExecuteAction`'s no-bundling local path can * reuse the same parse-and-lookup step without pulling in a bundle. @@ -649,6 +691,7 @@ async function handleExecuteActionViaCloud( doAuthenticatedRequest: DoAuthenticatedRequest, longPolling: LongPollingConfig, log: Logger, + oversizedFunctionsWarned: Set, ): Promise { try { const { func, code, args } = await validateAndBundle(req, functionsByName, bundle); @@ -668,6 +711,13 @@ async function handleExecuteActionViaCloud( sendSuccess(res, result); } catch (error: unknown) { + if ( + error instanceof OversizedBundleError && + !oversizedFunctionsWarned.has(error.displayName) + ) { + oversizedFunctionsWarned.add(error.displayName); + log.warn(error.message); + } // Labeled by the actual URL hit, not a fixed name, since dev-verify mode also reaches this via /__dd/executeAction. handleHttpError(res, error, log, req.url ?? 'executeActionViaCloud'); } @@ -683,6 +733,7 @@ function routeToCloudHandler( doAuthenticatedRequest: DoAuthenticatedRequest | undefined, longPolling: LongPollingConfig, log: Logger, + oversizedFunctionsWarned: Set, ): void { guardAuthenticated(res, doAuthenticatedRequest, (authedRequest) => handleExecuteActionViaCloud( @@ -694,6 +745,7 @@ function routeToCloudHandler( authedRequest, longPolling, log, + oversizedFunctionsWarned, ), ); } @@ -722,10 +774,12 @@ export function createDevServerMiddleware( log: Logger, mode: string, isHostAllowed: HostCheck, + serverConditions: readonly string[] | undefined, ): (req: IncomingMessage, res: ServerResponse, next: () => void) => void { const bundle = (func: BackendFunction) => - bundleBackendFunction(viteBuild, func, projectRoot, log); + bundleBackendFunction(viteBuild, serverConditions, func, projectRoot, log); const isDevVerifyMode = mode === DEV_VERIFY_MODE; + const oversizedFunctionsWarned = new Set(); const initialFunctions = getBackendFunctions(); if (initialFunctions.length > 0) { @@ -744,6 +798,7 @@ export function createDevServerMiddleware( doAuthenticatedRequest, longPolling, log, + oversizedFunctionsWarned, ); const runLocally: RouteHandler = (req, res, functionsByName) => guardAuthenticated(res, doAuthenticatedRequest, (authedRequest) => diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 68d3cc7b0..ff1ecbf12 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -43,6 +43,7 @@ import { } from '../constants'; import * as buildPackage from './build-package'; +import * as devServer from './dev-server'; import { makeProbeDirOutsideTmp } from './network-guard.fixtures'; import * as networkGuard from './network-guard'; @@ -1348,6 +1349,29 @@ describe('Backend Functions - getVitePlugin', () => { } }); + const serverConditionCases = [ + { vite: 'Vite 6 and later', defaultServerConditions: ['module', 'node'] }, + { vite: 'Vite 5', defaultServerConditions: undefined }, + ]; + test.each(serverConditionCases)( + 'Should hand the middleware the default server conditions of $vite', + ({ defaultServerConditions }) => { + const middlewareSpy = jest.spyOn(devServer, 'createDevServerMiddleware'); + const bundler = { ...mockVite, defaultServerConditions }; + const plugin = getVitePlugin({ ...defaultOptions, bundler }); + const configureServer = getConfigureServer(plugin); + const use = jest.fn(); + const ssrLoadModule = jest.fn(); + const config = fakeServerConfig(DEV_VERIFY_MODE); + + configureServer({ middlewares: { use }, ssrLoadModule, config }); + + const [middlewareArgs] = middlewareSpy.mock.calls; + const passedConditions = middlewareArgs.at(-1); + expect(passedConditions).toBe(defaultServerConditions); + }, + ); + // Uses the real configureServer hook, not createDevServerMiddleware directly, to catch mode-forwarding regressions. test('Should route /__dd/executeAction to the cloud path when configureServer sees a dev-verify server.config.mode', async () => { const plugin = getVitePlugin(defaultOptions); diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index fdc2ad6cd..53a745fac 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -56,6 +56,8 @@ import { loadViteParseAst } from './vite-parse-ast'; export type ViteBundler = { build: typeof build; + // Vite 6 and later. + defaultServerConditions?: readonly string[]; }; export interface VitePluginOptions { @@ -534,6 +536,7 @@ export const getVitePlugin = ({ log, server.config.mode, isHostAllowed, + bundler.defaultServerConditions, ); server.middlewares.use(middleware); },