Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/symbolicator-native/src/symbolication/apple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ impl SymbolicationActor {
sources: Arc<[SourceConfig]>,
scraping: ScrapingConfig,
) -> Result<CompletedSymbolicationResponse> {
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?;
Expand Down
50 changes: 19 additions & 31 deletions crates/symbolicator-native/src/symbolication/attachments.rs
Original file line number Diff line number Diff line change
@@ -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<DownloadService>,
file: AttachmentFile,
) -> anyhow::Result<File> {
let (storage_url, storage_token) = match file {
Expand All @@ -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())
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions crates/symbolicator-sources/src/sources/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading