From 1c17276d15d4eda8ff6ba2e7ed3ba936904c079e Mon Sep 17 00:00:00 2001
From: Daniel Szoke <daniel.szoke@sentry.io>
Date: Mon, 29 Sep 2025 09:57:08 +0200
Subject: [PATCH] ref(vcs): Improve `find_base_sha` debuggability

Rather than emitting `debug!` logs (which we don't show in tests) within the `find_base_sha` and `extract_pr_base_sha_from_event` function, improve these methods by returning proper error types, which include the underlying error as a cause.

The calling code is now responsible for debug-logging these errors, which is still desired, as we expect these functions to error when run outside of a GitHub Actions environment. But, now we will have more information available when debugging test failures, and the debug logs at runtime will also likely be more detailed.
---
 src/commands/build/upload.rs |  8 +++++-
 src/utils/vcs.rs             | 56 ++++++++++++++++--------------------
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/src/commands/build/upload.rs b/src/commands/build/upload.rs
index ab2de1d462..b1e90bd488 100644
--- a/src/commands/build/upload.rs
+++ b/src/commands/build/upload.rs
@@ -231,7 +231,13 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
         .get_one("base_sha")
         .map(String::as_str)
         .map(Cow::Borrowed)
-        .or_else(|| vcs::find_base_sha().ok().map(Cow::Owned));
+        .or_else(|| {
+            vcs::find_base_sha()
+                .inspect_err(|e| debug!("Error finding base SHA: {}", e))
+                .ok()
+                .flatten()
+                .map(Cow::Owned)
+        });
     let pr_number = matches
         .get_one("pr_number")
         .copied()
diff --git a/src/utils/vcs.rs b/src/utils/vcs.rs
index 93f190650f..8ec876444d 100644
--- a/src/utils/vcs.rs
+++ b/src/utils/vcs.rs
@@ -3,7 +3,7 @@
 use std::fmt;
 use std::path::PathBuf;
 
-use anyhow::{bail, format_err, Error, Result};
+use anyhow::{bail, format_err, Context as _, Error, Result};
 use chrono::{DateTime, FixedOffset, TimeZone as _};
 use git2::{Commit, Repository, Time};
 use if_chain::if_chain;
@@ -569,20 +569,13 @@ pub fn find_head() -> Result<String> {
     Ok(head.id().to_string())
 }
 
-pub fn find_base_sha() -> Result<String> {
-    if let Some(pr_base_sha) = std::env::var("GITHUB_EVENT_PATH")
-        .ok()
-        .and_then(|event_path| std::fs::read_to_string(event_path).ok())
-        .and_then(|content| extract_pr_base_sha_from_event(&content))
-    {
-        debug!(
-            "Using GitHub Actions PR base SHA from event payload: {}",
-            pr_base_sha
-        );
-        return Ok(pr_base_sha);
-    }
+pub fn find_base_sha() -> Result<Option<String>> {
+    let github_event = std::env::var("GITHUB_EVENT_PATH")
+        .map_err(Error::from)
+        .and_then(|event_path| std::fs::read_to_string(event_path).map_err(Error::from))
+        .context("Failed to read GitHub event path")?;
 
-    Err(anyhow::anyhow!("No base SHA available"))
+    extract_pr_base_sha_from_event(&github_event)
 }
 
 /// Extracts the PR head SHA from GitHub Actions event payload JSON.
@@ -602,19 +595,15 @@ fn extract_pr_head_sha_from_event(json_content: &str) -> Option<String> {
 }
 
 /// Extracts the PR base SHA from GitHub Actions event payload JSON.
-/// Returns None if not a PR event or if SHA cannot be extracted.
-fn extract_pr_base_sha_from_event(json_content: &str) -> Option<String> {
-    let v: Value = match serde_json::from_str(json_content) {
-        Ok(v) => v,
-        Err(_) => {
-            debug!("Failed to parse GitHub event payload as JSON");
-            return None;
-        }
-    };
+/// Returns Ok(None) if not a PR event or if SHA cannot be extracted.
+/// Returns an error if we cannot parse the JSON.
+fn extract_pr_base_sha_from_event(json_content: &str) -> Result<Option<String>> {
+    let v: Value = serde_json::from_str(json_content)
+        .context("Failed to parse GitHub event payload as JSON")?;
 
-    v.pointer("/pull_request/base/sha")
+    Ok(v.pointer("/pull_request/base/sha")
         .and_then(|s| s.as_str())
-        .map(|s| s.to_owned())
+        .map(|s| s.to_owned()))
 }
 
 /// Given commit specs, repos and remote_name this returns a list of head
@@ -1712,7 +1701,7 @@ mod tests {
         .to_string();
 
         assert_eq!(
-            extract_pr_base_sha_from_event(&pr_json),
+            extract_pr_base_sha_from_event(&pr_json).unwrap(),
             Some("55e6bc8c264ce95164314275d805f477650c440d".to_owned())
         );
 
@@ -1726,10 +1715,10 @@ mod tests {
         })
         .to_string();
 
-        assert_eq!(extract_pr_base_sha_from_event(&push_json), None);
+        assert_eq!(extract_pr_base_sha_from_event(&push_json).unwrap(), None);
 
         // Test with malformed JSON
-        assert_eq!(extract_pr_base_sha_from_event("invalid json {"), None);
+        assert!(extract_pr_base_sha_from_event("invalid json {").is_err());
 
         // Test with missing base SHA
         let incomplete_json = r#"{
@@ -1740,7 +1729,10 @@ mod tests {
   }
 }"#;
 
-        assert_eq!(extract_pr_base_sha_from_event(incomplete_json), None);
+        assert_eq!(
+            extract_pr_base_sha_from_event(incomplete_json).unwrap(),
+            None
+        );
     }
 
     #[test]
@@ -1769,8 +1761,10 @@ mod tests {
         std::env::set_var("GITHUB_EVENT_PATH", event_file.to_str().unwrap());
 
         let result = find_base_sha();
-        assert!(result.is_ok());
-        assert_eq!(result.unwrap(), "55e6bc8c264ce95164314275d805f477650c440d");
+        assert_eq!(
+            result.unwrap().unwrap(),
+            "55e6bc8c264ce95164314275d805f477650c440d"
+        );
 
         // Test without GITHUB_EVENT_PATH
         std::env::remove_var("GITHUB_EVENT_PATH");