From 6f6cf853322f48ebaf7ab8a323bcce6a38e587f7 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 21 Oct 2024 09:51:08 +0200 Subject: [PATCH] feat(js): Parse debug ids from scraped source files (#1534) --- CHANGELOG.md | 2 + crates/symbolicator-js/src/lookup.rs | 23 +++++++-- ...ation__sourcemap__e2e_scraped_debugid.snap | 47 ++++++++++++++++++ .../tests/integration/sourcemap.rs | 31 ++++++++++++ .../sourcemaps/e2e_scraped_debugid/app.js | 2 + .../sourcemaps/e2e_scraped_debugid/bundle.zip | Bin 0 -> 886 bytes 6 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 crates/symbolicator-js/tests/integration/snapshots/integration__sourcemap__e2e_scraped_debugid.snap create mode 100644 tests/fixtures/sourcemaps/e2e_scraped_debugid/app.js create mode 100644 tests/fixtures/sourcemaps/e2e_scraped_debugid/bundle.zip diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a4d317c0..99edc9348 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - Added `dry-run` mode to the `cleanup` command that simulates the cleanup without deleting files. ([#1531](https://github.com/getsentry/symbolicator/pull/1531)) +- Parse debug identifiers from scraped JavaScript source files + ([#1534](https://github.com/getsentry/symbolicator/pull/1534)) ### Dependencies diff --git a/crates/symbolicator-js/src/lookup.rs b/crates/symbolicator-js/src/lookup.rs index 24dc42286..ce056cb95 100644 --- a/crates/symbolicator-js/src/lookup.rs +++ b/crates/symbolicator-js/src/lookup.rs @@ -31,7 +31,7 @@ use reqwest::Url; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use symbolic::common::{ByteView, DebugId, SelfCell}; -use symbolic::debuginfo::js::discover_sourcemaps_location; +use symbolic::debuginfo::js::{discover_debug_id, discover_sourcemaps_location}; use symbolic::debuginfo::sourcebundle::{ SourceBundleDebugSession, SourceFileDescriptor, SourceFileType, }; @@ -447,6 +447,7 @@ impl CachedFileEntry { pub struct CachedFile { pub contents: Option, sourcemap_url: Option>, + debug_id: Option, } impl fmt::Debug for CachedFile { @@ -493,6 +494,7 @@ impl CachedFile { Ok(Self { contents, sourcemap_url: sourcemap_url.map(Arc::new), + debug_id: None, }) } @@ -580,12 +582,17 @@ impl ArtifactFetcher { self.metrics.record_not_found(SourceFileType::Source); } - // Then fetch the corresponding sourcemap if we have a sourcemap reference - let sourcemap_url = match &minified_source.entry { - Ok(minified_source) => minified_source.sourcemap_url.as_deref(), - Err(_) => None, + // Then fetch the corresponding sourcemap reference and debug_id + let (sourcemap_url, source_debug_id) = match &minified_source.entry { + Ok(minified_source) => ( + minified_source.sourcemap_url.as_deref(), + minified_source.debug_id, + ), + Err(_) => (None, None), }; + let debug_id = debug_id.or(source_debug_id); + // If we don't have sourcemap reference, nor a `DebugId`, we skip creating `SourceMapCache`. if sourcemap_url.is_none() && debug_id.is_none() { self.metrics.record_sourcemap_not_needed(); @@ -600,6 +607,7 @@ impl ArtifactFetcher { entry: Ok(CachedFile { contents: Some(data.clone()), sourcemap_url: None, + debug_id, }), resolved_with: minified_source.resolved_with, }, @@ -781,12 +789,15 @@ impl ArtifactFetcher { .and_then(|sm_ref| SourceMapUrl::parse_with_prefix(abs_path, sm_ref).ok()) .map(Arc::new); + let debug_id = discover_debug_id(&contents); + self.scraping_attempts .push(JsScrapingAttempt::success(abs_path.to_owned())); Ok(CachedFile { contents: Some(contents), sourcemap_url, + debug_id, }) } Err(e) => { @@ -931,9 +942,11 @@ impl ArtifactFetcher { // Get the sourcemap reference from the artifact, either from metadata, or file contents let sourcemap_url = resolve_sourcemap_url(abs_path, &artifact.headers, &contents); + let debug_id = discover_debug_id(&contents); CachedFile { contents: Some(contents), sourcemap_url: sourcemap_url.map(Arc::new), + debug_id, } }), resolved_with: artifact.resolved_with, diff --git a/crates/symbolicator-js/tests/integration/snapshots/integration__sourcemap__e2e_scraped_debugid.snap b/crates/symbolicator-js/tests/integration/snapshots/integration__sourcemap__e2e_scraped_debugid.snap new file mode 100644 index 000000000..f13245fd8 --- /dev/null +++ b/crates/symbolicator-js/tests/integration/snapshots/integration__sourcemap__e2e_scraped_debugid.snap @@ -0,0 +1,47 @@ +--- +source: crates/symbolicator-js/tests/integration/sourcemap.rs +assertion_line: 702 +expression: response +--- +stacktraces: + - frames: + - function: onFailure + filename: test.js + module: files/test + abs_path: "http://localhost:/files/test.js" + lineno: 5 + colno: 11 + pre_context: + - "var makeAFailure = (function() {" + - " function onSuccess(data) {}" + - "" + - " function onFailure(data) {" + context_line: " throw new Error('failed!');" + post_context: + - " }" + - "" + - " function invoke(data) {" + - " var cb = null;" + - " if (data.failed) {" + data: + sourcemap: "http://localhost:/files/app.js" + sourcemap_origin: + bundled: + - "sentry://project_debug_file/1" + - source_map: + debug_id: 2f259f80-58b7-44cb-d7cd-de1505e7e718 + resolved_with: debug-id + symbolicated: true +raw_stacktraces: + - frames: + - abs_path: "http://localhost:/files/app.js" + lineno: 1 + colno: 64 + context_line: "var makeAFailure=function(){function n(n){}function e(n){throw new Error(\"failed!\")}function r(r){var i=null;if(r.failed){i=e}else{i=n}i(r)} {snip}" + post_context: + - "//# debugId=2f259f80-58b7-44cb-d7cd-de1505e7e718" + data: + symbolicated: false +scraping_attempts: + - url: "http://localhost:/files/app.js" + status: success diff --git a/crates/symbolicator-js/tests/integration/sourcemap.rs b/crates/symbolicator-js/tests/integration/sourcemap.rs index e2ab496aa..9707c9880 100644 --- a/crates/symbolicator-js/tests/integration/sourcemap.rs +++ b/crates/symbolicator-js/tests/integration/sourcemap.rs @@ -671,6 +671,37 @@ async fn e2e_multiple_smref_scraped() { assert_snapshot!(response); } +#[tokio::test] +async fn e2e_scraped_debugid() { + let (symbolication, _cache_dir) = setup_service(|_| ()); + let (srv, source) = sourcemap_server("e2e_scraped_debugid", |url, query| { + assert_eq!(query, "debug_id=2f259f80-58b7-44cb-d7cd-de1505e7e718"); + json!([{ + "type": "bundle", + "id": "1", + "url": format!("{url}/bundle.zip"), + "resolved_with": "debug-id", + }]) + }); + + let url = srv.url("/files/"); + let frames = format!( + r#"[{{ + "abs_path": "{url}app.js", + "lineno": 1, + "colno": 64 + }}]"# + ); + let modules = r#"[]"#; + + let mut request = make_js_request(source, &frames, modules, None, None); + request.scraping.enabled = true; + + let response = symbolication.symbolicate_js(request).await; + + assert_snapshot!(response); +} + #[tokio::test] async fn sorted_bundles() { let (symbolication, _cache_dir) = setup_service(|_| ()); diff --git a/tests/fixtures/sourcemaps/e2e_scraped_debugid/app.js b/tests/fixtures/sourcemaps/e2e_scraped_debugid/app.js new file mode 100644 index 000000000..7b5df9c81 --- /dev/null +++ b/tests/fixtures/sourcemaps/e2e_scraped_debugid/app.js @@ -0,0 +1,2 @@ +var makeAFailure=function(){function n(n){}function e(n){throw new Error("failed!")}function r(r){var i=null;if(r.failed){i=e}else{i=n}i(r)}function i(){var n={failed:true,value:42};r(n)}return i}(); +//# debugId=2f259f80-58b7-44cb-d7cd-de1505e7e718 diff --git a/tests/fixtures/sourcemaps/e2e_scraped_debugid/bundle.zip b/tests/fixtures/sourcemaps/e2e_scraped_debugid/bundle.zip new file mode 100644 index 0000000000000000000000000000000000000000..6d4718991f32e92aaef85515b74fdfceeb394601 GIT binary patch literal 886 zcmWHJ40d8-U|i#qaAG&O!2i;VD+9e=U3R;6HX(S$L1$mdwc3k=BZL31_h$IfA1InH zv#nEJ^}ne2(u+bjrtUJ+yznta%zR^O+~vk=Q`>e1+rIWBF(0Vado$Co_T zZ0ejpkE@iwRipi&(>djHLFZI=KYjF2J-WC~SKyy=#1ZDJu6hbe>F0h5O|s#-!8Lum zjgIx6tE!)jR?8;NVUE0WL)cjCKwaPSYd!@N<0BqS(%t!hL&u3*M}aN8IHukB0*825 zJ?GY=Q$zptR=wUUWX0vsyncu@a}z(r9X0*5 zmgSd^aKUn)$a&UlPIRjB&bc$uTjp~{nty($PWI2nE&DGuK3Qf`_|*PuyZx!^$2>a~ zwiUb8T*^?E+!%jOeQ{#7!JVBE;gP40Wi00|xc%9HyMF2gAGK$8r!+pkG?4Gx|NQdh ze-*QQek~51^<}f?tgptEW_GftJLk;t&HVk86*Up0Dy))N4~+cZK+Froxruq1X{p5} zz%-Db=i7gmugQR?_5RZ9BJ4(N`+8npTO`4I+DyOaV0wA;jEd`@zs~u$Cvnz$p@(1Y zbu(AS-%t9h5Fb%h*(iMEspOnF+-nX5?@Igjb9-2UXcFgb1+m|I9-O&(#c!?La>s>o zYYwg}7Q1QTHqA|2*UiWHz^-*y<*f1*t*3Qdy7A4qJNCZtB@ecnmrnSZPJWhrZ%2!_ z+47%)hMqnVCHwSr%nMhRaQ!*S)pVyd>F2VQp=%TVG6i@uGKnzbPT)ZAfx(hSPzFFx l?&!MFLk6muVM!w|FfXBmQ-C)s8%UZ72rGf~b5Leu001TGZ5jXo literal 0 HcmV?d00001