Skip to content
Permalink

Comparing changes

This is a direct comparison between two commits made in this repository or its related repositories. View the default comparison for this range or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: getsentry/symbolicator
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: a8ee1cfb817a711a9e0b574e30f20d409781de5c
Choose a base ref
..
head repository: getsentry/symbolicator
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 3498cecdd8bd1f536ef4b8f2b9216155951e6202
Choose a head ref
18 changes: 10 additions & 8 deletions crates/symbolicator-js/src/bundle_lookup.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::fmt;
use std::time::Duration;

use symbolicator_service::caches::ByteViewString;
use symbolicator_sources::RemoteFileUri;

use crate::interface::ResolvedWith;
@@ -35,11 +34,12 @@ impl FileInBundleCache {
.time_to_idle(Duration::from_secs(60 * 60))
.name("file-in-bundle")
.weigher(|_k, v| {
let content_size =
v.0.entry
.as_ref()
.map(|cached_file| cached_file.contents.len())
.unwrap_or_default();
let content_size = v
.0
.entry
.as_ref()
.map(|cached_file| cached_file.contents.as_deref().unwrap_or_default().len())
.unwrap_or_default();
(std::mem::size_of_val(v) + content_size)
.try_into()
.unwrap_or(u32::MAX)
@@ -83,8 +83,10 @@ impl FileInBundleCache {
let mut file_entry = file_entry.clone();
if matches!(key, FileKey::SourceMap { .. }) {
if let Ok(cached_file) = file_entry.entry.as_mut() {
cached_file.has_contents = true;
cached_file.contents = ByteViewString::from(String::new());
// SourceMaps are usually very large, and we only need them for
// `sourcemapcache` creation. So do not persist the actual contents here.
// Rather load them later in `sourcemapcache` generation if needed.
cached_file.contents = None;
}
}
let key = (bundle_uri.clone(), key.clone());
10 changes: 8 additions & 2 deletions crates/symbolicator-js/src/lookup.rs
Original file line number Diff line number Diff line change
@@ -540,7 +540,9 @@ impl CachedFile {
}

pub fn owned_contents(&self) -> ByteViewString {
self.contents.unwrap_or(ByteViewString::from(String::new()))
self.contents
.clone()
.unwrap_or(ByteViewString::from(String::new()))
}

/// Returns a string representation of a SourceMap URL if it was coming from a remote resource.
@@ -1286,7 +1288,11 @@ impl ArtifactFetcher {
&self.artifact_bundles,
&sourcemap.uri,
sourcemap_entry,
);
)
.unwrap_or_else(|| {
tracing::error!("expected either a `Bundled` or `Resolved` SourceMap");
SourceMapContents::Resolved(ByteViewString::from(String::new()))
});

let req = FetchSourceMapCacheInternal {
source: source_content,
2 changes: 1 addition & 1 deletion crates/symbolicator-js/src/sourcemap_cache.rs
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@ impl SourceMapContents {
return None;
};

let (bundle, _) = artifact_bundles.get(bundle_uri)?.ok()?;
let (bundle, _) = artifact_bundles.get(bundle_uri)?.as_ref().ok()?;
let bundle = bundle.owner().clone();
let contents = Self::FromBundle(bundle, key.clone());
Some(contents)
4 changes: 2 additions & 2 deletions crates/symbolicator-js/src/symbolication.rs
Original file line number Diff line number Diff line change
@@ -121,7 +121,7 @@ async fn symbolicate_js_frame(
match &module.minified_source.entry {
Ok(minified_source) => {
if should_apply_source_context {
apply_source_context(raw_frame, &minified_source.contents)?
apply_source_context(raw_frame, minified_source.contents())?
}
}
Err(CacheError::DownloadError(msg)) if msg == "Scraping disabled" => {
@@ -267,7 +267,7 @@ async fn symbolicate_js_frame(
if source_file
.as_ref()
.map_err(|_| JsModuleErrorKind::MissingSource)
.and_then(|file| apply_source_context(&mut frame, &file.contents))
.and_then(|file| apply_source_context(&mut frame, file.contents()))
.is_err()
{
// It's arguable whether we should collect it, but this is what monolith does now,