timestamp property in the protocol should be DateTime<Utc>, not SystemTime

[vaind]

Environment

• sentry-cli, current master, which depends on sentry-types v0.29.3
• Windows

Steps to Reproduce

I ran into this after enabling this Sentry-CLI test case on my Windows PC.
That panics with overflow when adding duration to instant on SystemTime::from() because the given timestamp in the test case overflows what can be represented by SystemTime on Windows.

You can also reproduce this simply by running the following code on Windows.

    let timestamp = Utc.timestamp_opt(1649335000929, 0).unwrap();
    println!("{}", timestamp.to_rfc3339());
    let systemtime = SystemTime::from(timestamp);

Change request

As the event timestamp is currently represented by SystemTime, which depends on the operating system and architecture (the same would also fail on any 32-bit platform), the sentry-types crate cannot be used generally to access (deserialize) events produced by other platforms, if, for whatever reason, they include timestamps out of the range representable by SystemTime.

Instead, I'd suggest evaluating the whole repository for all uses of SystemTime as a struct field or an argument, etc. and consider changing them to DateTime<Utc> which, in the given example of event.timestamp, is the actual type of the value.

[Swatinem]

Just as some background: The chrono crate used to be unmaintained for quite some time, and had RUSTSEC advisories against it, that’s why I migrated away to SystemTime.
I’m a bit surprised that SystemTime::from is even a thing? Which impl does that use? What is the type of timestamp in your example above?
Usually you use SystemTime::UNIX_EPOCH + Duration::from_millis() for this. Or even checked_add to be extra safe.

Looks like your timestamp is in milliseconds, but timestamp_opt expects seconds. So your timestamp is in the year 54235 according to JS, which I believe does not suffer from limited precision here :-D

Things definitely should not panic though.

[vaind]

SystemTime::UNIX_EPOCH + Duration::from_millis()

That's what the SystemTime::from() does and that's what panics (on Windows):

SystemTime::UNIX_EPOCH + Duration::new(1649335000929, 0);

because it overflows

[Swatinem]

Does #554 cover this?

[kamilogorek]

I believe it does. If not, and I'm wrong, please ping me @vaind