Prompt user to Open a Folder or Workspace

[ucheNkadiCode]

Users commonly try to debug, build, scaffold or start a container with our Docker Tools when a folder or workspace is not currently open. It is important to note there shouldn't be a way to get into a debug path without having a folder open. This error should be repro'd.

Possible Scenario: The user selects a single python file and opens with VS Code. The user would be able to run this python script with a Python launch configuration. If the user tries to scaffold a dockerfile or build a docker image, they'll receive a prompt in the bottom right with text along the lines of "To build Docker files you must first open a folder or workspace in VS Code" because the user is not a part of a folder or workspace.

Issue: We are not providing a helpful pathway to correcting the problem. Instead we simply give them this popup:

Instead of giving the user the option to report an issue (which is used way too often for the magnitude of issue), I suggest we instead add a button with the same functionality as the one in VS Code.

This error affects the most customers by far. Over 3 thousand users have seen this error almost 5 thousand times in the past 60 days.

[bwateratmsft]

@ejizba I was discussing this yesterday with @ucheNkadiCode, and I think potentially the easiest and best way to solve it would be to define a new special error type in vscode-azureextensionui (e.g. NoWorkspaceError), and have specific handling to catch it that would not show 'Report Issue', but instead a button to open a workspace/folder. Then, in all the (many) places we throw errors about this, we can just throw NoWorkspaceError and avoid inconsistency and complexity.

[ejizba]

Sounds good

[bwateratmsft]

@ejizba There's a couple approaches that seem viable to me, relating to IParsedError. Right now it has a boolean isUserCancelledError.

1. We could add a new boolean for this new error
2. We could replace it with an enum (which would be a "breaking" change as several extensions reference that property).
3. There might be some way to make it a subclass of UserCancelledError, while still offering the popup with the "Open workspace" button.

Any thoughts?

[ejizba]

I can see us expanding this pattern to other buttons (e.g. "Reload Window" or "Learn More"), so it would be great to have a more generic/scalable solution. First idea that comes to mind is maybe NoWorkspaceError extends a base class like CustomButtonError. That way callWithTelemetryAndErrorHandling and IParsedError would have to know about CustomButtonError, but not each individual type of error/button.

Other thought is just that I'm fine with breaking changes in the shared packages.

[bwateratmsft]

I like that idea. Should we limit the resultant action to text for a button and a command an action to execute, or undefined to show nothing (i.e. in the UserCancelledError case)?

e.g. something like

interface ErrorAction {
    buttonText: string;
    action: () => Promise<void>;
}

class CustomButtonError extends Error {
    // ...

    public get action(): ErrorAction | undefined {
        // ...
    }
}

[ejizba]

If we're trying to combine this with UserCancelledError, I might go for something even more generic than CustomButtonError. First, I'd probably add buttons to IActionContext:

export interface IErrorHandlingContext {
    ...

    /**
     * Defaults to `false`. If true, does not show the "Report Issue" button in the error notification.
     * @deprecated Set `buttons` to `[]` instead
     */
    suppressReportIssue?: boolean;

    /**
     * Buttons to include in error notification. Defaults to an array containing the "Report an Issue" button
     */
    buttons: ErrorAction[];

    ...
}

And then I'd do something like this for the errors:

export abstract class AzExtHandlerError extends Error {
    public abstract handleError(context: types.IErrorHandlerContext): void;
}

export class NoWorkspaceError extends AzExtHandlerError {
    constructor() {
        super('todo message');
    }

    public handleError(context: types.IErrorHandlerContext): void {
        context.errorHandling.buttons = [
            // Open Workspace button
        ];
    }
}

export class UserCancelledError extends AzExtHandlerError {
    constructor() {
        super(localize('userCancelledError', 'Operation cancelled.'));
    }

    public handleError(context: types.IErrorHandlerContext): void {
        context.telemetry.properties.result = 'Canceled';
        // this would basically tell `callWithTelemetryAndErrorHandling` to pretend there was no error
        context.error = undefined;
    }
}

I'm assuming you're going to do this work since you brought it up in the issue first, but let me know and I'd be happy to do it myself. It relates to some stuff I've had kicking around in my head for a while and has expanded in scope from what I think you were originally after

[bwateratmsft]

I'd be happy to work on it, we're probably going to do it in our 1.10.0 release which will go out sometime in January. So we've got plenty of time.

[ejizba]

Well we have a release in the next week or two and I was hoping to get some of this work in for that. Not sure if you're familiar, but we have a stupid "Entry not found in cache" error that happens a ton of the time, but can be fixed if the user reloads VS Code. I was hoping to improve the message and add a "Reload Window" button, but it's a bit complicated because it can happen for like any Azure call (hence why I want some of the above work)

[bwateratmsft]

That makes sense. Anything I can do to help, in that case?

[ejizba]

Submitted a PR specific to the "Entry not found in cache." case: microsoft/vscode-azuretools#825

I needed to add buttons to IErrorHandlingContext, but otherwise didn't need to create/modify any of the error classes, which you're welcome to do.

[bwateratmsft]

This is now available in Docker extension version 1.9.1.