Report unhandled errors, but only those that are from our extension #2125
Conversation
| // The stack trace gets redacted before being sent as telemetry, but at | ||
| // this point in the code we have the full unredacted information. | ||
| const isFromThisExtension = getErrorStack(error).includes( | ||
| "/extensions/ql-vscode/", | ||
| ); |
There was a problem hiding this comment.
Have you tested this using a packaged extension (VSIX)? I believe the stack trace in the packaged extension looks something like shown in the source map script:
Error: Failed to find CodeQL distribution.
at CodeQLCliServer.getCodeQlPath (/Users/user/.vscode/extensions/github.vscode-codeql-1.7.8/out/extension.js:131164:13)
at CodeQLCliServer.launchProcess (/Users/user/.vscode/extensions/github.vscode-codeql-1.7.8/out/extension.js:131169:24)
at CodeQLCliServer.runCodeQlCliInternal (/Users/user/.vscode/extensions/github.vscode-codeql-1.7.8/out/extension.js:131194:24)
at CodeQLCliServer.runJsonCodeQlCliCommand (/Users/user/.vscode/extensions/github.vscode-codeql-1.7.8/out/extension.js:131330:20)
at CodeQLCliServer.resolveRam (/Users/user/.vscode/extensions/github.vscode-codeql-1.7.8/out/extension.js:131455:12)
at QueryServerClient2.startQueryServerImpl (/Users/user/.vscode/extensions/github.vscode-codeql-1.7.8/out/extension.js:138618:21)
This does not include /extensions/ql-vscode because when the extension is packaged, that part of the path is not included.
There was a problem hiding this comment.
Good point, I was testing with the development host rather than a packaged extension. If the paths do always look like the ones you've included here, that looks very promising for a robust way to detect extension code.
There was a problem hiding this comment.
I've rewritten this check to use extension.extensionPath instead, which will be much more accurate. It'll also avoid issues on windows because I accidentally used forward slashes in the previous implementation.
There was a problem hiding this comment.
This time I tested using a vsix that I build and installed.
robertbrignull
force-pushed
the
robertbrignull/error_listener
branch
from
a6c4fdd to
9386817
Compare
|
Thanks @koesie10 for the earlier review. It's ready for re-review now. |
| const isFromThisExtension = | ||
| extension && getErrorStack(error).includes(extension.extensionPath); | ||
|
|
There was a problem hiding this comment.
Nice...so this will handle the local development case as well as bundled extensions?
There was a problem hiding this comment.
It does still work for local development, because the extension path still matches the location of the installed extension. In my case it's /Users/robertbrignull/github/vscode-codeql/extensions/ql-vscode.
This is a second attempt at #2076. That PR worked a bit too well, in that it was picking up errors from other extensions and presenting them to the user. The fix for this that we discussed was to check the stack trace to see if it's from our extension or somewhere else.
I've implemented this by checking for
/extensions/ql-vscode/. I couldn't think of a better string to check for. We might actually have trouble in the future if we move the extension to the top level of the repo. I'm not sure if that means we should look for a different approach now, or save that problem for if/when we do move the code in the repo.I've tested this using https://github.com/robertbrignull/error-extension. I created a new extension (from a template) that provides commands that intentionally throw errors and reject promises. Using this I was able to reproduce the existing bad behaviour and then also show that we now don't report errors from other extensions.
Checklist
ready-for-doc-reviewlabel there.