diff --git a/crates/symbolicator-native/src/symbolication/apple.rs b/crates/symbolicator-native/src/symbolication/apple.rs index afe51d8e3..0f2b641e0 100644 --- a/crates/symbolicator-native/src/symbolication/apple.rs +++ b/crates/symbolicator-native/src/symbolication/apple.rs @@ -134,7 +134,7 @@ impl SymbolicationActor { sources: Arc<[SourceConfig]>, scraping: ScrapingConfig, ) -> Result { - let report = download_attachment(&self.download_svc, report).await?; + let report = download_attachment(self.download_svc.clone(), report).await?; let (request, state) = self.parse_apple_crash_report(platform, scope, report, sources, scraping)?; let mut response = self.symbolicate(request).await?; diff --git a/crates/symbolicator-native/src/symbolication/attachments.rs b/crates/symbolicator-native/src/symbolication/attachments.rs index e206e4106..cd49fe5b1 100644 --- a/crates/symbolicator-native/src/symbolication/attachments.rs +++ b/crates/symbolicator-native/src/symbolication/attachments.rs @@ -1,15 +1,14 @@ use std::fs::File; -use std::pin::pin; +use std::sync::Arc; -use futures::TryStreamExt; -use symbolicator_service::download::DownloadService; -use tokio::io::{AsyncSeekExt, AsyncWriteExt, BufWriter}; -use tokio_util::io::StreamReader; +use symbolicator_service::download::{DownloadService, fetch_file}; +use symbolicator_sources::{HttpRemoteFile, RemoteFile}; +use url::Url; use crate::interface::AttachmentFile; pub async fn download_attachment( - download_svc: &DownloadService, + download_svc: Arc, file: AttachmentFile, ) -> anyhow::Result { let (storage_url, storage_token) = match file { @@ -20,31 +19,20 @@ pub async fn download_attachment( } => (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. - let mut request = download_svc.trusted_client.get(storage_url); + let mut http_remote_file = HttpRemoteFile::from_url(Url::parse(&storage_url)?, true); + if let Some(token) = storage_token { - request = request.bearer_auth(token); + http_remote_file = http_remote_file.bearer_auth(&token); } - let stream = request - .send() - .await? - .error_for_status()? - .bytes_stream() - .map_err(std::io::Error::other); - let mut reader = pin!(StreamReader::new(stream)); - - let file = tempfile::tempfile()?; - let mut writer = BufWriter::new(tokio::fs::File::from_std(file)); - tokio::io::copy(&mut reader, &mut writer).await?; - writer.flush().await?; - let mut file = writer.into_inner(); - file.sync_data().await?; - - file.rewind().await?; - - Ok(file.into_std().await) + + 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()) } diff --git a/crates/symbolicator-native/src/symbolication/process_minidump.rs b/crates/symbolicator-native/src/symbolication/process_minidump.rs index 0c52b6d98..f07ae8102 100644 --- a/crates/symbolicator-native/src/symbolication/process_minidump.rs +++ b/crates/symbolicator-native/src/symbolication/process_minidump.rs @@ -611,7 +611,7 @@ impl SymbolicationActor { scraping, rewrite_first_module, } = 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); diff --git a/crates/symbolicator-sources/src/sources/http.rs b/crates/symbolicator-sources/src/sources/http.rs index 5e19c1449..2c20d7de8 100644 --- a/crates/symbolicator-sources/src/sources/http.rs +++ b/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() {