Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 44 additions & 8 deletions packages/core/src/v3/utils/flattenAttributes.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Missing changeset for public package change

This PR modifies packages/core which is the published @trigger.dev/core package. Per CONTRIBUTING.md and CLAUDE.md, changes to packages in packages/* require a changeset (pnpm run changeset:add). The diff only shows changes to packages/core/src/v3/utils/flattenAttributes.ts and packages/core/test/flattenAttributes.test.ts with no changeset file visible. This is a process requirement rather than a code bug.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,40 @@ export const CIRCULAR_REFERENCE_SENTINEL = "$@circular((";

const DEFAULT_MAX_DEPTH = 128;

/** Escape literal dots in a key segment so they are not confused with the path delimiter. */
function escapeKey(key: string): string {
return key.replace(/\\/g, "\\\\").replace(/\./g, "\\.");
}

/** Unescape a key segment that was escaped by `escapeKey`. */
function unescapeKey(key: string): string {
return key.replace(/\\(.)/g, "$1");
}

/**
* Split a flattened attribute path on unescaped dots.
* Escaped dots (`\\.`) are preserved inside key segments and later unescaped.
*/
function splitKey(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) {
// Keep the escape sequence intact for now; unescapeKey will handle it
current += ch + key[i + 1];
i++;
} else if (ch === ".") {
parts.push(current);
current = "";
} else {
current += ch;
}
}
parts.push(current);
return parts;
}

export function flattenAttributes(
obj: unknown,
prefix?: string,
Expand Down Expand Up @@ -116,7 +150,7 @@ class AttributeFlattener {
for (const [key, value] of obj) {
if (!this.canAddMoreAttributes()) break;
// Use the key directly if it's a string, otherwise convert it
const keyStr = typeof key === "string" ? key : String(key);
const keyStr = typeof key === "string" ? escapeKey(key) : escapeKey(String(key));
this.#processValue(value, `${prefix || "map"}.${keyStr}`, depth);
}
return;
Expand Down Expand Up @@ -200,7 +234,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++) {
Expand Down Expand Up @@ -278,19 +313,20 @@ export function unflattenAttributes(
continue;
}

const parts = key.split(".").reduce(
const parts = splitKey(key).reduce(
(acc, part) => {
if (part.startsWith("[") && part.endsWith("]")) {
// Handle array indices more precisely
const match = part.match(/^\[(\d+)\]$/);
if (match && match[1]) {
acc.push(parseInt(match[1]));
const inner = part.slice(1, -1);
const match = inner.match(/^\d+$/);
if (match) {
acc.push(parseInt(inner));
} else {
// Remove brackets for non-numeric array keys
acc.push(part.slice(1, -1));
acc.push(unescapeKey(inner));
}
} else {
acc.push(part);
acc.push(unescapeKey(part));
Comment on lines +316 to +329
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Backward compatibility preserved for existing stored data

The change from key.split(".") to splitKey(key) in unflattenAttributes is backward-compatible: old flattened data (stored in PostgreSQL/ClickHouse via eventRepository.server.ts) never contains backslashes in keys, so splitKey behaves identically to .split(".") and unescapeKey is a no-op for those keys. However, the forward direction is a breaking format change: new flattenAttributes output (e.g., "Key 0\\.002mm") will be misinterpreted by old unflattenAttributes code that uses .split("."). In this monorepo, flattenAttributes runs both server-side (apps/webapp) and client-side (SDK in user containers via packages/core). If the SDK is upgraded but the server is not (or vice versa), escaped keys produced by the new code will be incorrectly split by the old code. This is acceptable if both are always deployed together, but worth noting for SDK consumers who may upgrade independently.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
return acc;
},
Expand Down
101 changes: 101 additions & 0 deletions packages/core/test/flattenAttributes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,83 @@ describe("flattenAttributes", () => {
// Should complete without stack overflow
expect(() => flattenAttributes({ arr: deepArray })).not.toThrow();
});

it("handles keys containing periods correctly", () => {
// The exact case from issue #1510
const obj = { "Key 0.002mm": 31.4 };
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({ "Key 0\\.002mm": 31.4 });

const unflattened = unflattenAttributes(flattened);
expect(unflattened).toEqual({ "Key 0.002mm": 31.4 });
});

it("handles nested objects with dotted keys", () => {
const obj = {
measurements: {
"tolerance.min": 0.5,
"tolerance.max": 1.5,
},
};
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({
"measurements.tolerance\\.min": 0.5,
"measurements.tolerance\\.max": 1.5,
});

const unflattened = unflattenAttributes(flattened);
expect(unflattened).toEqual(obj);
});

it("handles keys with multiple periods", () => {
const obj = { "a.b.c": "value" };
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({ "a\\.b\\.c": "value" });

const unflattened = unflattenAttributes(flattened);
expect(unflattened).toEqual({ "a.b.c": "value" });
});

it("handles dotted keys mixed with normal nesting", () => {
const obj = {
parent: {
"key.with.dots": "dotted",
normalKey: "normal",
},
};
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({
"parent.key\\.with\\.dots": "dotted",
"parent.normalKey": "normal",
});

const unflattened = unflattenAttributes(flattened);
expect(unflattened).toEqual(obj);
});

it("handles keys containing backslashes", () => {
const obj = { "back\\slash": "value" };
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({ "back\\\\slash": "value" });

const unflattened = unflattenAttributes(flattened);
expect(unflattened).toEqual({ "back\\slash": "value" });
});

it("round-trips dotted keys with arrays", () => {
const obj = {
"config.v2": [10, 20, 30],
};
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({
"config\\.v2.[0]": 10,
"config\\.v2.[1]": 20,
"config\\.v2.[2]": 30,
});

const unflattened = unflattenAttributes(flattened);
expect(unflattened).toEqual({ "config.v2": [10, 20, 30] });
});
});

describe("unflattenAttributes", () => {
Expand Down Expand Up @@ -667,4 +744,28 @@ describe("unflattenAttributes", () => {
}
expect(current).toBeUndefined();
});

it("unflattens keys with escaped dots correctly", () => {
const flattened = {
"parent.dotted\\.key": "value",
};
const result = unflattenAttributes(flattened);
expect(result).toEqual({
parent: {
"dotted.key": "value",
},
});
});

it("unflattens keys with escaped backslashes correctly", () => {
const flattened = {
"parent.back\\\\slash": "value",
};
const result = unflattenAttributes(flattened);
expect(result).toEqual({
parent: {
"back\\slash": "value",
},
});
});
});