Skip to content

Fix decoding source map with VS Code internal files#2987

Merged
koesie10 merged 2 commits intomainfrom
koesie10/source-map-fix
Oct 17, 2023
Merged

Fix decoding source map with VS Code internal files#2987
koesie10 merged 2 commits intomainfrom
koesie10/source-map-fix

Conversation

@koesie10
Copy link
Copy Markdown
Member

@koesie10 koesie10 commented Oct 17, 2023

This makes it possible to decode source maps containing references to code that is not part of the extension. If it finds any such references, it will simply not decode the source map and use the original stack trace instead.

Example stack trace:

Unhandled error: Error: Illegal argument: character must be non-negative
    at w (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:10:1101)
    at new s (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:121:4419)
    at new o (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:121:5899)
    at asRange (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/out/extension.js:55430:24)
    at asDiagnostic (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/out/extension.js:55364:66)
    at convertBatch (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/out/extension.js:54258:23)
    at Object.map2 [as map] (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/out/extension.js:54266:19)
    at Object.asDiagnostics (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/out/extension.js:55354:22)
    at CodeQLLanguageClient.workDiagnosticQueue (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/out/extension.js:63122:19)
    at Immediate.<anonymous> (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/out/extension.js:63107:16)
    at processImmediate (node:internal/timers:476:21)

Decoded:

Unhandled error: Error: Illegal argument: character must be non-negative
    at w (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:10:1101)
    at new s (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:121:4419)
    at new o (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:121:5899)
    at asRange (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/node_modules/vscode-languageclient/lib/common/protocolConverter.js:140:23)
    at asDiagnostic (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/node_modules/vscode-languageclient/lib/common/protocolConverter.js:70:65)
    at convertBatch (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/node_modules/vscode-languageclient/lib/common/utils/async.js:193:24)
    at Object.map2 [as map] (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/node_modules/vscode-languageclient/lib/common/utils/async.js:202:16)
    at Object.asDiagnostics (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/node_modules/vscode-languageclient/lib/common/protocolConverter.js:60:21)
    at CodeQLLanguageClient.workDiagnosticQueue (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/node_modules/vscode-languageclient/lib/common/client.js:1012:18)
    at Immediate.<anonymous> (/Users/user/.vscode/extensions/github.vscode-codeql-1.9.2/node_modules/vscode-languageclient/lib/common/client.js:997:84)
    at processImmediate (node:internal/timers:476:21)

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

This makes it possible to decode source maps containing references to
code that is not part of the extension. If it finds any such references,
it will simply not decode the source map and use the original stack
trace instead.
@koesie10 koesie10 requested a review from a team as a code owner October 17, 2023 11:37
Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

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

Seems reasonable!

}
}

const sourceMap = rawSourceMaps.get(file) as RawSourceMap | null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we need the casting.

Suggested change
const sourceMap = rawSourceMaps.get(file) as RawSourceMap | null;
const sourceMap = rawSourceMaps.get(file);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks! I think this will change the type slightly (since sourceMap can then also be undefined), but that shouldn't matter.

@koesie10 koesie10 enabled auto-merge October 17, 2023 13:24
@koesie10 koesie10 merged commit a7f8019 into main Oct 17, 2023
@koesie10 koesie10 deleted the koesie10/source-map-fix branch October 17, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants