working copy - properly return false when save fails (#139764)

src/vs/workbench/services/textfile/browser/textFileService.ts

@@ -426,14 +426,14 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
 			}
 		}
 
-		// Revert the source if result is success
-		if (success) {
-			await this.revert(source);
-
-			return target;
+		if (!success) {
+			return undefined;
 		}
 
-		return undefined;
+		// Revert the source
+		await this.revert(source);
+
+		return target;
 	}
 
 	private async doSaveAsTextFile(sourceModel: IResolvedTextEditorModel | ITextModel, source: URI, target: URI, options?: ITextFileSaveOptions): Promise<boolean> {

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

@@ -711,7 +711,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 		await this.doSave(options);
 		this.logService.trace('[text file model] save() - exit', this.resource.toString(true));
 
-		return true;
+		return this.hasState(TextFileEditorModelState.SAVED);
 	}
 
 	private async doSave(options: ITextFileSaveOptions): Promise<void> {

src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts

@@ -194,6 +194,26 @@ suite('Files - TextFileEditorModel', () => {
 		assert.ok(!accessor.modelService.getModel(model.resource));
 	});
 
+	test('save - returns false when save fails', async function () {
+		const model: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/index_async.txt'), 'utf8', undefined);
+
+		await model.resolve();
+
+		accessor.fileService.writeShouldThrowError = new Error('failed to write');
+		try {
+			const res = await model.save({ force: true });
+			assert.strictEqual(res, false);
+		} finally {
+			accessor.fileService.writeShouldThrowError = undefined;
+		}
+
+		const res = await model.save({ force: true });
+		assert.strictEqual(res, true);
+
+		model.dispose();
+		assert.ok(!accessor.modelService.getModel(model.resource));
+	});
+
 	test('save error (generic)', async function () {
 		const model: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/index_async.txt'), 'utf8', undefined);
 

src/vs/workbench/services/workingCopy/common/abstractFileWorkingCopyManager.ts

@@ -149,15 +149,15 @@ export abstract class BaseFileWorkingCopyManager<M extends IFileWorkingCopyModel
 	private async saveWithFallback(workingCopy: W): Promise<void> {
 
 		// First try regular save
-		let saveFailed = false;
+		let saveSuccess = false;
 		try {
-			await workingCopy.save();
+			saveSuccess = await workingCopy.save();
 		} catch (error) {
-			saveFailed = true;
+			// Ignore
 		}
 
 		// Then fallback to backup if that exists
-		if (saveFailed || workingCopy.isDirty()) {
+		if (!saveSuccess || workingCopy.isDirty()) {
 			const backup = await this.workingCopyBackupService.resolve(workingCopy);
 			if (backup) {
 				await this.fileService.writeFile(workingCopy.resource, backup.value, { unlock: true });

src/vs/workbench/services/workingCopy/common/fileWorkingCopyManager.ts

@@ -406,7 +406,10 @@ export class FileWorkingCopyManager<S extends IStoredFileWorkingCopyModel, U ext
 		await targetStoredFileWorkingCopy.model?.update(sourceContents, CancellationToken.None);
 
 		// Save target
-		await targetStoredFileWorkingCopy.save({ ...options, force: true  /* force to save, even if not dirty (https://github.com/microsoft/vscode/issues/99619) */ });
+		const success = await targetStoredFileWorkingCopy.save({ ...options, force: true  /* force to save, even if not dirty (https://github.com/microsoft/vscode/issues/99619) */ });
+		if (!success) {
+			return undefined;
+		}
 
 		// Revert the source
 		await sourceWorkingCopy?.revert();

src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts

@@ -764,7 +764,7 @@ export class StoredFileWorkingCopy<M extends IStoredFileWorkingCopyModel> extend
 		await this.doSave(options);
 		this.trace('[stored file working copy] save() - exit');
 
-		return true;
+		return this.hasState(StoredFileWorkingCopyState.SAVED);
 	}
 
 	private async doSave(options: IStoredFileWorkingCopySaveOptions): Promise<void> {

src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts

@@ -338,7 +338,6 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp
 			}
 
 			// If we still have dirty working copies, revert those directly
-			// unless the revert operation was not successful (e.g. cancelled)
 			await Promises.settled(dirtyWorkingCopies.map(workingCopy => workingCopy.isDirty() ? workingCopy.revert(revertOptions) : Promise.resolve()));
 		}, localize('revertBeforeShutdown', "Reverting editors with unsaved changes is taking longer than expected..."));
 	}

src/vs/workbench/services/workingCopy/test/browser/storedFileWorkingCopy.test.ts

@@ -604,6 +604,22 @@ suite('StoredFileWorkingCopy', function () {
 		assert.ok(error);
 	});
 
+	test('save - returns false when save fails', async function () {
+		await workingCopy.resolve();
+
+		try {
+			accessor.fileService.writeShouldThrowError = new FileOperationError('write error', FileOperationResult.FILE_PERMISSION_DENIED);
+
+			const res = await workingCopy.save({ force: true });
+			assert.strictEqual(res, false);
+		} finally {
+			accessor.fileService.writeShouldThrowError = undefined;
+		}
+
+		const res = await workingCopy.save({ force: true });
+		assert.strictEqual(res, true);
+	});
+
 	test('save participant', async () => {
 		await workingCopy.resolve();
 

src/vscode-dts/vscode.d.ts

@@ -136,9 +136,8 @@ declare module 'vscode' {
 		/**
 		 * Save the underlying file.
 		 *
-		 * @return A promise that will resolve to true when the file
-		 * has been saved. If the file was not dirty or the save failed,
-		 * will return false.
+		 * @return A promise that will resolve to `true` when the file
+		 * has been saved. If the save failed, will return `false`.
 		 */
 		save(): Thenable<boolean>;
 
@@ -10984,7 +10983,8 @@ declare module 'vscode' {
 		 * Save all dirty files.
 		 *
 		 * @param includeUntitled Also save files that have been created during this session.
-		 * @return A thenable that resolves when the files have been saved.
+		 * @return A thenable that resolves when the files have been saved. Will return `false`
+		 * for any file that failed to save.
 		 */
 		export function saveAll(includeUntitled?: boolean): Thenable<boolean>;