Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix last modified since error while writing settings #144058

Merged
merged 1 commit into from
Feb 28, 2022
Merged

Fix last modified since error while writing settings #144058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 29 additions & 12 deletions src/vs/workbench/services/configuration/common/configurationEditingService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment'
import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
import { IConfigurationService, IConfigurationUpdateOverrides } from 'vs/platform/configuration/common/configuration';
import { FOLDER_SETTINGS_PATH, WORKSPACE_STANDALONE_CONFIGURATIONS, TASKS_CONFIGURATION_KEY, LAUNCH_CONFIGURATION_KEY, USER_STANDALONE_CONFIGURATIONS, TASKS_DEFAULT, FOLDER_SCOPES } from 'vs/workbench/services/configuration/common/configuration';
import { IFileService } from 'vs/platform/files/common/files';
import { FileOperationError, FileOperationResult, IFileService } from 'vs/platform/files/common/files';
import { IResolvedTextEditorModel, ITextModelService } from 'vs/editor/common/services/resolverService';
import { IConfigurationRegistry, Extensions as ConfigurationExtensions, ConfigurationScope, keyFromOverrideIdentifiers, OVERRIDE_PROPERTY_REGEX } from 'vs/platform/configuration/common/configurationRegistry';
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
Expand Down Expand Up @@ -90,7 +90,12 @@ export const enum ConfigurationEditingErrorCode {
/**
* Error when trying to write to a configuration file that contains JSON errors.
*/
ERROR_INVALID_CONFIGURATION
ERROR_INVALID_CONFIGURATION,

/**
* Internal Error.
*/
ERROR_INTERNAL
}

export class ConfigurationEditingError extends Error {
Expand Down Expand Up @@ -161,16 +166,19 @@ export class ConfigurationEditingService {
});
}

writeConfiguration(target: EditableConfigurationTarget, value: IConfigurationValue, options: IConfigurationEditingOptions = {}): Promise<void> {
async writeConfiguration(target: EditableConfigurationTarget, value: IConfigurationValue, options: IConfigurationEditingOptions = {}): Promise<void> {
const operation = this.getConfigurationEditOperation(target, value, options.scopes || {});
return Promise.resolve(this.queue.queue(() => this.doWriteConfiguration(operation, options) // queue up writes to prevent race conditions
.then(() => { },
async error => {
if (!options.donotNotifyError) {
await this.onError(error, operation, options.scopes);
}
return Promise.reject(error);
})));
// queue up writes to prevent race conditions
return this.queue.queue(async () => {
try {
await this.doWriteConfiguration(operation, options);
} catch (error) {
if (options.donotNotifyError) {
throw error;
}
await this.onError(error, operation, options.scopes);
}
});
}

private async doWriteConfiguration(operation: IConfigurationEditOperation, options: ConfigurationEditingOptions): Promise<void> {
Expand All @@ -192,7 +200,15 @@ export class ConfigurationEditingService {

const edit = this.getEdits(operation, model.getValue(), formattingOptions)[0];
if (edit && this.applyEditsToBuffer(edit, model)) {
await this.textFileService.save(model.uri);
try {
await this.textFileService.save(model.uri, { ignoreErrorHandler: true });
} catch (error) {
if ((<FileOperationError>error).fileOperationResult === FileOperationResult.FILE_MODIFIED_SINCE) {
try { await this.textFileService.revert(model.uri); } catch (error) { /* Ignore */ }
throw this.toConfigurationEditingError(ConfigurationEditingErrorCode.ERROR_CONFIGURATION_FILE_MODIFIED_SINCE, operation.target, operation);
}
throw this.toConfigurationEditingError(ConfigurationEditingErrorCode.ERROR_INTERNAL, operation.target, operation);
}
}
}

Expand Down Expand Up @@ -417,6 +433,7 @@ export class ConfigurationEditingService {
case EditableConfigurationTarget.WORKSPACE_FOLDER:
return nls.localize('errorConfigurationFileModifiedSinceFolder', "Unable to write into folder settings because the content of the file is newer.");
}
case ConfigurationEditingErrorCode.ERROR_INTERNAL: return nls.localize('errorUnknown', "Unable to write to {0} because of an internal error.", this.stringifyTarget(target));
}
}

Expand Down
28 changes: 17 additions & 11 deletions src/vs/workbench/services/configuration/test/browser/configurationEditingService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,52 +113,57 @@ suite('ConfigurationEditingService', () => {

test('errors cases - invalid key', async () => {
try {
await testObject.writeConfiguration(EditableConfigurationTarget.WORKSPACE, { key: 'unknown.key', value: 'value' });
assert.fail('Should fail with ERROR_UNKNOWN_KEY');
await testObject.writeConfiguration(EditableConfigurationTarget.WORKSPACE, { key: 'unknown.key', value: 'value' }, { donotNotifyError: true });
} catch (error) {
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_UNKNOWN_KEY);
return;
}
assert.fail('Should fail with ERROR_UNKNOWN_KEY');
});

test('errors cases - no workspace', async () => {
await workspaceService.initialize({ id: uuid.generateUuid() });
try {
await testObject.writeConfiguration(EditableConfigurationTarget.WORKSPACE, { key: 'configurationEditing.service.testSetting', value: 'value' });
assert.fail('Should fail with ERROR_NO_WORKSPACE_OPENED');
await testObject.writeConfiguration(EditableConfigurationTarget.WORKSPACE, { key: 'configurationEditing.service.testSetting', value: 'value' }, { donotNotifyError: true });
} catch (error) {
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_NO_WORKSPACE_OPENED);
return;
}
assert.fail('Should fail with ERROR_NO_WORKSPACE_OPENED');
});

test('errors cases - invalid configuration', async () => {
await fileService.writeFile(environmentService.settingsResource, VSBuffer.fromString(',,,,,,,,,,,,,,'));
try {
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'configurationEditing.service.testSetting', value: 'value' });
assert.fail('Should fail with ERROR_INVALID_CONFIGURATION');
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'configurationEditing.service.testSetting', value: 'value' }, { donotNotifyError: true });
} catch (error) {
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_INVALID_CONFIGURATION);
return;
}
assert.fail('Should fail with ERROR_INVALID_CONFIGURATION');
});

test('errors cases - invalid global tasks configuration', async () => {
const resource = joinPath(environmentService.userRoamingDataHome, USER_STANDALONE_CONFIGURATIONS['tasks']);
await fileService.writeFile(resource, VSBuffer.fromString(',,,,,,,,,,,,,,'));
try {
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'tasks.configurationEditing.service.testSetting', value: 'value' });
assert.fail('Should fail with ERROR_INVALID_CONFIGURATION');
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'tasks.configurationEditing.service.testSetting', value: 'value' }, { donotNotifyError: true });
} catch (error) {
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_INVALID_CONFIGURATION);
return;
}
assert.fail('Should fail with ERROR_INVALID_CONFIGURATION');
});

test('errors cases - dirty', async () => {
instantiationService.stub(ITextFileService, 'isDirty', true);
try {
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'configurationEditing.service.testSetting', value: 'value' });
assert.fail('Should fail with ERROR_CONFIGURATION_FILE_DIRTY error.');
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'configurationEditing.service.testSetting', value: 'value' }, { donotNotifyError: true });
} catch (error) {
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_CONFIGURATION_FILE_DIRTY);
return;
}
assert.fail('Should fail with ERROR_CONFIGURATION_FILE_DIRTY error.');
});

test('do not notify error', async () => {
Expand All @@ -167,11 +172,12 @@ suite('ConfigurationEditingService', () => {
instantiationService.stub(INotificationService, <INotificationService>{ prompt: target, _serviceBrand: undefined, onDidAddNotification: undefined!, onDidRemoveNotification: undefined!, notify: null!, error: null!, info: null!, warn: null!, status: null!, setFilter: null! });
try {
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'configurationEditing.service.testSetting', value: 'value' }, { donotNotifyError: true });
assert.fail('Should fail with ERROR_CONFIGURATION_FILE_DIRTY error.');
} catch (error) {
assert.strictEqual(false, target.calledOnce);
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_CONFIGURATION_FILE_DIRTY);
return;
}
assert.fail('Should fail with ERROR_CONFIGURATION_FILE_DIRTY error.');
});

test('write one setting - empty file', async () => {
Expand Down