fix(tracemetrics): Validate aggregate sort bys for metrics #103555
Conversation
github-actions
bot
added
the
Scope: Frontend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
|
|
||
| const baseVisualize = value.find(isBaseVisualize); | ||
| return baseVisualize ? Visualize.fromJSON(baseVisualize) : []; |
There was a problem hiding this comment.
Bug: Support for Multiple Visualizations Broken
The parseVisualizes function uses find() to get only the first BaseVisualize object from the array, discarding any additional visualizations. This breaks support for multiple visualizations that worked in the previous implementation which used filter().flatMap() to process all BaseVisualize objects.
| } | ||
|
|
||
| return !!value.name && !!value.type; | ||
| } |
There was a problem hiding this comment.
Bug: Refine isTraceMetric Validation for Null and Types
The isTraceMetric function crashes when value is null because typeof null === 'object' in JavaScript, causing line 27 to attempt property access on null which throws a TypeError. Additionally, the function incorrectly rejects valid TraceMetric objects with empty strings (like {name: '', type: ''} used in defaultMetricQuery) because !!'' evaluates to false. The checks should verify property types instead of truthiness and explicitly exclude null.
gggritso
left a comment
gggritso
left a comment
There was a problem hiding this comment.
Makes sense overall, but those anys are a big liability, especially for any of them that parse URL data which could be null or whatever. Worth fixing IMO!
Thanks for the fix 🙏🏻
Zylphrex
deleted the
txiao/fix/validate-aggregate-sort-bys-for-metrics
branch
It was possible for the sort by not to be one of the selected fields.