Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fall back to co-location heuristic if sourcemap url appears remote #1871

Merged
Merged
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
16 changes: 15 additions & 1 deletion src/utils/sourcemaps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,19 @@ fn is_remote_url(url: &str) -> bool {
};
}

/// Return true if url appears to be a URL path.
/// Most often, a URL path will begin with `/`,
/// particularly in the case of static asset collection and hosting,
/// but such a path is very unlikely to exist in the local filesystem.
fn is_url_path(url: &str) -> bool {
url.starts_with('/') && !Path::new(url).exists()
szokeasaurusrex marked this conversation as resolved.
Show resolved Hide resolved
}

/// Return true iff url is probably not a local file path.
fn is_remote_sourcemap(url: &str) -> bool {
is_remote_url(url) || is_url_path(url)
}

impl SourceMapProcessor {
/// Creates a new sourcemap validator.
pub fn new() -> SourceMapProcessor {
Expand Down Expand Up @@ -378,7 +391,8 @@ impl SourceMapProcessor {
// that can't be resolved to a source map file.
// Instead, we pretend we failed to discover the location, and we fall back to
// guessing the source map location based on the source location.
let location = discover_sourcemaps_location(contents).filter(|loc| !is_remote_url(loc));
let location =
discover_sourcemaps_location(contents).filter(|loc| !is_remote_sourcemap(loc));
let sourcemap_reference = match location {
Some(url) => SourceMapReference::from_url(url.to_string()),
None => match guess_sourcemap_reference(&sourcemaps, &source.url) {
Expand Down
6 changes: 4 additions & 2 deletions tests/integration/_cases/sourcemaps/sourcemaps-inject.trycmd
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
$ sentry-cli sourcemaps inject ./server ./static
? success
> Searching ./server
> Found 14 files
> Found 16 files
> Searching ./static
> Found 8 files
> Analyzing 22 sources
> Analyzing 24 sources
> Injecting debug ids

Source Map Debug ID Injection Report
Expand All @@ -14,6 +14,7 @@ Source Map Debug ID Injection Report
[..]-[..]-[..]-[..]-[..] - ./server/chunks/1.js
[..]-[..]-[..]-[..]-[..] - ./server/dummy_embedded.js
[..]-[..]-[..]-[..]-[..] - ./server/dummy_hosted.js
[..]-[..]-[..]-[..]-[..] - ./server/dummy_hosted_full.js
[..]-[..]-[..]-[..]-[..] - ./server/flight-manifest.js
[..]-[..]-[..]-[..]-[..] - ./server/pages/_document.js
[..]-[..]-[..]-[..]-[..] - ./server/pages/api/hello.js
Expand All @@ -23,6 +24,7 @@ Source Map Debug ID Injection Report
[..]-[..]-[..]-[..]-[..] - ./static/chunks/pages/asdf-05b39167abbe433b.js
Modified: The following sourcemap files have been modified to have debug ids
[..]-[..]-[..]-[..]-[..] - ./server/dummy_hosted.js.map
[..]-[..]-[..]-[..]-[..] - ./server/dummy_hosted_full.js.map
[..]-[..]-[..]-[..]-[..] - ./server/pages/_document.js.map
[..]-[..]-[..]-[..]-[..] - ./static/chunks/575-bb7d7e0e6de8d623.js.map
Ignored: The following source files already have debug ids
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/_fixtures/inject/server/dummy_hosted.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading