diff --git a/.changeset/fix-dotted-keys-logging.md b/.changeset/fix-dotted-keys-logging.md new file mode 100644 index 0000000000..dafa8061ab --- /dev/null +++ b/.changeset/fix-dotted-keys-logging.md @@ -0,0 +1,5 @@ +--- +"@trigger.dev/core": patch +--- + +Fix issue where logging objects with keys containing dots resulted in incorrect nested object structure in logs (#1510) diff --git a/apps/webapp/app/components/runs/v3/RunFilters.tsx b/apps/webapp/app/components/runs/v3/RunFilters.tsx index cff56573ad..af2ebd80c3 100644 --- a/apps/webapp/app/components/runs/v3/RunFilters.tsx +++ b/apps/webapp/app/components/runs/v3/RunFilters.tsx @@ -635,7 +635,7 @@ function TasksDropdown({ {trigger} { if (onClose) { onClose(); @@ -655,7 +655,7 @@ function TasksDropdown({ } > - + ))} @@ -896,10 +896,10 @@ function TagsDropdown({ {filtered.length > 0 ? filtered.map((tag, index) => ( - - {tag} - - )) + + {tag} + + )) : null} {filtered.length === 0 && fetcher.state !== "loading" && ( No tags found @@ -981,8 +981,7 @@ function QueuesDropdown({ searchParams.set("query", s); } fetcher.load( - `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${ - environment.slug + `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug }/queues?${searchParams.toString()}` ); }, @@ -1054,20 +1053,20 @@ function QueuesDropdown({ {filtered.length > 0 ? filtered.map((queue) => ( - - ) : ( - - ) - } - > - {queue.name} - - )) + + ) : ( + + ) + } + > + {queue.name} + + )) : null} {filtered.length === 0 && fetcher.state !== "loading" && ( No queues found @@ -1243,8 +1242,7 @@ function VersionsDropdown({ searchParams.set("query", s); } fetcher.load( - `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${ - environment.slug + `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug }/versions?${searchParams.toString()}` ); }, @@ -1305,13 +1303,13 @@ function VersionsDropdown({ {filtered.length > 0 ? filtered.map((version) => ( - - - {version.version} - {version.isCurrent ? Current : null} - - - )) + + + {version.version} + {version.isCurrent ? Current : null} + + + )) : null} {filtered.length === 0 && fetcher.state !== "loading" && ( No versions found diff --git a/apps/webapp/package.json b/apps/webapp/package.json index 661795ba08..d896024b48 100644 --- a/apps/webapp/package.json +++ b/apps/webapp/package.json @@ -49,7 +49,7 @@ "@codemirror/view": "6.7.2", "@conform-to/react": "0.9.2", "@conform-to/zod": "0.9.2", - "@depot/cli": "0.0.1-cli.2.80.0", + "@depot/cli": "0.0.1-cli.2.101.3", "@depot/sdk-node": "^1.0.0", "@electric-sql/react": "^0.3.5", "@headlessui/react": "^1.7.8", @@ -298,4 +298,4 @@ "engines": { "node": ">=18.19.0 || >=20.6.0" } -} +} \ No newline at end of file diff --git a/packages/cli-v3/package.json b/packages/cli-v3/package.json index 838593006f..6b44086cb5 100644 --- a/packages/cli-v3/package.json +++ b/packages/cli-v3/package.json @@ -82,7 +82,7 @@ }, "dependencies": { "@clack/prompts": "0.11.0", - "@depot/cli": "0.0.1-cli.2.80.0", + "@depot/cli": "0.0.1-cli.2.101.3", "@modelcontextprotocol/sdk": "^1.25.2", "@opentelemetry/api": "1.9.0", "@opentelemetry/api-logs": "0.203.0", @@ -159,4 +159,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/cli-v3/src/build/buildWorker.ts b/packages/cli-v3/src/build/buildWorker.ts index b23b802f50..32768792b1 100644 --- a/packages/cli-v3/src/build/buildWorker.ts +++ b/packages/cli-v3/src/build/buildWorker.ts @@ -8,9 +8,10 @@ import { resolvePluginsForContext, } from "./extensions.js"; import { createExternalsBuildExtension } from "./externals.js"; -import { join, relative, sep } from "node:path"; +import { join, relative, sep, basename } from "node:path"; import { generateContainerfile } from "../deploy/buildImage.js"; import { writeFile } from "node:fs/promises"; +import fsModule from "node:fs/promises"; import { buildManifestToJSON } from "../utilities/buildManifest.js"; import { readPackageJSON } from "pkg-types"; import { writeJSONFile } from "../utilities/fileSystem.js"; @@ -53,16 +54,16 @@ export async function buildWorker(options: BuildWorkerOptions) { const buildContext = createBuildContext(options.target, resolvedConfig, { logger: options.plain ? { - debug: (...args) => console.log(...args), - log: (...args) => console.log(...args), - warn: (...args) => console.log(...args), - progress: (message) => console.log(message), - spinner: (message) => { - const $spinner = spinner({ plain: true }); - $spinner.start(message); - return $spinner; - }, - } + debug: (...args) => console.log(...args), + log: (...args) => console.log(...args), + warn: (...args) => console.log(...args), + progress: (message) => console.log(message), + spinner: (message) => { + const $spinner = spinner({ plain: true }); + $spinner.start(message); + return $spinner; + }, + } : undefined, }); buildContext.prependExtension(externalsExtension); @@ -196,6 +197,7 @@ async function writeDeployFiles({ join(outputPath, "package.json"), { ...packageJson, + version: "0.0.0", // Strip version for better docker caching name: packageJson.name ?? "trigger-project", dependencies: { ...dependencies, @@ -208,8 +210,13 @@ async function writeDeployFiles({ true ); + if (resolvedConfig.lockfilePath) { + const lockfileName = basename(resolvedConfig.lockfilePath); + await fsModule.copyFile(resolvedConfig.lockfilePath, join(outputPath, lockfileName)); + } + await writeJSONFile(join(outputPath, "build.json"), buildManifestToJSON(buildManifest)); - await writeContainerfile(outputPath, buildManifest); + await writeContainerfile(outputPath, buildManifest, resolvedConfig.lockfilePath); } async function readProjectPackageJson(packageJsonPath: string) { @@ -218,17 +225,31 @@ async function readProjectPackageJson(packageJsonPath: string) { return packageJson; } -async function writeContainerfile(outputPath: string, buildManifest: BuildManifest) { +async function writeContainerfile( + outputPath: string, + buildManifest: BuildManifest, + lockfilePath?: string +) { if (!buildManifest.runControllerEntryPoint || !buildManifest.indexControllerEntryPoint) { throw new Error("Something went wrong with the build. Aborting deployment. [code 7789]"); } + const packageManager = lockfilePath + ? lockfilePath.endsWith("pnpm-lock.yaml") + ? ("pnpm" as const) + : lockfilePath.endsWith("yarn.lock") + ? ("yarn" as const) + : ("npm" as const) + : undefined; + const containerfile = await generateContainerfile({ runtime: buildManifest.runtime, entrypoint: buildManifest.runControllerEntryPoint, build: buildManifest.build, image: buildManifest.image, indexScript: buildManifest.indexControllerEntryPoint, + packageManager, + lockfile: lockfilePath ? basename(lockfilePath) : undefined, }); const containerfilePath = join(outputPath, "Containerfile"); diff --git a/packages/cli-v3/src/build/bundle.test.ts b/packages/cli-v3/src/build/bundle.test.ts new file mode 100644 index 0000000000..8b8a5350c5 --- /dev/null +++ b/packages/cli-v3/src/build/bundle.test.ts @@ -0,0 +1,132 @@ +import { expect, test, vi, describe, beforeEach, afterEach } from "vitest"; +import { bundleWorker } from "./bundle.js"; +import * as esbuild from "esbuild"; +import { copyFile, mkdir } from "node:fs/promises"; +import { shims } from "./packageModules.js"; +import { join } from "node:path"; + +vi.mock("esbuild", () => ({ + build: vi.fn(), + context: vi.fn(() => ({ + watch: vi.fn(), + dispose: vi.fn(), + })), +})); + +vi.mock("node:fs/promises", () => ({ + copyFile: vi.fn(), + mkdir: vi.fn(), + writeFile: vi.fn(), + readdir: vi.fn(() => []), + readFile: vi.fn(), +})); + +vi.mock("../utilities/fileSystem.js", () => ({ + createFile: vi.fn(), + createFileWithStore: vi.fn(), +})); + +vi.mock("./entryPoints.js", () => ({ + createEntryPointManager: vi.fn(() => ({ + entryPoints: ["src/trigger/task.ts"], + patterns: [], + stop: vi.fn(), + })), +})); + +vi.mock("./plugins.js", () => ({ + buildPlugins: vi.fn(() => []), + SdkVersionExtractor: vi.fn(() => ({ + plugin: { name: "sdk-version" }, + })), +})); + +vi.mock("./manifests.js", () => ({ + copyManifestToDir: vi.fn(), +})); + +vi.mock("../utilities/sourceFiles.js", () => ({ + resolveFileSources: vi.fn(), +})); + +vi.mock("../utilities/logger.js", () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + }, +})); + +vi.mock("../utilities/cliOutput.js", () => ({ + cliLink: vi.fn(), + prettyError: vi.fn(), +})); + +vi.mock("../cli/common.js", () => ({ + SkipLoggingError: class extends Error { }, +})); + +describe("bundleWorker with Yarn PnP support", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + test("should copy shims to .trigger/shims and use them in inject", async () => { + const workingDir = "/project"; + const options = { + target: "deploy" as const, + destination: "/dist", + cwd: workingDir, + resolvedConfig: { + workingDir, + dirs: ["src/trigger"], + build: { + jsx: { automatic: true } + } + } as any, + }; + + vi.mocked(esbuild.build).mockResolvedValue({ + outputFiles: [], + metafile: { + outputs: { + "dist/index.mjs": { + entryPoint: "src/entryPoints/managed-run-worker.js", + }, + "dist/controller.mjs": { + entryPoint: "src/entryPoints/managed-run-controller.js", + }, + "dist/index-worker.mjs": { + entryPoint: "src/entryPoints/managed-index-worker.js", + }, + "dist/index-controller.mjs": { + entryPoint: "src/entryPoints/managed-index-controller.js", + }, + "dist/config.mjs": { + entryPoint: "trigger.config.ts", + } + } + }, + errors: [], + warnings: [], + } as any); + + await bundleWorker(options); + + // Verify mkdir was called for .trigger/shims + expect(mkdir).toHaveBeenCalledWith(join(workingDir, ".trigger", "shims"), { recursive: true }); + + // Verify copyFile was called for each shim + for (const shim of shims) { + expect(copyFile).toHaveBeenCalledWith(shim, expect.stringContaining(join(".trigger", "shims"))); + } + + // Verify esbuild.build was called with local shim paths in inject + expect(esbuild.build).toHaveBeenCalledWith( + expect.objectContaining({ + inject: expect.arrayContaining([expect.stringContaining(join(".trigger", "shims"))]), + }) + ); + }); +}); diff --git a/packages/cli-v3/src/build/bundle.ts b/packages/cli-v3/src/build/bundle.ts index 4d1cfd53f8..63a4e66a1d 100644 --- a/packages/cli-v3/src/build/bundle.ts +++ b/packages/cli-v3/src/build/bundle.ts @@ -3,6 +3,7 @@ import { DEFAULT_RUNTIME, ResolvedConfig } from "@trigger.dev/core/v3/build"; import { BuildManifest, BuildTarget, TaskFile } from "@trigger.dev/core/v3/schemas"; import * as esbuild from "esbuild"; import { createHash } from "node:crypto"; +import { copyFile, mkdir } from "node:fs/promises"; import { basename, join, relative, resolve } from "node:path"; import { createFile, createFileWithStore } from "../utilities/fileSystem.js"; import { logger } from "../utilities/logger.js"; @@ -128,10 +129,13 @@ export async function bundleWorker(options: BundleOptions): Promise; @@ -176,7 +180,11 @@ export async function bundleWorker(options: BundleOptions): Promise { const customConditions = options.resolvedConfig.build?.conditions ?? []; const conditions = [...customConditions, "trigger.dev", "module", "node"]; @@ -216,7 +224,8 @@ async function createBuildOptions( ".wasm": "copy", }, outExtension: { ".js": ".mjs" }, - inject: [...shims], // TODO: copy this into the working dir to work with Yarn PnP + outExtension: { ".js": ".mjs" }, + inject: options.shims ?? [...shims], jsx: options.jsxAutomatic ? "automatic" : undefined, jsxDev: options.jsxAutomatic && options.target === "dev" ? true : undefined, plugins: [ @@ -425,3 +434,19 @@ export async function createBuildManifestFromBundle({ return copyManifestToDir(buildManifest, destination, workerDir, storeDir); } + +async function prepareShims(workingDir: string): Promise { + const shimsDir = join(workingDir, ".trigger", "shims"); + await mkdir(shimsDir, { recursive: true }); + + const localShims: string[] = []; + + for (const shimPath of shims) { + const shimFilename = basename(shimPath); + const destination = join(shimsDir, shimFilename); + await copyFile(shimPath, destination); + localShims.push(destination); + } + + return localShims; +} diff --git a/packages/cli-v3/src/deploy/buildImage.test.ts b/packages/cli-v3/src/deploy/buildImage.test.ts new file mode 100644 index 0000000000..a674e0e0d9 --- /dev/null +++ b/packages/cli-v3/src/deploy/buildImage.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect } from "vitest"; +import { generateContainerfile, GenerateContainerfileOptions } from "./buildImage.js"; + +describe("generateContainerfile", () => { + const baseOptions: GenerateContainerfileOptions = { + runtime: "node-22", + build: { + env: {}, + commands: [], + }, + image: { + pkgs: [], + instructions: [], + }, + indexScript: "index.js", + entrypoint: "entrypoint.js", + }; + + it("should include ARG SOURCE_DATE_EPOCH", async () => { + const result = await generateContainerfile(baseOptions); + expect(result).toContain("ARG SOURCE_DATE_EPOCH"); + }); + + it("should generate npm ci command when package-lock.json is present", async () => { + const result = await generateContainerfile({ + ...baseOptions, + lockfile: "package-lock.json", + }); + expect(result).toContain("COPY --chown=node:node package.json package-lock.json ./"); + expect(result).toContain("RUN npm ci --no-audit --no-fund"); + }); + + it("should generate bun install --frozen-lockfile command when bun.lockb is present", async () => { + const result = await generateContainerfile({ + ...baseOptions, + runtime: "bun", + lockfile: "bun.lockb", + }); + expect(result).toContain("COPY --chown=bun:bun package.json bun.lockb ./"); + expect(result).toContain("RUN bun install --frozen-lockfile --production"); + }); + + it("should generate pnpm install command and copy pnpm-lock.yaml", async () => { + const result = await generateContainerfile({ + ...baseOptions, + packageManager: "pnpm", + lockfile: "pnpm-lock.yaml", + }); + expect(result).toContain("COPY --chown=node:node package.json pnpm-lock.yaml ./"); + expect(result).toContain("RUN npx pnpm i --prod --no-frozen-lockfile"); + }); + + it("should generate npm install command by default", async () => { + const result = await generateContainerfile(baseOptions); + expect(result).toContain("RUN npm i --no-audit --no-fund --no-save --no-package-lock"); + }); + + it("should generate npm install command when npm is specified", async () => { + const result = await generateContainerfile({ + ...baseOptions, + packageManager: "npm", + }); + expect(result).toContain("RUN npm i --no-audit --no-fund --no-save --no-package-lock"); + }); + + it("should generate pnpm install command when pnpm is specified", async () => { + const result = await generateContainerfile({ + ...baseOptions, + packageManager: "pnpm", + }); + expect(result).toContain("RUN npx pnpm i --prod --no-frozen-lockfile"); + }); + + it("should generate yarn install command when yarn is specified", async () => { + const result = await generateContainerfile({ + ...baseOptions, + packageManager: "yarn", + }); + expect(result).toContain("RUN yarn install --production --no-lockfile"); + }); +}); diff --git a/packages/cli-v3/src/deploy/buildImage.ts b/packages/cli-v3/src/deploy/buildImage.ts index 2225d7db05..75175a34a7 100644 --- a/packages/cli-v3/src/deploy/buildImage.ts +++ b/packages/cli-v3/src/deploy/buildImage.ts @@ -51,9 +51,7 @@ export interface BuildImageOptions { buildEnvVars?: Record; onLog?: (log: string) => void; - // Optional deployment spinner - deploymentSpinner?: any; // Replace 'any' with the actual type if known -} + export async function buildImage(options: BuildImageOptions): Promise { const { @@ -683,6 +681,8 @@ export type GenerateContainerfileOptions = { image: BuildManifest["image"]; indexScript: string; entrypoint: string; + packageManager?: "npm" | "pnpm" | "yarn"; + lockfile?: string; }; const BASE_IMAGE: Record = { @@ -738,6 +738,7 @@ async function generateBunContainerfile(options: GenerateContainerfileOptions) { return `# syntax=docker/dockerfile:1 # check=skip=SecretsUsedInArgOrEnv +ARG SOURCE_DATE_EPOCH FROM ${baseImage} AS base ${baseInstructions} @@ -763,8 +764,11 @@ ${buildArgs} ${buildEnvVars} -COPY --chown=bun:bun package.json ./ -RUN bun install --production --no-save +COPY --chown=bun:bun package.json ${options.lockfile ? `${options.lockfile} ` : ""}./ +RUN ${options.lockfile && options.lockfile.endsWith(".lockb") + ? "bun install --frozen-lockfile --production" + : "bun install --production --no-save" + } # Now copy all the files # IMPORTANT: Do this after running npm install because npm i will wipe out the node_modules directory @@ -840,6 +844,7 @@ async function generateNodeContainerfile(options: GenerateContainerfileOptions) return `# syntax=docker/dockerfile:1 # check=skip=SecretsUsedInArgOrEnv +ARG SOURCE_DATE_EPOCH FROM ${baseImage} AS base ${baseInstructions} @@ -868,8 +873,15 @@ ${buildEnvVars} ENV NODE_ENV=production ENV NPM_CONFIG_UPDATE_NOTIFIER=false -COPY --chown=node:node package.json ./ -RUN npm i --no-audit --no-fund --no-save --no-package-lock +COPY --chown=node:node package.json ${options.lockfile ? `${options.lockfile} ` : ""}./ +RUN ${options.packageManager === "pnpm" + ? "npx pnpm i --prod --no-frozen-lockfile" + : options.packageManager === "yarn" + ? "yarn install --production --no-lockfile" + : options.lockfile && options.lockfile.endsWith("package-lock.json") + ? "npm ci --no-audit --no-fund" + : "npm i --no-audit --no-fund --no-save --no-package-lock" + } # Now copy all the files # IMPORTANT: Do this after running npm install because npm i will wipe out the node_modules directory diff --git a/packages/cli-v3/src/index.ts b/packages/cli-v3/src/index.ts index 01f8666d87..cafb420511 100644 --- a/packages/cli-v3/src/index.ts +++ b/packages/cli-v3/src/index.ts @@ -2,8 +2,13 @@ import { program } from "./cli/index.js"; import { logger } from "./utilities/logger.js"; +import { ensureSufficientMemory } from "./utilities/memory.js"; const main = async () => { + if (ensureSufficientMemory()) { + return; + } + await program.parseAsync(); }; diff --git a/packages/cli-v3/src/utilities/memory.test.ts b/packages/cli-v3/src/utilities/memory.test.ts new file mode 100644 index 0000000000..2108c945bf --- /dev/null +++ b/packages/cli-v3/src/utilities/memory.test.ts @@ -0,0 +1,89 @@ +import { expect, test, vi, describe, beforeEach, afterEach } from "vitest"; +import { ensureSufficientMemory } from "./memory.js"; +import { spawn } from "node:child_process"; +import { getHeapStatistics } from "node:v8"; + +vi.mock("node:child_process", () => ({ + spawn: vi.fn(() => ({ + on: vi.fn(), + })), +})); + +vi.mock("node:v8", () => ({ + getHeapStatistics: vi.fn(), +})); + +vi.mock("./logger.js", () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + error: vi.fn(), + }, +})); + +describe("ensureSufficientMemory", () => { + const originalEnv = process.env; + const originalArgv = process.argv; + + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...originalEnv }; + process.argv = ["node", "trigger", "deploy"]; + }); + + afterEach(() => { + process.env = originalEnv; + process.argv = originalArgv; + }); + + test("should not respawn if memory limit is already high enough", () => { + vi.mocked(getHeapStatistics).mockReturnValue({ + heap_size_limit: 5000 * 1024 * 1024, // 5GB + } as any); + + const result = ensureSufficientMemory(); + expect(result).toBe(false); + expect(spawn).not.toHaveBeenCalled(); + }); + + test("should not respawn if TRIGGER_CLI_MEMORY_RESPAWNED is set", () => { + process.env.TRIGGER_CLI_MEMORY_RESPAWNED = "1"; + vi.mocked(getHeapStatistics).mockReturnValue({ + heap_size_limit: 1000 * 1024 * 1024, // 1GB + } as any); + + const result = ensureSufficientMemory(); + expect(result).toBe(false); + expect(spawn).not.toHaveBeenCalled(); + }); + + test("should not respawn if command is not memory intensive", () => { + process.argv = ["node", "trigger", "whoami"]; + vi.mocked(getHeapStatistics).mockReturnValue({ + heap_size_limit: 1000 * 1024 * 1024, // 1GB + } as any); + + const result = ensureSufficientMemory(); + expect(result).toBe(false); + expect(spawn).not.toHaveBeenCalled(); + }); + + test("should respawn if memory limit is low and command is deploy", () => { + process.argv = ["node", "trigger", "deploy"]; + vi.mocked(getHeapStatistics).mockReturnValue({ + heap_size_limit: 2000 * 1024 * 1024, // 2GB + } as any); + + const result = ensureSufficientMemory(); + expect(result).toBe(true); + expect(spawn).toHaveBeenCalledWith( + process.execPath, + expect.arrayContaining(["--max-old-space-size=4096"]), + expect.objectContaining({ + env: expect.objectContaining({ + TRIGGER_CLI_MEMORY_RESPAWNED: "1", + }), + }) + ); + }); +}); diff --git a/packages/cli-v3/src/utilities/memory.ts b/packages/cli-v3/src/utilities/memory.ts new file mode 100644 index 0000000000..e3b257d71b --- /dev/null +++ b/packages/cli-v3/src/utilities/memory.ts @@ -0,0 +1,58 @@ +import { spawn } from "node:child_process"; +import { getHeapStatistics } from "node:v8"; +import { logger } from "./logger.js"; + +const DEFAULT_MEMORY_LIMIT_MB = 4096; + +/** + * Ensures that the current Node process has enough memory to perform builds. + * If not, it respawns the process with a larger heap size. + * @returns true if the process is being respawned, false otherwise. + */ +export function ensureSufficientMemory(): boolean { + if (process.env.TRIGGER_CLI_MEMORY_RESPAWNED === "1") { + logger.debug("Already respawned with more memory, skipping check."); + return false; + } + + const heapStats = getHeapStatistics(); + const heapLimitMB = heapStats.heap_size_limit / 1024 / 1024; + + // If the limit is already 4GB or more, we're good + if (heapLimitMB >= DEFAULT_MEMORY_LIMIT_MB) { + logger.debug(`Current heap limit (${Math.round(heapLimitMB)}MB) is sufficient.`); + return false; + } + + // We only want to respawn for memory-intensive commands like deploy or dev + const isMemoryIntensive = + process.argv.includes("deploy") || process.argv.includes("dev") || process.argv.includes("build"); + + if (!isMemoryIntensive) { + return false; + } + + logger.debug( + `Increasing memory limit from ${Math.round(heapLimitMB)}MB to ${DEFAULT_MEMORY_LIMIT_MB}MB...` + ); + + const args = ["--max-old-space-size=4096", ...process.argv.slice(1)]; + + const child = spawn(process.execPath, args, { + stdio: "inherit", + env: { + ...process.env, + TRIGGER_CLI_MEMORY_RESPAWNED: "1", + }, + }); + + child.on("exit", (code, signal) => { + if (signal) { + process.kill(process.pid, signal); + } else { + process.exit(code ?? 0); + } + }); + + return true; +} diff --git a/packages/core/src/v3/utils/flattenAttributes.ts b/packages/core/src/v3/utils/flattenAttributes.ts index 83d1a14f2c..4669eef4b2 100644 --- a/packages/core/src/v3/utils/flattenAttributes.ts +++ b/packages/core/src/v3/utils/flattenAttributes.ts @@ -24,7 +24,7 @@ class AttributeFlattener { constructor( private maxAttributeCount?: number, private maxDepth: number = DEFAULT_MAX_DEPTH - ) {} + ) { } get attributes(): Attributes { return this.result; @@ -200,7 +200,8 @@ class AttributeFlattener { break; } - const newPrefix = `${prefix ? `${prefix}.` : ""}${Array.isArray(obj) ? `[${key}]` : key}`; + const escapedKey = Array.isArray(obj) ? `[${key}]` : escapeKey(key); + const newPrefix = `${prefix ? `${prefix}.` : ""}${escapedKey}`; if (Array.isArray(value)) { for (let i = 0; i < value.length; i++) { @@ -249,6 +250,43 @@ function isRecord(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); } +/** + * Escapes dots and backslashes in a key so they are not confused with + * the `.` nesting separator used by flattenAttributes. + */ +function escapeKey(key: string): string { + if (!key.includes(".") && !key.includes("\\")) { + return key; + } + return key.replace(/\\/g, "\\\\").replace(/\./g, "\\."); +} + +/** + * Splits a flattened attribute key on unescaped dots, then unescapes + * each resulting part. Handles both the new escaped format (`\.`) + * and legacy unescaped keys transparently. + */ +function splitEscapedKey(key: string): string[] { + const parts: string[] = []; + let current = ""; + + for (let i = 0; i < key.length; i++) { + const ch = key[i]; + if (ch === "\\" && i + 1 < key.length) { + // Consume the escaped character literally + current += key[i + 1]; + i++; + } else if (ch === ".") { + parts.push(current); + current = ""; + } else { + current += ch; + } + } + parts.push(current); + return parts; +} + export function unflattenAttributes( obj: Attributes, filteredKeys?: string[], @@ -278,7 +316,7 @@ export function unflattenAttributes( continue; } - const parts = key.split(".").reduce( + const parts = splitEscapedKey(key).reduce( (acc, part) => { if (part.startsWith("[") && part.endsWith("]")) { // Handle array indices more precisely diff --git a/packages/core/test/flattenAttributes.test.ts b/packages/core/test/flattenAttributes.test.ts index 28f137deaf..b1a1d4f0b4 100644 --- a/packages/core/test/flattenAttributes.test.ts +++ b/packages/core/test/flattenAttributes.test.ts @@ -1,3 +1,4 @@ +import { describe, it, expect } from "vitest"; import { flattenAttributes, unflattenAttributes } from "../src/v3/utils/flattenAttributes.js"; describe("flattenAttributes", () => { @@ -297,9 +298,9 @@ describe("flattenAttributes", () => { }); it("handles function values correctly", () => { - function namedFunction() {} - const anonymousFunction = function () {}; - const arrowFunction = () => {}; + function namedFunction() { } + const anonymousFunction = function () { }; + const arrowFunction = () => { }; const result = flattenAttributes({ named: namedFunction, @@ -317,7 +318,7 @@ describe("flattenAttributes", () => { it("handles mixed problematic types", () => { const complexObj = { error: new Error("Mixed error"), - func: function testFunc() {}, + func: function testFunc() { }, date: new Date("2023-01-01"), normal: "string", number: 42, @@ -415,10 +416,10 @@ describe("flattenAttributes", () => { it("handles Promise objects correctly", () => { const resolvedPromise = Promise.resolve("value"); const rejectedPromise = Promise.reject(new Error("failed")); - const pendingPromise = new Promise(() => {}); // Never resolves + const pendingPromise = new Promise(() => { }); // Never resolves // Catch the rejection to avoid unhandled promise rejection warnings - rejectedPromise.catch(() => {}); + rejectedPromise.catch(() => { }); const result = flattenAttributes({ resolved: resolvedPromise, @@ -481,7 +482,7 @@ describe("flattenAttributes", () => { it("handles complex mixed object with all special types", () => { const complexObj = { error: new Error("Test error"), - func: function testFunc() {}, + func: function testFunc() { }, date: new Date("2023-01-01"), mySet: new Set([1, 2, 3]), myMap: new Map([["key", "value"]]), @@ -547,6 +548,52 @@ describe("flattenAttributes", () => { // Should complete without stack overflow expect(() => flattenAttributes({ arr: deepArray })).not.toThrow(); }); + + it("handles keys with periods correctly (issue #1510)", () => { + const input = { "Key 0.002mm": 31.4 }; + const flattened = flattenAttributes(input); + // The dot in the key should be escaped + expect(flattened["Key 0\\.002mm"]).toBe(31.4); + // Roundtrip should preserve the original key + expect(unflattenAttributes(flattened)).toEqual(input); + }); + + it("handles nested objects with dotted keys", () => { + const input = { + measurements: { + "diameter.mm": 12.5, + "length.cm": 30, + }, + }; + const flattened = flattenAttributes(input); + expect(flattened["measurements.diameter\\.mm"]).toBe(12.5); + expect(flattened["measurements.length\\.cm"]).toBe(30); + expect(unflattenAttributes(flattened)).toEqual(input); + }); + + it("handles keys with backslashes correctly", () => { + const input = { "path\\to\\file": "value" }; + const flattened = flattenAttributes(input); + expect(flattened["path\\\\to\\\\file"]).toBe("value"); + expect(unflattenAttributes(flattened)).toEqual(input); + }); + + it("handles keys with both dots and backslashes", () => { + const input = { "file\\.ext": "value" }; + const flattened = flattenAttributes(input); + expect(unflattenAttributes(flattened)).toEqual(input); + }); + + it("handles multiple dotted keys at different nesting levels", () => { + const input = { + "v1.0": { + "config.json": "data", + }, + normal: "value", + }; + const flattened = flattenAttributes(input); + expect(unflattenAttributes(flattened)).toEqual(input); + }); }); describe("unflattenAttributes", () => {