bug(replays): Fix error where some recordings can not be parsed due to type errors #1765
Conversation
relay-replays/src/recording.rs
Outdated
| @@ -313,7 +313,7 @@ struct MetaEvent { | |||
| struct CustomEvent { | |||
| #[serde(rename = "type")] | |||
| ty: u8, | |||
| timestamp: f64, | |||
| timestamp: Value, | |||
There was a problem hiding this comment.
Would it be better to parse this into proper Timestamp? Like in the event protocol:
relay/relay-general/src/protocol/event.rs
Line 331 in 7834adb
There was a problem hiding this comment.
Or it can be just DateTime that serde supports
| }, | ||
| { | ||
| "type": 5, | ||
| "timestamp": 1658770772, |
There was a problem hiding this comment.
Did this actually fail to parse as f64?
There was a problem hiding this comment.
@jjbayer You're right. I had test coverage that was failing but 1658770772.0 (instead of 1658770772) should be reasonably parse-able by the UI. I think its okay to leave this as f64.
There was a problem hiding this comment.
assert-json-diff seems to have a more lenient compare mode, with which you might be able to keep the test cases:
https://docs.rs/assert-json-diff/latest/assert_json_diff/enum.NumericMode.html#variant.AssumeFloat
closes: https://github.com/getsentry/replay-backend/issues/258
Sentry event timestamps can be float or integer type. Additionally id fields can be negative so we're using i32 now.