microsoft / vscode-js-debug

A DAP-compatible JavaScript debugger. Used in VS Code, VS, + more
MIT License
1.66k stars 279 forks source link

resolveSourceMapLocations matching is broken #2091

Open jasonwilliams opened 3 days ago

jasonwilliams commented 3 days ago

Describe the bug I try to use the resolveSourceMapLocations feature and it doesn't seem to match most of the time. I've been investigating what has been going on.

To Reproduce Steps to reproduce the behavior:

This is difficult to reproduce as I use a bespoke module loader, not the main NodeJS one but a more internal one which resolves paths using a different schema. I'll give you an example though:

my-module-1.268.1:index.js would resolve to a folder called my-module-1.268.1 with the file index.js inside. So it could end up being a URL structured like https://sourcemap-server.project.com/my-module-1.268.1/index.js. Most of the time these don't work so I get a lot of these errors coming from the debugger:

Could not read source map for module-name-1.2:module/file.js: Unexpected 503 response from module-name:module-1.2/file.js: Unsupported protocol "module-name-1.2:"

I don't want VSCode's JS debugger trying to fetch source maps for these strings, they're not needed, so I set this as a resolve pattern:

"!*[0-9]+.[0-9]+.[0-9]+:*"

This should match any string with a version like the above and block VSCode from trying to fetch the source map.

Problem... These files were still being fetched, so I took a look here

Issue 1 (absolute paths are added before matching)

The first issue is the shouldResolveSourceMap function doesn't just test against the string you give it, it also adds a rebased version (absolute path) and matches against that too. so

['my-module-1.268.1:index.js']

Becomes

[
   'my-module-1.268.1:index.js',
   'C:\\Users\\User\\AppData\\Local\\Programs\\Microsoft VS Code\\my-module-1.268.1:index.js'
]

This means my matcher from before matches the first entry, but fails against the second (because I only have a single * which doesn't match against the directories before it. Without debugging the debugger this would have been impossible to know.

I can fix this by adjusting the above matcher to be !**/*[0-9]+.[0-9]+.[0-9]+:* instead, but this now fails on URLS that don't have a directory preceeding it. So I need both:

Not to mention directories after this pattern so I also need a third...

Issue 2 (Globs are unreliable)

The second issue is some patterns match and some don't for no reason, for instance, the third glob (above) just doesn't work. Here is a link to a playground:

https://www.digitalocean.com/community/tools/glob?comments=true&glob=%2A%5B0-9%5D%2B.%5B0-9%5D%2B.%5B0-9%5D%2B%3A%2A%2A%2F%2A.js&matches=false&tests=my-module-1.130.0%3Apath%2Fto%2Fscript.js

Some Solutions

I wonder if we can use regex instead here? And have a disallow list which just does a str.includes(regExp), this would drastically allow me to cut down on the number of matches I would need. Even if there's a rebased directory like above my string match would still catch it, it would also catch the string if there's a directory after, so I can use 1 regex in place of 3 (or more) globs.

Another solution (if you don't want to change matching) could be to ignore unsupported protocols. Take the URL of the path then check the schema, if it is not in the default list of ["http", "https", "file"] ignore it and noop. If you add this logic to shouldResolveSourceMap then the function can just return false for anything that is not recognised, maybe the user can override the schemas in the config, this should also help too

VS Code Version: 1.93.1

connor4312 commented 16 hours ago

I made a change to avoid issue (1) that you mentioned.

The paths and patterns passed in this and other config options are not regexes, but globs that we match using micromatch. In your case it seems like micromatch wants an additional globstar, for some reason (*[0-9]+.[0-9]+.[0-9]+:**/**/* works.) I'm not sure that we'd want to reject unknown protocols, since there are some cases where good sources can be served from them, such as in Electron apps.

However, you could match the protocol as an include or exclude and get a similar effect, e.g. foo://**/* or !foo://**/*