diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c3a6bea375..3e189794024 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Features**: - No longer write the deprecated `sentry.transaction` and `db.system` attributes. ([#6237](https://github.com/getsentry/relay/pull/6237), [#6238](https://github.com/getsentry/relay/pull/6238)) +- Allow additional exceptions in minidump and apple crash report events. ([#6241](https://github.com/getsentry/relay/pull/6241)) ## 26.7.0 diff --git a/relay-dynamic-config/src/feature.rs b/relay-dynamic-config/src/feature.rs index 959b5476071..d206b0a7ab6 100644 --- a/relay-dynamic-config/src/feature.rs +++ b/relay-dynamic-config/src/feature.rs @@ -107,9 +107,6 @@ pub enum Feature { /// See . #[serde(rename = "projects:relay-upload-multipart")] UploadMultipart, - /// Allow additional exceptions to accompany minidumps. - #[serde(rename = "projects:minidump-multi-exception")] - MinidumpMultiException, /// Enable relay billing outcome generation. #[serde(rename = "organizations:relay-generate-billing-outcome")] GenerateBillingOutcome, diff --git a/relay-server/src/processing/errors/errors/apple_crash_report.rs b/relay-server/src/processing/errors/errors/apple_crash_report.rs index 3baa3b22412..e3f3d2a0155 100644 --- a/relay-server/src/processing/errors/errors/apple_crash_report.rs +++ b/relay-server/src/processing/errors/errors/apple_crash_report.rs @@ -28,7 +28,7 @@ impl SentryError for AppleCrashReport { utils::if_processing!(ctx, { crate::utils::process_apple_crash_report( event.get_or_insert_with(Default::default), - ctx.processing.project_info, + crate::utils::AdditionalExceptions::Retain, ); metrics.bytes_ingested_event_applecrashreport = (apple_crash_report.len() as u64).into(); diff --git a/relay-server/src/processing/errors/errors/minidump.rs b/relay-server/src/processing/errors/errors/minidump.rs index 45aa22fd041..502d79ce0ef 100644 --- a/relay-server/src/processing/errors/errors/minidump.rs +++ b/relay-server/src/processing/errors/errors/minidump.rs @@ -5,6 +5,8 @@ use crate::managed::{Counted, Quantities, RecordKeeper}; use crate::processing::ForwardContext; use crate::processing::errors::errors::{Context, Expansion, SentryError, utils}; use crate::processing::errors::{Error, Result}; +#[cfg(feature = "processing")] +use crate::utils::AdditionalExceptions; #[derive(Debug)] pub struct Minidump(pub Item); @@ -29,7 +31,7 @@ impl SentryError for Minidump { crate::utils::process_minidump( event.get_or_insert_with(Default::default), &minidump, - ctx.processing.project_info, + AdditionalExceptions::Retain, ); metrics.bytes_ingested_event_minidump = (minidump.attachment_body_size() as u64).into(); }); diff --git a/relay-server/src/processing/errors/errors/playstation.rs b/relay-server/src/processing/errors/errors/playstation.rs index 6c2f346c4f3..30b3d3b0587 100644 --- a/relay-server/src/processing/errors/errors/playstation.rs +++ b/relay-server/src/processing/errors/errors/playstation.rs @@ -107,7 +107,7 @@ impl SentryError for Playstation { // If the original prosperodump is already rate limited, so will be the minidump. item.set_rate_limited(prosperodump.rate_limited()); - crate::utils::process_minidump(event.get_or_insert_with(Event::default), &item, ctx.processing.project_info); + crate::utils::process_minidump(event.get_or_insert_with(Event::default), &item, crate::utils::AdditionalExceptions::Delete); item }; diff --git a/relay-server/src/processing/errors/errors/unreal.rs b/relay-server/src/processing/errors/errors/unreal.rs index 44af0186767..e0eb89fe979 100644 --- a/relay-server/src/processing/errors/errors/unreal.rs +++ b/relay-server/src/processing/errors/errors/unreal.rs @@ -5,6 +5,8 @@ use crate::managed::{Counted, Quantities, RecordKeeper}; use crate::processing::ForwardContext; use crate::processing::errors::Result; use crate::processing::errors::errors::{Context, Expansion, SentryError, utils}; +#[cfg(feature = "processing")] +use crate::utils::AdditionalExceptions; #[derive(Debug)] pub enum UnrealReport { @@ -102,14 +104,13 @@ impl SentryError for Unreal { if let Some(minidump) = &minidump { crate::utils::process_minidump( event.get_or_insert_with(Default::default), - minidump, - ctx.processing.project_info + minidump, AdditionalExceptions::Delete ); metrics.bytes_ingested_event_minidump = (minidump.attachment_body_size() as u64).into(); } if let Some(acr) = &apple_crash_report { crate::utils::process_apple_crash_report( - event.get_or_insert_with(Default::default), ctx.processing.project_info + event.get_or_insert_with(Default::default), AdditionalExceptions::Delete ); metrics.bytes_ingested_event_applecrashreport = (acr.len() as u64).into(); } diff --git a/relay-server/src/utils/native.rs b/relay-server/src/utils/native.rs index 141a92a1d6c..e54236fecca 100644 --- a/relay-server/src/utils/native.rs +++ b/relay-server/src/utils/native.rs @@ -10,7 +10,6 @@ use chrono::{TimeZone, Utc}; use minidump::{ MinidumpAnnotation, MinidumpCrashpadInfo, MinidumpModuleList, Module, StabilityReport, }; -use relay_dynamic_config::Feature; use relay_event_schema::protocol::{ ClientSdkInfo, Context, Contexts, Event, Exception, JsonLenientString, Level, Mechanism, StabilityReportContext, Values, @@ -18,7 +17,6 @@ use relay_event_schema::protocol::{ use relay_protocol::{Annotated, Value, get_value}; use crate::envelope::{Item, ItemType}; -use crate::services::projects::project::ProjectInfo; type Minidump<'a> = minidump::Minidump<'a, &'a [u8]>; @@ -40,6 +38,13 @@ struct NativePlaceholder { mechanism_type: &'static str, } +#[derive(Clone, Copy)] +/// What to do with additional exceptions in a minidump / apple crash report event. +pub enum AdditionalExceptions { + Retain, + Delete, +} + /// Writes a placeholder to indicate that this event has an associated minidump or an apple /// crash report. /// @@ -48,7 +53,7 @@ struct NativePlaceholder { fn write_native_placeholder( event: &mut Event, placeholder: NativePlaceholder, - project_info: &ProjectInfo, + additional_exceptions: AdditionalExceptions, ) { // Events must be native platform. let platform = event.platform.value_mut(); @@ -71,7 +76,6 @@ fn write_native_placeholder( .value_mut() .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(), @@ -79,13 +83,12 @@ fn write_native_placeholder( additional_exception_mechanism = ?get_value!(exc.mechanism.ty), sentry_project = ?event.project, event_id = ?event.id, - has_feature = allow_multiple_exceptions, platform = ?event.platform, "Native event has additional exceptions", ) } - if !allow_multiple_exceptions { + if matches!(additional_exceptions, AdditionalExceptions::Delete) { exceptions.clear(); // clear previous errors if any } @@ -223,14 +226,18 @@ fn write_crashpad_annotations( /// /// This function operates at best-effort. It always attaches the placeholder and returns /// successfully, even if the minidump or part of its data cannot be parsed. -pub fn process_minidump(event: &mut Event, item: &Item, project_info: &ProjectInfo) { +pub fn process_minidump( + event: &mut Event, + item: &Item, + additional_exceptions: AdditionalExceptions, +) { debug_assert_eq!(item.ty(), &ItemType::Attachment); let placeholder = NativePlaceholder { exception_type: "Minidump", exception_value: "Invalid Minidump", mechanism_type: "minidump", }; - write_native_placeholder(event, placeholder, project_info); + write_native_placeholder(event, placeholder, additional_exceptions); if item.is_attachment_ref() { // We don't have a full minidump, just a placeholder for something that was uploaded @@ -293,11 +300,11 @@ pub fn process_minidump(event: &mut Event, item: &Item, project_info: &ProjectIn /// Writes minimal information into the event to indicate it is associated with an Apple Crash /// Report. -pub fn process_apple_crash_report(event: &mut Event, project_info: &ProjectInfo) { +pub fn process_apple_crash_report(event: &mut Event, additional_exceptions: AdditionalExceptions) { let placeholder = NativePlaceholder { exception_type: "AppleCrashReport", exception_value: "Invalid Apple Crash Report", mechanism_type: "applecrashreport", }; - write_native_placeholder(event, placeholder, project_info); + write_native_placeholder(event, placeholder, additional_exceptions); } diff --git a/tests/integration/test_minidump.py b/tests/integration/test_minidump.py index fde87229890..2b0a646f55a 100644 --- a/tests/integration/test_minidump.py +++ b/tests/integration/test_minidump.py @@ -714,9 +714,8 @@ def test_minidump_with_processing_invalid( ] -@pytest.mark.parametrize("feature_flag", [False, True]) def test_minidump_with_event_exception( - mini_sentry, relay_with_processing, attachments_consumer, feature_flag + mini_sentry, relay_with_processing, attachments_consumer ): """ An envelope can carry both a minidump attachment and an event item that already @@ -732,8 +731,6 @@ def test_minidump_with_event_exception( project_id = 42 project_config = mini_sentry.add_full_project_config(project_id) config = project_config["config"] - if feature_flag: - config.setdefault("features", []).append("projects:minidump-multi-exception") # Disable scrubbing, the basic and full project configs from the mini_sentry fixture # will modify the minidump since it contains user paths in the module list. @@ -791,12 +788,9 @@ def test_minidump_with_event_exception( assert minidump_exception["mechanism"]["type"] == "minidump" # The user-provided exception with its stack trace must be preserved. - if feature_flag: - (user_exception,) = additional_exceptions - assert user_exception["value"] == "division by zero" - assert user_exception["stacktrace"]["frames"][0]["function"] == "divide" - else: - assert not additional_exceptions + (user_exception,) = additional_exceptions + assert user_exception["value"] == "division by zero" + assert user_exception["stacktrace"]["frames"][0]["function"] == "divide" # The minidump must still be forwarded as an attachment. assert any(att["name"] == "minidump.dmp" for att in message["attachments"])