add a source field to ErrorMetadata to prevent re-reporting (#34444)

Adds a `source` field to `ErrorMetadata` that is populated when we (attempt to) report an error to Sentry.
We'll check this field in `should_report_to_sentry`, so errors that are sent across the wire to downstream callers won't
be re-reported.

GitOrigin-RevId: 3407b1d3e621abae6376c4430af6eb7bf85d5536

Author: gautamg795

Cargo.lock

@@ -16,7 +16,10 @@ pub use errors::{
     INTERNAL_SERVER_ERROR,
     INTERNAL_SERVER_ERROR_MSG,
 };
-use metrics::log_counter;
+use metrics::{
+    log_counter,
+    SERVICE_NAME,
+};
 use pb::common::{
     FrameData as FrameDataProto,
     JsError as JsErrorProto,
@@ -122,17 +125,30 @@ pub fn report_error_sync(err: &mut anyhow::Error) {
         log_errors_reported_total(label);
     }
 
-    if let Some(e) = err.downcast_ref::<ErrorMetadata>() {
-        if let Some(counter) = e.custom_metric() {
-            log_counter(counter, 1);
-        }
-    }
-
     tracing::error!(
         "Caught error (RUST_BACKTRACE=1 RUST_LOG=info,{}=debug for full trace): {err:#}",
         module_path!(),
     );
     tracing::debug!("{err:?}");
+
+    if let Some(e) = err.downcast_mut::<ErrorMetadata>() {
+        if let Some(counter) = e.custom_metric() {
+            log_counter(counter, 1);
+        }
+        // Set the source of this error to the service name if it's not already set,
+        // denoting that this error has been reported and downstream callers that
+        // receive this error need not re-report it.
+        match &e.source {
+            Some(source) => {
+                tracing::debug!("Not reporting above error: already reported by {source}");
+                return;
+            },
+            None => {
+                e.source = Some(SERVICE_NAME.clone());
+            },
+        }
+    }
+
     let Some(sentry_client) = sentry::Hub::current().client() else {
         tracing::error!("Not reporting above error: Sentry is not configured");
         return;

crates/common/src/errors.rs

@@ -18,6 +18,7 @@ proptest-derive = { workspace = true, optional = true }
 sentry = { workspace = true }
 thiserror = { workspace = true }
 tonic = { workspace = true }
+tracing = { workspace = true }
 tungstenite = { workspace = true }
 
 [dev-dependencies]

crates/errors/Cargo.toml

@@ -36,6 +36,11 @@ pub struct ErrorMetadata {
     /// human readable - developer facing. Should be longer and descriptive.
     /// Eg "The module name is invalid because it contains an invalid character"
     pub msg: Cow<'static, str>,
+
+    // Optional source of the error (i.e. a service name)
+    // If present, this implies that the error originated in an upstream
+    // service call (and may have already been reported to Sentry).
+    pub r#source: Option<String>,
 }
 
 #[cfg_attr(any(test, feature = "testing"), derive(proptest_derive::Arbitrary))]
@@ -74,6 +79,7 @@ impl ErrorMetadata {
             code,
             short_msg: Cow::Borrowed(""),
             msg: Cow::Borrowed(""),
+            source: None,
         }
     }
 
@@ -89,6 +95,7 @@ impl ErrorMetadata {
             code: ErrorCode::BadRequest,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         }
     }
 
@@ -109,6 +116,7 @@ impl ErrorMetadata {
             code: ErrorCode::NotFound,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         }
     }
 
@@ -125,6 +133,7 @@ impl ErrorMetadata {
             code: ErrorCode::Unauthenticated,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         }
     }
 
@@ -143,6 +152,7 @@ impl ErrorMetadata {
             code: ErrorCode::AuthUpdateFailed,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         }
     }
 
@@ -159,6 +169,7 @@ impl ErrorMetadata {
             code: ErrorCode::Forbidden,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         }
     }
 
@@ -168,6 +179,7 @@ impl ErrorMetadata {
             code: ErrorCode::ClientDisconnect,
             short_msg: CLIENT_DISCONNECTED.into(),
             msg: CLIENT_DISCONNECTED_MSG.into(),
+            source: None,
         }
     }
 
@@ -179,6 +191,7 @@ impl ErrorMetadata {
             code: ErrorCode::MisdirectedRequest,
             short_msg: "MisdirectedRequest".into(),
             msg: "Instance not served by this Conductor".into(),
+            source: None,
         }
     }
 
@@ -200,6 +213,7 @@ impl ErrorMetadata {
             code: ErrorCode::RateLimited,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         }
     }
 
@@ -213,6 +227,7 @@ impl ErrorMetadata {
             code: ErrorCode::OperationalInternalServerError,
             short_msg: INTERNAL_SERVER_ERROR.into(),
             msg: INTERNAL_SERVER_ERROR_MSG.into(),
+            source: None,
         }
     }
 
@@ -241,6 +256,7 @@ impl ErrorMetadata {
             code: ErrorCode::Overloaded,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         }
     }
 
@@ -258,6 +274,7 @@ impl ErrorMetadata {
             code: ErrorCode::RejectedBeforeExecution,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         }
     }
 
@@ -274,6 +291,7 @@ impl ErrorMetadata {
             },
             short_msg: OCC_ERROR.into(),
             msg: OCC_ERROR_MSG.into(),
+            source: None,
         }
     }
 
@@ -306,6 +324,7 @@ impl ErrorMetadata {
                 subsequent retry. {write_source_description}See https://docs.convex.dev/error#1",
             )
             .into(),
+            source: None,
         }
     }
 
@@ -314,6 +333,7 @@ impl ErrorMetadata {
             code: ErrorCode::Overloaded,
             short_msg: "ServiceUnavailable".into(),
             msg: "Service temporarily unavailable".into(),
+            source: None,
         }
     }
 
@@ -326,6 +346,7 @@ impl ErrorMetadata {
             code: ErrorCode::OutOfRetention,
             short_msg: INTERNAL_SERVER_ERROR.into(),
             msg: INTERNAL_SERVER_ERROR_MSG.into(),
+            source: None,
         }
     }
 
@@ -343,6 +364,7 @@ impl ErrorMetadata {
             code: ErrorCode::PaginationLimit,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         }
     }
 
@@ -356,6 +378,7 @@ impl ErrorMetadata {
             code,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: None,
         })
     }
 
@@ -442,6 +465,7 @@ impl ErrorMetadata {
         if self.short_msg.is_empty() {
             return None;
         }
+
         match self.code {
             ErrorCode::ClientDisconnect => None,
             ErrorCode::BadRequest

crates/errors/src/lib.rs

@@ -31,6 +31,7 @@ message ErrorMetadata {
   optional string short_msg = 2;
   optional string msg = 3;
   optional OccInfo occ_info = 4;
+  optional string source = 5;
 }
 
 // The message we put in tonic::Status details.

crates/pb/protos/errors.proto

@@ -85,6 +85,7 @@ impl From<ErrorMetadata> for ErrorMetadataProto {
                 }),
                 _ => None,
             },
+            source: metadata.source,
         }
     }
 }
@@ -101,6 +102,7 @@ impl TryFrom<ErrorMetadataProto> for ErrorMetadata {
             code,
             short_msg: short_msg.into(),
             msg: msg.into(),
+            source: metadata.source,
         })
     }
 }