feat(nel): Update attributes & human readable messages - #4874
Conversation
This updates the initial log attribues plus it imports the human-readable messages from the Sentry repository.
|
|
||
| /// Mapping of NEL error types to their human-readable descriptions | ||
| /// Based on W3C Network Error Logging specification and Chromium-specific extensions | ||
| static NEL_CULPRITS: &[(&str, &str)] = &[ |
There was a problem hiding this comment.
| ), | ||
| ( | ||
| "http.error", | ||
| "The user agent successfully received a response, but it had a {} status code", |
There was a problem hiding this comment.
This error takes a status code. See helper functions on handling this case.
|
bugbot run |
Dav1dde
left a comment
Dav1dde
left a comment
There was a problem hiding this comment.
Just a quick review, as I don't have more time right now.
|
bugbot run |
armenzg
left a comment
armenzg
left a comment
There was a problem hiding this comment.
This is now in better shape. I still want to keep it as a draft.
| let body = raw_report.body.into_value()?; | ||
|
|
||
| // Extract the error type string to avoid borrowing issues later | ||
| let error_type = body.ty.as_str().unwrap_or("unknown"); |
There was a problem hiding this comment.
@Dav1dde What is a good strategy for when the reports may not have the data shape that we expect?
Should we return Sentry errors so we can investigate?
There was a problem hiding this comment.
That would be a possibility, we can create an error and add the raw json as attachment, if we return an error from this function we just have to slightly change the caller side and you'd get that error.
See relay-server/src/services/processing/nel.rs
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
| use super::*; | ||
| use chrono::{DateTime, Utc}; | ||
| use relay_event_schema::protocol::{BodyRaw, IpAddr, NetworkReportPhases}; | ||
| use relay_protocol::{Annotated, SerializableAnnotated}; | ||
|
|
||
| #[test] | ||
| fn test_create_message() { | ||
| // Test cases for create_message | ||
| struct CreateMessageCase { | ||
| error_type: &'static str, | ||
| status_code: Option<u16>, | ||
| expected: &'static str, | ||
| } | ||
|
|
||
| let message_cases = vec
There was a problem hiding this comment.
The test test_create_message is well-structured, but consider adding edge cases like: empty error_type, very large status codes, and error_type with special characters. Also, test the case where status_code is provided for non-http.error types to ensure it's ignored correctly.
Co-authored-by: seer-by-sentry[bot] <157164994+seer-by-sentry[bot]@users.noreply.github.com>
Co-authored-by: seer-by-sentry[bot] <157164994+seer-by-sentry[bot]@users.noreply.github.com>
aab68c4 to
bcb7d10
Compare
Dav1dde
left a comment
Dav1dde
left a comment
There was a problem hiding this comment.
Mostly nits and just Rust things 👍
All the docs I suggested changes on are to follow our little internal style guide, but that usually lines up in general with Rust best practices.
| let body = raw_report.body.into_value()?; | ||
|
|
||
| // Extract the error type string to avoid borrowing issues later | ||
| let error_type = body.ty.as_str().unwrap_or("unknown"); |
There was a problem hiding this comment.
That would be a possibility, we can create an error and add the raw json as attachment, if we return an error from this function we just have to slightly change the caller side and you'd get that error.
See relay-server/src/services/processing/nel.rs
Co-authored-by: David Herberth <[email protected]>
The changes are reflecting the discussions from [this document](https://www.notion.so/sentry/Browser-Reports-Attribute-21e8b10e4b5d8045a7fdc936c814ed70) and follow-up to [this Relay PR](getsentry/relay#4874).
The changes are reflecting the discussions from [this document](https://www.notion.so/sentry/Browser-Reports-Attribute-21e8b10e4b5d8045a7fdc936c814ed70) and follow-up to [this Relay PR](getsentry/relay#4874).
2 participants
This updates the initial set of attributes plus it imports the human-readable messages from the Sentry repository.