fix(core): escape periods in attribute keys during flatten/unflatten (#1510)#3857
Conversation
Object keys containing periods (e.g. 'Key 0.002mm') were incorrectly split on the '.' delimiter during flatten/unflatten, causing data corruption in the dashboard logs view. Escape dots with \$@dot sentinel during flattening and unescape during unflattening. Fixes triggerdotdev#1510
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds dot-character escaping to the ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @deepshekhardas, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 Map keys with dots are NOT escaped — inconsistency with object key escaping
The PR escapes dots in regular object keys at line 204, but Map keys at packages/core/src/v3/utils/flattenAttributes.ts:120-121 are used directly without dot escaping: this.#processValue(value, \${prefix || "map"}.${keyStr}`, depth)`. This means Map keys containing dots will still be incorrectly split during unflattening, just as they were before this PR. While Maps can't be round-tripped anyway (they become plain objects during unflatten), this is an incomplete fix that could confuse developers who expect consistent behavior.
(Refers to lines 120-121)
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
|
|
||
| const newPrefix = `${prefix ? `${prefix}.` : ""}${Array.isArray(obj) ? `[${key}]` : key}`; | ||
| const escapedKey = Array.isArray(obj) ? `[${key}]` : key.replace(/\./g, DOT_ESCAPE); |
There was a problem hiding this comment.
🟡 Escape sentinel $@dot is not self-escaped, corrupting keys that naturally contain $@dot
The flattenAttributes function escapes dots in keys by replacing . with $@dot (line 204), but does NOT first escape any pre-existing $@dot substrings. During unflattenAttributes, ALL occurrences of $@dot are unconditionally unescaped back to . via part.split(DOT_ESCAPE).join(".") at line 285. This means a key that naturally contains $@dot (e.g., {"x$@doty": 1}) will be corrupted on round-trip: flattenAttributes leaves it as "x$@doty" (no dots to replace), but unflattenAttributes converts it to "x.y", splitting it into a nested structure {x: {y: 1}} instead of preserving the original key.
Reproduction example
flattenAttributes({"x$@DOty": 1}) → {"x$@DOty": 1}
unflattenAttributes({"x$@DOty": 1}) → {x: {y: 1}} // Should be {"x$@DOty": 1}
Prompt for agents
The DOT_ESCAPE sentinel ($@dot) used to escape dots in object keys is not itself escaped, causing a sentinel collision for keys that naturally contain the substring $@dot. To fix this, flattenAttributes (line 204 in flattenAttributes.ts) needs to first escape any pre-existing DOT_ESCAPE occurrences before replacing dots. For example, you could introduce a secondary escape like $@esc that replaces $@ with $@esc in the key first, then replace . with $@dot. In unflattenAttributes (line 285), reverse the process: first replace $@dot with ., then replace $@esc with $@. This ensures that keys containing $@dot are preserved through the round-trip. Alternatively, choose an escape token that is guaranteed not to collide (though this is difficult in general), or use a different encoding scheme entirely (e.g., percent-encoding dots as %2E).
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| export const NULL_SENTINEL = "$@null(("; | ||
| export const CIRCULAR_REFERENCE_SENTINEL = "$@circular(("; | ||
| const DOT_ESCAPE = "$@dot"; |
There was a problem hiding this comment.
🚩 Cross-version SDK/webapp compatibility considerations
This is a @trigger.dev/core change (a published package) that affects both the SDK running in customer containers and the webapp. If the webapp is upgraded first (likely in the hosted service), old SDK versions will continue sending unescaped keys — this is safe since the new unflatten is backward compatible. If a customer upgrades their SDK first while running an older self-hosted webapp, the escaped $@dot in keys would appear as literal text in the old webapp's unflatten output. This is an acceptable trade-off but worth noting for self-hosted users. The PR does not include a changeset, which is required per CONTRIBUTING.md for changes to packages in /packages.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Fixes #1510
Object keys containing periods (e.g.
{ "Key 0.002mm": 31.4 }) were incorrectly split on the.delimiter during flattening, causing dashboard display corruption.Fix: Escape
.in object keys with$@dotsentinel during flatten and unescape during unflatten. This is consistent with the existing$@null((and$@circular((sentinel pattern.Changes:
flattenAttributes.ts: escape dots in non-array keys during flattenunflattenAttributes(): unescape sentinel back to.after splitting