Fix for automatic folderOpen tasks with Remote SSH extension #204008
Conversation
In some cases (very often for me), the task configuration is loaded after the RunAutomaticTasks class has searched through the configured tasks to see if there are any automatic tasks to run. Since this search is a one time action, it means that the automatic tasks aren't always run. This fix works around this race condition by exposing an event (onDidChangeTaskConfig) on the ITaskService interface and letting the AbstractTaskService implementation fire this event when it reloads the task configuration. If the RunAutomaticTasks class encounters zero automatic tasks, it waits for up to 10 seconds for this new event to fire before giving up. If the task configuration is updated during this time, RunAutomaticTasks looks for automatic tasks once more in the updated task configuration.
|
@microsoft-github-policy-service agree |
| Event.toPromise(Event.once(this._taskService.onDidChangeTaskConfig)).then(() => resolve(true)); | ||
| }), | ||
| new Promise<boolean>((resolve) => { | ||
| const timer = setTimeout(() => { clearTimeout(timer); resolve(false); }, 10000); |
There was a problem hiding this comment.
Waiting on some timeout is suspicious to me. Seems like you should just be able to know for sure when the task configuration is loaded. But I reloaded the window a few times and it worked when I tried it.
There was a problem hiding this comment.
Many thanks for your review!
I fully agree that this isn't the prettiest solution. In my opinion, it would have been better if TaskService waited with firing the existing onDidChangeTaskSystemInfo event until it had tried to fetch the task configuration, if any. I suspect this is where the hidden timing issue really is. But I know too little about the interactions involved in this code base, also together with the remote SSH extension, to look into that right now. And debugging all this was really quite complex when I couldn't use the remote SSH extension with my custom Code OSS build.
The timer is just there to not wait forever if there is no task configuration at all. Removing the timer (and not waiting just once for the new event) could be a way to support running automatic tasks that are configured manually after having opened the folder. But that's new functionality and not something I'd put in a bug fix.
Anyway, I just tested the latest Insiders build on Windows, which includes this commit. I can no longer reproduce the issues I had.
In some cases (very often for me), the task configuration is loaded after the RunAutomaticTasks class has searched through the configured tasks to see if there are any automatic tasks to run. Since this search is a one time action, it means that the automatic tasks aren't always run.
This fix works around this race condition by exposing an event (onDidChangeTaskConfig) on the ITaskService interface and letting the AbstractTaskService implementation fire this event when it reloads the task configuration. If the RunAutomaticTasks class encounters zero automatic tasks, it waits for up to 10 seconds for this new event to fire before giving up. If the task configuration is updated during this time, RunAutomaticTasks looks for automatic tasks once more in the updated task configuration.
Fixes #203639