telemetry.properties.result reflects "Failed" even if the operation was canceled and UserCancledError was thrown

[shsuman]

Here is one of our sample telemetry event being captured in Kusto:

{
  "result": "Failed",
  "isactivationevent": "false",
  "stack": "n.<anonymous> extension.js:2:1981387\ns extension.js:2:1980640",
  "contextvalue": "Uri",
  "error": "o",
  "errormessage": "Operation cancelled.",
  "laststep": "ApplicationShell.showInformationMessage",
}

It is clear that the operation was canceled from the error message and the laststep property but the result property says "Failed" instead of "Canceled"

My best guess is that its because of this piece of code here
utils\src\parseError.ts at parseError() function

if (error.constructor !== Object) {
      errorType = error.constructor.name;
}

A constructor.name comparison has been relied upon to check whether its a UserCanceledError or not. This doesn't work with webpack as the constructor names are minified.

[alexweininger]

Thanks for reporting the issue. We have also configured Webpack to not mangle function names as you have done in your fix.

https://github.com/microsoft/vscode-azuretools/blob/main/dev/src/webpack/getDefaultWebpackConfig.ts#L102-L103

Without configuring Webpack properly, I'm not sure how to fix this.

[shsuman]

I am a little nit confused here. You are not exporting minified code, are you ? I am looking inside the node_modules/@microsoft/vscode-azext-utils folder and its all JS files.

Well, as for a fix, why not use instanceof operator ? :)

[alexweininger]

I am a little nit confused here. You are not exporting minified code, are you ? I am looking inside the node_modules/@microsoft/vscode-azext-utils folder and its all JS files.

We are not exporting minified code.

Well, as for a fix, why not use instanceof operator ? :)

Unfortunately, instanceof breaks when objects are passed between different instances of the utils package (ex: between extensions).

[shsuman]

I think if there is a limitation, then we will stick to the webpack configuration. You may close the issue now :)
Thanks for the all the information :)

[alexweininger]

Were you able to fix this issue by altering your webpack config?

[shsuman]

Yes. Here's the webpack changes that we made:

 optimization: {
        minimize: true,
        /**
         * **DO NOT REMOVE**
         * Workaround for https://github.com/Azure/ms-rest-nodeauth/issues/83
         */
        minimizer: [
            new TerserPlugin({
                terserOptions: {
                    keep_fnames: /AbortSignal|UserCancelledError/,
                    keep_classnames: /AbortSignal|UserCancelledError/
                }
            })
        ]
    },

[StephenWeatherford]

I had thought we'd placed a property on UserCancelledError to avoid this issue:

export class UserCancelledError extends Error {
  public get isUserCancelledError() { return true; }
...

Then clients can simply check for the existence of this property and name mangling won't matter.

[alexweininger]

I had thought we'd placed a property on UserCancelledError to avoid this issue:

export class UserCancelledError extends Error {
  public get isUserCancelledError() { return true; }
...

Then clients can simply check for the existence of this property and name mangling won't matter.

It doesn't look like we ever did that. And I agree we should do that so runtime behavior isn't impacted by bundling configuration.

[bwateratmsft]

It might be overkill for the use case, but we may also benefit from having another method like this, that checks the isUserCancelledError property on the input object:

function isUserCancelledError(foo: unknown): foo is UserCancelledError {
    // ...
}