feat(client-reports): Client report protocol#1144
Conversation
|
7872da8 to
131cae7
Compare
66c55a9 to
2fc5269
Compare
131cae7 to
91aa345
Compare
2fc5269 to
6bd618a
Compare
91aa345 to
8ab55da
Compare
6bd618a to
1f9b20f
Compare
8ab55da to
15eeea4
Compare
1f9b20f to
a1c5872
Compare
15eeea4 to
ffbee4e
Compare
41afa32 to
ad0ada5
Compare
ffbee4e to
ac9351c
Compare
c738dd0 to
ece93f5
Compare
9e08f3a to
f8521cc
Compare
ece93f5 to
181f46f
Compare
f8521cc to
193723f
Compare
181f46f to
dbf16e8
Compare
|
|
||
| - Added [`TransportFactory::create_transport_with_options`](https://docs.rs/sentry-core/latest/sentry_core/trait.TransportFactory.html#method.create_transport_with_options), which constructs transports from [`TransportOptions`](https://docs.rs/sentry-core/latest/sentry_core/struct.TransportOptions.html) instead of full [`ClientOptions`](https://docs.rs/sentry-core/latest/sentry_core/struct.ClientOptions.html) ([#1142](https://github.com/getsentry/sentry-rust/pull/1142)). | ||
| - Added transport-specific options types and `with_options` constructors for built-in HTTP transports, including `ReqwestHttpTransportOptions`, `CurlHttpTransportOptions`, `UreqHttpTransportOptions`, and `EmbeddedSVCHttpTransportOptions` ([#1142](https://github.com/getsentry/sentry-rust/pull/1142)). | ||
| - Added client report protocol types in `sentry-types`, including [`ClientReport`](https://docs.rs/sentry-types/latest/sentry_types/protocol/v7/struct.ClientReport.html), [`ClientReportItem`](https://docs.rs/sentry-types/latest/sentry_types/protocol/v7/struct.ClientReportItem.html), [`DataCategory`](https://docs.rs/sentry-types/latest/sentry_types/protocol/v7/enum.DataCategory.html), and [`DiscardReason`](https://docs.rs/sentry-types/latest/sentry_types/protocol/v7/enum.DiscardReason.html), plus support for serializing [`client_report` envelope items](https://docs.rs/sentry-types/latest/sentry_types/protocol/v7/enum.EnvelopeItem.html#variant.ClientReport) ([#1144](https://github.com/getsentry/sentry-rust/pull/1144)). |
There was a problem hiding this comment.
These docs.rs links of course do not work yet, but should be accurate after release.
193723f to
87946bc
Compare
543b2cc to
1121016
Compare
87946bc to
f0916b0
Compare
1121016 to
7360117
Compare
44a5c99 to
cf7b15e
Compare
5e3017a to
8034ba5
Compare
cf7b15e to
fb0de20
Compare
8034ba5 to
ea171be
Compare
c49b952 to
c5d137d
Compare
| #[serde(default = "SystemTime::now", with = "utils::ts_seconds_float")] | ||
| timestamp: SystemTime, |
There was a problem hiding this comment.
Bug: The Report struct's timestamp field will fail to deserialize if the JSON value is explicitly null, because the custom deserializer does not handle null values.
Severity: HIGH
Suggested Fix
Modify the custom deserializer to handle null values. Implement visit_none in the SecondsTimestampVisitor to return a default value, or wrap the deserialization logic in a function that handles an Option<T> to correctly process null before it reaches the custom timestamp logic.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sentry-types/src/protocol/client_report/mod.rs#L21-L22
Potential issue: The `Report` struct uses `#[serde(default = "SystemTime::now")]`
combined with a custom deserializer `#[serde(with = "utils::ts_seconds_float")]` for its
`timestamp` field. The `default` attribute is only triggered when the field is missing
from the JSON, not when it is explicitly `null`. If a client report is received with
`{"timestamp": null}`, `serde` will attempt to deserialize the `null` value using the
custom `ts_seconds_float` deserializer. This will fail because its internal visitor,
`SecondsTimestampVisitor`, does not implement `visit_none()` to handle `null` values,
causing the entire report to be dropped.
Also affects:
sentry-types/src/protocol/v7.rs:751sentry-types/src/protocol/v7.rs:895sentry-types/src/protocol/v7.rs:1143
408f667 to
a3bcb31
Compare
c5d137d to
84179b2
Compare
a3bcb31 to
a504ed6
Compare
84179b2 to
6d3fc31
Compare
This will allow us to add additional construction options, either to all transports or to individual transports, in the future without breaking the public API. This will be useful in #1004, as we will need to provide transports with a handle to record data losses. For users who use a custom transport/transport factory, this change may produce a minor behavior breakage, as the `&ClientOptions` received by the `create_transport` method (or the closure, in the case of transport factories which are just closures) will no longer contain all of the options set when init-ing the SDK, only those options which are also present in `TransportOptions`. As the API remains fully compatible, however, we are not considering this to be a public API breakage, and will release these changes in a minor/patch release. References [#1004](#1004) References [RUST-156](https://linear.app/getsentry/issue/RUST-156/record-transport-drops-and-attach-client-report-envelopes)
Added WIP client report protocol. Resolves [#1001](#1001) Resolves [RUST-153](https://linear.app/getsentry/issue/RUST-153/add-client-report-protocol-envelope-item-support-in-sentry-types)
6d3fc31 to
1bad2bf
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1bad2bf. Configure here.

Added the client report protocol.
Resolves #1001
Resolves RUST-153