Proposal: Progress Events

[chuckries]

We propose that adapters can send progress events to a fronted, in order to display progress related UI to a user in the case of long running tasks on the adapter side. Specific examples of long running tasks that an adapter may want to report progress for include symbol loading and expression evaluation.

Progress events may be generic (unrelated to a specific request) or specific (related directly to a request). This allows frontends to organize incoming progress events and directly show progress UI or defer progress information to API related to an original request.

• An example of a generic progress event is automatic symbol loading progress, as this can happen any time a target process is running
• An example of a specific progress event is loading symbols due to a LoadSymbols request, as this action is taken as a direct cause of a request.

Progress events are grouped using a unique id. This allows adapters to report progress for a long running task and frontends to group progress events together.

Progress events can be cancellable and are cancelled by extending the existing CancelRequest.

/** Event message for 'progress' event type
 *      The event indicates that a long running task has progress to report that can be displayed to the user.
 *      Multiple progress events with the same id represent a single logical progrses operation that can be updated.
 *      Sequence of events is as follows (for a given id):
 *      - adapter sends 1 progress event of category 'new'
 *      - adapter sends 0 or more progress events of category 'update'
 *      - adapter sends 1 progress event of category complete
 *      After this the adapter cannot send any more progress events with the same id
 * 
 *      Progress events may be generic (not related to a specific request) or be specific to a request id 
 * 
 *      Progress events may be cancellable. To cancel an operation that is reporting progress, a Cancel request is sent specifying the progressId to cancel.
 */
export interface ProgressEvent extends Event {
    body: {
        /**
         * The unique id for this progress operation. 
         */
        progressId: number;

        /** The request id that this progress event is related to, if any */
        requestId?: number;

        /** The progress category.
         *      Values: 'new', 'update', 'complete'
         */
        category: string;

        /** If cancellable is true, the task reporting progress may be canceled with a CancelRequest. If not present, assumed false */
        cancellable?: boolean;

        /** If 'category' is 'complete', this field may be present to report whether the completed progress task was cancelled. If not present, assumed false */
        cancelled?: boolean;

        /** A description of the progress event to display to the user; used to explain the long running task.  
         *      This field should be present when category is 'new' or 'update'
        */
        description?: string;

        /** The current step of the progress task, 0 indexed. This field combined with 'totalSteps' can be used to report a percentage progress for the long running task.
         *      If both 'currentStep' and 'totalSteps' are present and totalSteps is not 0, these fields represent a percentage progress where percent is 'currentStep' / 'totalSteps'
         *      If both 'currentStep' and 'totalSteps' are present and are 0, the progress is 'indeterminate', i.e. an indeterminate progress bar may be shown
         *      If either or both 'currentStep and 'totalSteps' are not present, their is no numeric progress to report and only the 'description' should be used.
         */
        currentStep?: number;

        /** The total number of steps this long running task will require. This field combined with 'currentStep' can be used to report a percentage progress for the long running task. 
         * 
        */
        totalSteps?: number;
    }
}

export interface CancelRequest extends Request {
    ...

    /** The progressId of a cancellable ProgressEvent */
    progressId?: number;
}

CC: @andrewcrawley @gregg-miskelly

[gregg-miskelly]

A few small suggestions:

1. Rather than having a single description string, I wonder if you really want two (or maybe even three) -- one that would be appropriate for a title bar (for debugger UIs that choose to display this in a wait dialog) and one that would be appropriate for the body. Debugger UIs that don't display this in a separate window could combine the two together in another way. Looks like IVsThreadedWaitDialog4.StartWaitDialogWithCallback actually uses a third string -- it divides the body text between a description and a progress text. I don't know if the dialog displays these any differently?
2. Seems like the description strings shouldn't necessarily be required for an update -- the UI could continue to show the same text
3. Feels like omitting currentStep/totalSteps and passing 0 should be equivalent.

[andrewcrawley]

Re: Gregg's comments:

1. I think a single string is OK, especially for a UI like VS Code that doesn't do a lot of modal dialogs. We could extend this with VS-specific props if necessary, but in my experience, wait dialogs in VS usually just have "Microsoft Visual Studio" as their title, so I don't think this functionality is heavily used.
2. My initial assumption was that an update message would only update the fields for which values were provided, so the description would remain the same if it wasn't specified. However, this would make it kind of weird if you wanted an update to remove the description - I guess you'd have to pass an empty string instead of NULL?
3. I think differentiating between "0" and "unspecified" is necessary to support both showing an indeterminate progress bar and no progress bar at all. We could also introduce a new field to indicate the progress type (determinate / indeterminate / none) rather than inferring from the provided progress values.

[gregg-miskelly]

1. For the first planned progress feature we have, I think we would want to set the title when the debugger UI is VS. We definitely could do the 'extra description strings' as VS extensions instead of parts of the main protocol. But I could imagine using multiple strings in other IDEs too. For example, if I understand the VS Code progress API correctly (see withProgress on this page), I believe they use both a title string (set through ProgressOptions), and a progress description string (set through the increment callback). Apologies in advance if I am misunderstanding how that API works.
2. Agreed
3. Got it. To me it feels unnecessary to allow the debug adapter to choose between an 'indeterminate' progress bar (aka marquee) and no progress bar. But sure, if we want to allow it I guess it is easy enough for VS.

[weinand]

@chuckries thanks for this great proposal (and sorry for the long time it took me to respond to it).

My comments:

• In order to prevent debug adapters from sending unnecessary progress events to old clients that don't understand them, a client capability "supportsProgressEvents" should be introduced.
• using a single event with a "category" discriminator and different semantics for the shared properties appeared to be difficult to understand. I've created three progressEvents instead.
• I agree with Gregg that splitting the single "description" into a "title" and a "message" is a good thing. This is in line with the progress functionality defined in the LSP spec.
• to simplify the step-based progress reporting I'm proposing to use the percentage-based approach used in LSP.
• I've omitted the "cancelled" property for now because I'm not sure that I like it here (cancelation is done via the "cancel" request, so returning cancellation status here again seems to be redundant).

Based on these comments, I've created a variant of your proposal.

First the client capability (the client frontend signals to the DA that progress reporting is supported):

/** Arguments for 'initialize' request. */
export interface InitializeRequestArguments {
	// ...

	/** Client supports progress reporting. */
	supportsProgressReporting?: boolean;
}

And now the three events:

/** Event message for 'progressStart' event type.
	The event signals that a long running operation is about to start and
	provides additional information for the client to set up a corresponding progress and cancellation UI.
	The client is free to delay the showing of the UI in order to reduce flicker.
*/
export interface ProgressStartEvent extends Event {
	// event: 'progressStart';
	body: {
		/** An ID that must be used in subsequent 'progressUpdate' and 'progressEnd' events to make them refer to the same progress reporting. */
		progressId: string;
		/** Mandatory (short) title of the progress reporting. Shown in the UI to describe the long running operation. */
		title: string;
		/** The request ID that this progress report is related to. If specified a debug adapter is expected to emit
			progress events for the long running request until the request has been either completed or cancelled.
			If the request ID is omitted, the progress report is assumed to be related to some general activity of the debug adapter.
		*/
		requestId?: number;
		/** If true, the request that reports progress may be canceled with a 'cancel' request.
			So this property basically controls whether the client should use UX that supports cancellation.
			Clients that don't support cancellation are allowed to ignore the setting.
		*/
		cancellable?: boolean;
		/** Optional, more detailed progress message. */
		message?: string;
		/** Optional progress percentage to display (value range: 0 to 100). If omitted no percentage will be shown. */
		percentage?: number;
	};
}

/** Event message for 'progressUpdate' event type.
	The event signals that the progress reporting needs to updated with a new message and/or percentage.
	The client does not have to update the UI immediately, but the clients needs to keep track of the message and/or percentage values.
*/
export interface ProgressUpdateEvent extends Event {
	// event: 'progressUpdate';
	body: {
		/** The ID that was introduced in the initial 'progressStart' event. */
		progressId: string;
		/** Optional, more detailed progress message. If omitted, the previous message (if any) is used. */
		message?: string;
		/** Optional progress percentage to display (value range: 0 to 100). If omitted no percentage will be shown. */
		percentage?: number;
	};
}

/** Event message for 'progressEnd' event type.
	The event signals the end of the progress reporting with an optional final message.
*/
export interface ProgressEndEvent extends Event {
	// event: 'progressEnd';
	body: {
		/** The ID that was introduced in the initial 'ProgressStartEvent'. */
		progressId: string;
		/** Optional, more detailed progress message. If omitted, the previous message (if any) is used. */
		message?: string;
	};
}

/cc @gregg-miskelly @andrewcrawley @pieandcakes @DanTup @DonJayamanne @hbenl

@isidorn please provide feedback from the client implementation perspective

[DanTup]

This sounds great to me! In Dart we already have a number of custom events sent from the debugger back to the extension to do just this, but ultimately I'd like the DA to be usable outside of Code, so the more VS Code-specific bits that move to standards, the better :-)

I presume it's ok for progressId to be reused (eg. once a progress completed, reusing the same ID again for a new progress later in the same session is fine)?

[connor4312]

This is perfect for pretty printing in js-debug as well, as formatting large minified scripts can take several seconds. That latest proposal would work well for us.

[DonJayamanne]

If I'm not mistaken this new API allows the debugger to add progress messages. I.e. this isn't about just cancellable progress messages anymore, it's more generic.

[chuckries]

@weinand Thank you for the feedback! I appreciate your changes to the proposal. I am currently implementing this in the C# Extension for VS Code Debug Adapter. I will incorporate your changes into that implementation and I will report back as I discover issues and/or have new ideas/insights.

[isidorn]

@weinand from the client implementation side this proposal makes perfect sense.
However please note that VS Code does not support to show progress perecentage-wise. We always show an infinite progress bar. So the inital implementation on the VS Code side would not respect the percentage parameter. However if in the future we support percentage progress in vscode in general we could easily adopt this for debug.

Also note that we can show progress in various places, for example debug viewlet or general workbench progress. I think we would noeed to decide on the location, or the event would have to specify this somehow.
And also we can not support two progresses in one location at the moment. So for example two workbench progress would be shown as running as long as one of them is still running.

[chuckries]

@weinand in case I wasn't clear, I suggest we don't commit this to the official protocol until I am done prototyping in the C# extension. Visual Studio's VS Code Debug Adapter Host is our target frontend for this.

[connor4312]

Yea, a way to provide priority or location for the progress may be useful. For instance, using the js-debug pretty printing example, in most cases it will take only a second or two so showing a notification would be a bit noisy, a loading indicator atop the debug view may be more appropriate. But for longer running tasks--like a hypothetical world we can offer to install Edge for the user if they don't have it already available--a notification would be appropriate. Maybe a property like display: 'quiet' | 'verbose' could be used to indicate that.

[weinand]

Thanks a lot for the feedback.

Here are some replies:

• @DanTup yes, ID can be reused. I've added this sentence to the ID's comment: "An ID can be reused after a 'progressEnd' event has been issued for it".
• @DonJayamanne yes, this proposal is about "reporting progress for DAP requests". The cancelability of requests has been addressed before in the cancel request. Progress events participate in "cancelability" through the cancellable and requestId properties. To make the purpose of the proposal clearer, I will remove the word "Cancellable" from the issue's title.
• @isidorn yes, I understand VS Code's current limitations of progress reporting but none of them would block adoption by VS Code.
• @isidorn "location of progress": in general we do not try to "drive" the UI directly from the debug adapter but instead we leave it up to the client (frontend) to create and manage the UI and define and implement the UI polices. The debug adapter can only provide "hints" so that the UI can make correct decisions. For showing progress this means that we should start using the proposed DAP in VS Code's generic debugger UI. VS Code should decide to pick the best "progress location" based on its knowledge about the used DAP request and the context where the request is being used. Only if we face difficulties with this approach, we should consider to introduce "hints".
• @connor4312 I see your point about the noisyness of a nervous progress UI. But instead of controlling this from the backend (and relying on debug adapters to do the right thing), I think we should make VS Code's progress UI smart enough to only show progress if a long running operation takes longer than some configurable amount of time.

[chuckries]

@weinand One part of my original proposal that I do not see in your revisions is the ability to cancel based on progressId (as opposed to the protocol sequence id). This is important to us for cancelling "generic" progress events that are not tied to a specific request. Specifically we need this for cancelling automatic symbol loading that occurs while the target process is running.

My original proposal was to extend the CancelRequest to add an optional progressId field in order to cancel an outstanding progress operation. My idea would look something like this in protocol:

<- Event: ProgressStart seq: 1, progressId: 1
<- Event ProgressUpdate seq: 2, progressId: 1
<- Event ProgressUdpate seq: 3, progressId: 1
<- Event ProgressUpdate seq: 4, progressId: 1
-> Request: Cancel progressId: 1

Did you have another idea for cancelling generic progress? Or can we amend your proposal to include the extension to CancelRequest?

[weinand]

@chuckries yes, that was an oversight on my part.

I think we can go with your proposal to add an additional attribute progressId on the cancel request (despite the fact that initially I thought that's a bad idea).

Here is why:
I was under the assumption that the cancel request already had a mandatory requestId argument. So adding an alternative optional progressId argument would result in the awkward situation that a caller who wants to cancel a progress, would have to pass some bogus value for the mandatory requestId argument.

But then I checked the spec and noticed that there is actually a misspelling that prevents the requestId attribute from becoming mandatory.

Due to this we are shipping the spec with an optional requestId argument (and the comment does not mention that the argument is mandatory either).

Given these lucky circumstances I propose the following:

• we are leaving the existing requestId optional but explain this fact in the description.
• we are introducing a new optional progressId attribute of type string.
• @DanTup I will remove the newly introduced clarification that "An ID can be reused after a 'progressEnd' event has been issued for it" because the progressId can now be used in a separate cancel request. By not reusing progress Ids we can eliminate the risk that the client (frontend) cancels the wrong progress due to race conditions.

[weinand]

This commit shows the (TypeScript) changes for the (experimental) progress support.

[weinand]

I've released the DAP progress feature (and VS Code Insiders provides the UI for it; see microsoft/vscode#92253).

[gjsjohnmurray]

Thanks @weinand

This para of the doc seems to have a redundant line break:

[weinand]

@gjsjohnmurray thanks for letting me know. There are more than one of these issue in the doc. I'll have to fix the Markdown generator...

[chuckries]

@weinand I missed this initially but what was the purpose of making progressId a string as opposed to an int?

[weinand]

We've started to represent IDs as strings because it makes it easier to use UUIDs.