vscode-js-debug icon indicating copy to clipboard operation
vscode-js-debug copied to clipboard

SourceRoot not Considered as Absolute Path

Open FrankEssenberger opened this issue 3 years ago • 5 comments

Describe the bug

Here is a small sample repo.

We try to use the sourceRoot to add the sources in a remote location so that we do not need to ship them. However, the mapping:

{"version":3,"file":"string.js","sourceRoot":"http:/raw.githubusercontent.com/SAP/cloud-sdk-js/main/packages/util/src/","sources":["string.ts"],"names":[],"mappings":";......"}

leads to an error because the CWD is added in front of the source route.

To Reproduce

  • Checkout repo.
  • Run install
  • Execute debug-test.mjs via launch.json
  • Step into method -> see the Error

Could not load source '/Users/XXX/source-map-test/node_modules/@sap-cloud-sdk/util/dist/http:/raw.githubusercontent.com/SAP/cloud-sdk-js/main/packages/util/src/string.ts': Unable to retrieve source content.

VS Code Version: 1.72.0

FrankEssenberger avatar Oct 10 '22 13:10 FrankEssenberger

We do not currently support loading remote sources from maps in Node.

However, note that you will also need changes in how you publish sourcemaps. The source root http:/raw.githubusercontent.com/SAP/... is not an absolute path, nor is it a URI. You will want to ensure it's a valid URI, starting with https:// or http:/ if we are to load it.

connor4312 avatar Oct 10 '22 15:10 connor4312

@connor4312 Thanks a lot for the quick reply. I guess the / was taken by the join() method of the path module. I made a quick adjustment to the code so that the double slash is there. http works for me but they do some redirect and this does not work for all http clients so https is better - also adjusted this.. Looks like this is also used internally because even when the source root contains the double slash in the map file in the resulting path printed in the error message the double slash is gone.

Anyhow if it is not supported it will not work. Since you said do not currently support is there a hope that you do it in the future? If so do you have a rough timeline?

FrankEssenberger avatar Oct 11 '22 07:10 FrankEssenberger

I don't have a timeline at the moment, but this may be easy to add. Code pointer:

https://github.com/microsoft/vscode-js-debug/blob/a5995596511282127cf31932449d89c22588ee24/src/adapter/sources.ts#L1136-L1140

Currently fileUrl is only a file URL, but if we could do an extra check to see if the resolvedUrl is a correctly formed URL, and if so pass that as a ~fileUrl~ contentUrl instead

connor4312 avatar Oct 11 '22 15:10 connor4312

I guess there is already a fetchHtt but it is not used because the absolutePath is fooled since the URL is not starting with a /.

This seems simple enough even for me :-) to fix and add a test. Perhaps I do that on our focus friday reserved for learning. DO I simply fork the repo and create a PR?

FrankEssenberger avatar Oct 12 '22 08:10 FrankEssenberger

Indeed 🙂 You can check out the contributing guide for setup: https://github.com/microsoft/vscode-js-debug/blob/main/CONTRIBUTING.md

connor4312 avatar Oct 12 '22 15:10 connor4312