ref(download): Use download service for objectstore requests

crates/symbolicator-native/src/symbolication/attachments.rs

@@ -1,18 +1,17 @@
 use std::fs::File;
+use std::sync::Arc;
 
-use futures::TryStreamExt;
-use symbolicator_service::caching::CacheError;
-use symbolicator_service::download::{self, DownloadService};
-use symbolicator_service::utils::fs;
-use tokio::io::{AsyncSeekExt, AsyncWriteExt, BufWriter};
+use symbolicator_service::download::{DownloadService, fetch_file};
+use symbolicator_sources::{HttpRemoteFile, RemoteFile};
+use url::Url;
 
 use crate::interface::AttachmentFile;
 
 #[tracing::instrument(skip(download_svc))]
 pub async fn download_attachment(
-    download_svc: &DownloadService,
+    download_svc: Arc<DownloadService>,
     file: AttachmentFile,
-) -> Result<File, CacheError> {
+) -> anyhow::Result<File> {
     let (storage_url, storage_token) = match file {
         AttachmentFile::Local(file) => return Ok(file),
         AttachmentFile::Remote {
@@ -21,39 +20,20 @@
         } => (storage_url, storage_token),
     };
 
-    // TODO: maybe its worth using the actual `DownloadService` instead of straight going to the `trusted_client`.
-    // Doing so would in theory allow us to have retries and error report, as well as being able to
-    // download files in multiple chunks concurrently, but I don’t think our `objecstore` server currently
-    // supports range requests, and those would also mess with streaming decompression.
-    // Not to mention that using the `DownloadService` is not that straight forward.
-    download::retry(|| async {
-        let mut request = download_svc.trusted_client.get(&storage_url);
-        if let Some(token) = storage_token.as_ref() {
-            request = request.bearer_auth(token);
-        }
-        let response = request.send().await?;
-        if !response.status().is_success() {
-            return Err(download::GenericErrorHandler::handle_status(
-                &storage_url,
-                response.status(),
-            )
-            .await);
-        }
-
-        let mut stream = response.bytes_stream();
-
-        let file = fs::tempfile(download_svc.tmp_dir.as_deref())?.into_file();
-        let mut writer = BufWriter::new(tokio::fs::File::from_std(file));
-        while let Some(chunk) = stream.try_next().await? {
-            writer.write_all(&chunk).await?;
-        }
-        writer.flush().await?;
-        let mut file = writer.into_inner();
-        file.sync_data().await?;
-
-        file.rewind().await?;
-
-        Ok(file.into_std().await)
-    })
-    .await
+    let mut http_remote_file = HttpRemoteFile::from_url(Url::parse(&storage_url)?, true);
+
+    if let Some(token) = storage_token {
+        http_remote_file = http_remote_file.bearer_auth(&token);
+    }
+
+    let mut temp_file = tempfile::NamedTempFile::new()?;
+
+    fetch_file(
+        download_svc,
+        RemoteFile::Http(http_remote_file),
+        &mut temp_file,
+    )
+    .await?;
+
+    Ok(temp_file.into_file())
 }

crates/symbolicator-native/src/symbolication/process_minidump.rs

@@ -625,7 +625,7 @@ impl SymbolicationActor {
             rewrite_first_module,
             extract_variables,
         } = request;
-        let minidump_file = download_attachment(&self.download_svc, minidump_file).await?;
+        let minidump_file = download_attachment(self.download_svc.clone(), minidump_file).await?;
         let len = minidump_file.metadata()?.len();
         tracing::debug!("Processing minidump ({} bytes)", len);
         metric!(distribution("minidump.upload.size") = len as f64);

crates/symbolicator-sources/src/sources/http.rs

@@ -88,6 +88,14 @@ impl HttpRemoteFile {
         HttpRemoteFile::new(source, location)
     }
 
+    /// Adds bearer authorization to the request and returns the updated file.
+    pub fn bearer_auth(mut self, token: &str) -> Self {
+        self.headers
+            .0
+            .insert("Authorization".to_owned(), format!("Bearer {token}"));
+        self
+    }
+
     /// Returns a [`RemoteFileUri`] for the file.
     pub fn uri(&self) -> RemoteFileUri {
         match self.url() {