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

fix(es): Improve sourcemap url error messages. #9422

Merged
merged 4 commits into from
Aug 14, 2024

Conversation

simonbuchan
Copy link
Contributor

@simonbuchan simonbuchan commented Aug 12, 2024

Description:

Parse based on the provided prefix: inline urls must start with "data:", so only trying one will ensure the correct and more helpful error message will be shown.

Currently, any error while trying to load a sourcemap file will give the unhelpful error:

  ERROR  failed to read input source map: failed to parse inline source map url
index.js.map

 Caused by:
     relative URL without a base
     at <snip>/swc-<ver>/src/lib.rs:386

Further, for the common "missing file" error, give a better message that hopefully describes the two paths tried, rather than increasing confusion by talking about a .js.map.map file if it was correct.

Now it will show, for example:

  ERROR  failed to read input source map: failed to find input source map file "index.js.map" in "file:///D:/github/skilitics/billing/api-client/lib/services/index.js" file as either "file:///D:/github/skilitics/billing/api-client/lib/services\\index.js.map" or with appended .map
    at D:\github\swc-project\swc\crates\swc\src\lib.rs:400

(This example appears to be an issue in swc-node, where it is passing a file URL which is treated as a path, and therefore never exists, I'm working on a PR for there too)

There are more improvements that could make this code clearer and more reliable, but this should resolve many of the confused users (including me!)

Related issue:

See (maybe can be considered to fix): #8944, #8910

PR in swc-node to fix the originating issue

Parse based on the provided prefix: inline urls must
start with "data:", so only trying one will ensure the
correct and more helpful error message will be shown.

Currently, any error while trying to load a sourcemap file will
give the unhelpful error:

>   ERROR  failed to read input source map: failed to parse inline source map url
>  index.js.map
>
>  Caused by:
>      relative URL without a base
>      at <snip>/swc-0.273.28/src/lib.rs:386

Further, for the common "missing file" error, give a better
message that hopefully describes the two paths tried, rather
than increasing confusion by talking about a `.js.map.map`
file if it was correct.

There are more improvements that could make this code
clearer and more reliable, but this should resolve many of
the confused users (including me!)

See (maybe can be considered to fix): swc-project#8944, swc-project#8910
@simonbuchan simonbuchan requested a review from a team as a code owner August 12, 2024 07:31
Copy link

changeset-bot bot commented Aug 12, 2024

🦋 Changeset detected

Latest commit: 5a43d86

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

simonbuchan added a commit to simonbuchan/swc-node that referenced this pull request Aug 12, 2024
compile() expects a filename, not a url: it passes the
value directly to the bindings module, which immediately
parses it as a `Path` unconditionally.

This causes input file source maps to fail to load, as it
attempts to find the URL as if it were a file on disk.
(see swc-project/swc#9422)

This is a targeted fix which hopefully minimizes the chance of things breaking.
Copy link

codspeed-hq bot commented Aug 12, 2024

CodSpeed Performance Report

Merging #9422 will not alter performance

Comparing simonbuchan:improve-sourcemap-error (2316195) with main (55f7268)

Summary

✅ 178 untouched benchmarks

Brooooooklyn pushed a commit to swc-project/swc-node that referenced this pull request Aug 13, 2024
compile() expects a filename, not a url: it passes the
value directly to the bindings module, which immediately
parses it as a `Path` unconditionally.

This causes input file source maps to fail to load, as it
attempts to find the URL as if it were a file on disk.
(see swc-project/swc#9422)

This is a targeted fix which hopefully minimizes the chance of things breaking.
@simonbuchan
Copy link
Contributor Author

Should I be concerned about those performance changes? I haven't touched anything that should affect them, so I assume they are noise, but the messaging on the bot implies it's pretty confident...

@kdy1
Copy link
Member

kdy1 commented Aug 13, 2024

Nope, it's quite flaky.

@kdy1 kdy1 requested a review from a team as a code owner August 14, 2024 06:36
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kdy1 kdy1 added this to the Planned milestone Aug 14, 2024
@kdy1 kdy1 enabled auto-merge (squash) August 14, 2024 06:37
@kdy1 kdy1 changed the title fix(common): Better sourcemap url error messages. fix(es): Improve sourcemap url error messages. Aug 14, 2024
@kdy1 kdy1 merged commit 230d1d9 into swc-project:main Aug 14, 2024
154 checks passed
@kdy1 kdy1 modified the milestones: Planned, v1.7.11 Aug 14, 2024
@simonbuchan simonbuchan deleted the improve-sourcemap-error branch August 29, 2024 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants