Task view randomly doesn't work

[CsCherrYY]

Extension Name: vscode-gradle
Extension Version: 3.6.1
OS Version: Windows_NT x64 10.0.19043
VSCode version: 1.59.0-insider

Sometimes, when opening Gradle panel in the activity bar, Gradle Tasks view shows nothing

After refreshing, it works well.

There is another bug, might come from the same root cause, sometimes collapse all button is disabled by default

it also works after refreshing or other operations (such as run a gradle task, etc.)

[CsCherrYY]

After investigation and debugging, I think it's the root cause of this issue.

When resolving tasks, there is an async function is awaiting.

vscode-gradle/extension/src/views/gradleTasks/GradleTasksTreeDataProvider.ts

Line 120 in f5bf5f8

return await this.buildTreeItems();

When awaiting this promise, there is a refresh command might be triggered, which comes from

vscode-gradle/extension/src/Extension.ts

Line 185 in f5bf5f8

this.client.onDidConnect(() => this.refresh());

To describe it simply, a new refresh is requested during the get task process, which results in there is no items in the view. you can easily reproduce this bug if you quickly click the Gradle in the Activity Bar once it appeared.

@badsyntax I'm not very sure about what is this.client.onDidConnect(() => this.refresh()); for. I just try to comment it and this issue seems not to appear again and do not break other features. Could you please give some suggestion or comment on this? Is there any side effect if we just remove this?

[badsyntax]

@CsCherrYY I've been able to replicate this behaviour. Unfortunately it looks like a race condition but it's not clear why. (FYI I have not seen this behaviour in the past, perhaps this race condition is due to a recent change in vscode.)

The purpose of this.client.onDidConnect(() => this.refresh()); is to refresh the task last after the client has reconnected. A use case for this scenario is when the server gets killed unexpectedly, in which case the extension will prompt the user to restart the server, and the grpc client will re-connect, but the task list is not automatically updated, which is why we are explicitly calling refresh().
You can see this yourself by killing the Java server process (on macos/linux killall java)

This is the prompt the user will see:

It would good to understand why this bug occurs (it's really not clear to me) but a possible workaround is to only add this event handler after the server has stopped, possibly in this method:

vscode-gradle/extension/src/client/GradleClient.ts

Lines 71 to 73 in cd69c0c

	private handleServerStop = (): void => {
	this.close();
	};

(If we add it there we'll need to unsubscribe the event handler after it's called.)

I can't think of other use cases that require this.client.onDidConnect(() => this.refresh()); so I think it's safe to move that event handling to handleServerStop.

[badsyntax]

To clarify a bit more, if we don't want the task list updated after the server has restarted then I think it's safe to remove that line. There's also maybe a better way to handle this scenario.

[CsCherrYY]

@badsyntax Thanks a lot for the clarification! It seems a VSCode treeview issue and I just find microsoft/vscode#129019 (though it's about a start-as-closed view, but for other start-as-open views I can sometimes reproduce it) might be related this issue, I'll keep eyes on it. From our side, we can currently have a workaround.

For the restarting server scenario, I'm wondering if the view's content would be changed after we call this.refresh() again. IMO the case causing "out of sync" is that the project has been changed during the client misses the connection to the server. Otherwise, the refreshing will bring the same result. Thus, I prefer to just removing it. Does this make sense?

[badsyntax]

Yes I can also replicate the treeview bug when I drag the gradle tasks view into the main file explorer view (start-as-open) and restart vscode (but as you say that issue might still be related).

IMO the case causing "out of sync" is that the project has been changed during the client misses the connection to the server. Otherwise, the refreshing will bring the same result. Thus, I prefer to just removing it. Does this make sense?

It makes sense yes. Refreshing will bring the same result in most cases. Maybe I added it for extreme edge cases where maybe the task view was not previously rendered with anything due to a server start failure, but i can't remember and the PR that introduced that change doesn't describe exactly why I added it. I think it's totally fine to remove it 👍