Build does not wait for files to save

[yecril71pl]

Issue Type: Bug

In order to be able to run my code, I have to modify
file:///proc/sys/fs/inotify/max_user_instances first. So I did and I told VS Code to run.
This file, however, requires privilege escalation to be saved. Here is the order of events:

1. Ask me whether to escalate.
2. Build my code.
3. Run my code.
4. Get root password.
5. Save the file.

Here is the expected order of events:

1. Ask me whether to escalate.
2. Get root password.
3. Save the file.
4. Build my code.
5. Run my code.

VS Code version: Code 1.63.2 (899d46d, 2021-12-15T09:39:46.686Z)
OS version: Linux x64 5.15.8-1-default
Restricted Mode: No

System Info

Item	Value
CPUs	Intel(R) Core(TM)2 Quad CPU Q6600 @ 2.40GHz (4 x 2393)
GPU Status	2d_canvas: enabled gpu_compositing: enabled multiple_raster_threads: enabled_on oop_rasterization: disabled_off opengl: enabled_on rasterization: disabled_software skia_renderer: enabled_on video_decode: disabled_software vulkan: disabled_off webgl: enabled webgl2: enabled
Load (avg)	3, 4, 4
Memory (System)	5.66GB (0.12GB free)
Process Argv	--unity-launch --crash-reporter-id 2a03cb52-9f4b-48bf-93d3-5c0537d534ee
Screen Reader	no
VM	0%
DESKTOP_SESSION	/usr/share/xsessions/plasma5
XDG_CURRENT_DESKTOP	KDE
XDG_SESSION_DESKTOP	KDE
XDG_SESSION_TYPE	x11

Extensions (26)

Extension	Author (truncated)	Version
doxdocgen	csc	1.3.2
vscode-resx-editor	Dom	0.0.5
vscode-pull-request-github	Git	0.34.2
Ionide-fsharp	Ion	5.10.2
better-cpp-syntax	jef	1.15.10
vscode-language-pack-pl	MS-	1.63.3
csharp	ms-	1.23.17
dotnet-interactive-vscode	ms-	1.0.2606010
vscode-dotnet-pack	ms-	1.0.4
python	ms-	2021.12.1559732655
vscode-pylance	ms-	2021.12.2
jupyter	ms-	2021.11.1001550889
jupyter-keymap	ms-	1.0.0
jupyter-renderers	ms-	1.0.4
remote-containers	ms-	0.209.6
remote-ssh	ms-	0.70.0
remote-ssh-edit	ms-	0.70.0
remote-wsl	ms-	0.63.13
cmake-tools	ms-	1.9.2
cpptools	ms-	1.7.1
cpptools-extension-pack	ms-	1.1.0
powershell	ms-	2021.12.0
vscode-markdown-notebook	ms-	0.0.26
vsliveshare	ms-	1.0.5200
vsliveshare-audio	ms-	0.1.91
cmake	twx	0.0.17

(1 theme extensions excluded)

A/B Experiments

vsliv368:30146709
vsreu685:30147344
python383:30185418
vspor879:30202332
vspor708:30202333
vspor363:30204092
pythontb:30283811
pythonvspyt551cf:30345471
pythonptprofiler:30281270
vshan820:30294714
vstes263:30335439
pythondataviewer:30285071
vscod805:30301674
pythonvspyt200:30340761
binariesv615:30325510
bridge0708:30335490
bridge0723:30353136
vsaa593cf:30376535
pythonvs932:30410667
vscop804cf:30404767
vscop940:30404999
vsrem710:30416614

[bpasero]

Well I think "build/debug" is waiting for files to save, but since the save fails (requiring admin password) it ignores that and continues to run. Maybe the user needs to be asked in this case.

[roblourens]

Sorry I don't understand. The OP says that build/run happened before saving? Does this resolve before the escalation process is done?

vscode/src/vs/workbench/contrib/debug/common/debugUtils.ts

Line 315 in e7b3724

await editorService.saveAll();

[yecril71pl]

The launcher should check the return value.

vscode/src/vs/workbench/services/editor/common/editorService.ts

Line 273 in e7b3724

* @returns `true` if all editors saved and `false` otherwise.

[roblourens]

I see that but I would expect that it doesn't resolve while the user is typing the password and then resolves true when the password is correct.

[roblourens]

Oh, I suppose it bails when it shows the notification to escalate

[yecril71pl]

Whether the implementation of this API makes sense is secondary to the fact that the client ignores the contract.

[bpasero]

Looks like a save error is swallowed from here:

vscode/src/vs/workbench/services/textfile/common/textFileEditorModel.ts

Line 711 in c8e2fd7

await this.doSave(options);

and

vscode/src/vs/workbench/services/textfile/common/textFileEditorModel.ts

Lines 864 to 866 in c8e2fd7

	} catch (error) {
	this.handleSaveError(error, versionId, options);
	}

Will see if I can return false in case of an error so that saveAll would return false too.

[yecril71pl]

A function that does not return anything returns undefined, which is just fine. There is no need to explicitly return false.

[bpasero]

With 42a43eb, a call to IEditorService.save or IEditorService.saveAll will now correctly return false if any editor that is saved failed to save.

[roblourens]

I think the original request is that we would wait and start debugging after "Retry as Sudo" is clicked. We don't have a way to wait on this and I wouldn't want to block debugging on a non-modal dialog anyway, so does it make sense to show a message like "Save failed, start debugging anyway?" which would show up at the same time as the "Failed to save" notification.

[bpasero]

Fair, but since this is not a modal dialog but just a notification the save operation at that point has just failed and the editor remains dirty until the user tries again. There is no way currently to join that via any API.

[yecril71pl]

Please read the description once more. There is no need to try again.