feat(native): GA multi-exceptions#6241
Conversation
| /// Allow additional exceptions to accompany minidumps. | ||
| #[serde(rename = "projects:minidump-multi-exception")] | ||
| MinidumpMultiException, |
There was a problem hiding this comment.
No need to add this to GRADUATED_FEATURE_FLAGS: The feature flag was only used for minidumps, for which the native placeholder was only ever written in processing relays.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| event: &mut Event, | ||
| placeholder: NativePlaceholder, | ||
| project_info: &ProjectInfo, | ||
| additional_exceptions: AdditionalExceptions, |
There was a problem hiding this comment.
This parameter exists to differentiate between the minidump and playstation cases?
There was a problem hiding this comment.
Yes, I got test failures on the playstation endpoint and cursor complains about unreal. We might want to get rid of this parameter eventually but I didn't want to risk breaking any of those use cases for now (it could be that they implicitly rely on the deletion).
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d6c4046. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| .get_or_insert_with(Vec::new); | ||
|
|
||
| let allow_multiple_exceptions = project_info.has_feature(Feature::MinidumpMultiException); | ||
| if let Some(exc) = exceptions.first() { | ||
| relay_log::info!( | ||
| additional_exceptions = exceptions.len(), | ||
| native_exception_mechanism = placeholder.exception_type, |
There was a problem hiding this comment.
Bug: The logic to delete additional exceptions for Playstation and Unreal events has been removed, contradicting the stated requirement in the pull request description to preserve this behavior.
Severity: MEDIUM
Suggested Fix
Re-introduce logic to clear additional exceptions for Playstation and Unreal platforms. This could be achieved by passing platform information to the write_native_placeholder function and conditionally calling exceptions.clear() based on the platform, or by adding the clearing logic directly within the specific processing paths for Playstation and Unreal.
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: relay-server/src/utils/native.rs#L65-L82
Potential issue: The code change removes the mechanism that previously controlled
whether additional exceptions are kept or deleted based on a project feature flag. The
function `write_native_placeholder` in `native.rs` no longer receives `project_info` and
the logic that checked for `Feature::MinidumpMultiException` before conditionally
calling `exceptions.clear()` has been removed. The pull request description explicitly
states that Playstation and Unreal platforms should continue to have additional
exceptions deleted. However, with this change, all platforms will now unconditionally
have multiple exceptions, which contradicts the intended behavior for Playstation and
Unreal.
Also affects:
relay-server/src/processing/errors/errors/playstation.rs:107~113relay-server/src/processing/errors/errors/unreal.rs:102~114
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
The sentry side is now ready to support multiple exceptions, see getsentry/sentry#119780
Playstation and Unreal still delete additional exceptions, just to reduce the blast radius.
ref: RELAY-256