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.
| @@ -6,28 +6,222 @@ use relay_event_schema::protocol::{ | |||
| }; | |||
| use relay_protocol::Annotated; | |||
|
|
|||
| /// 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 |
|
bugbot run |
| 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>
armenzg
force-pushed
the
nel/attributes/armenzg
branch
from
aab68c4 to
bcb7d10
Compare
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).
This updates the initial set of attributes plus it imports the human-readable messages from the Sentry repository.