Skip to content

Commit b7730c8

Browse files
sandy081sandy081
authored
Merge pull request microsoft#144058 from microsoft/sandy081/fix142342
Fix last modified since error while writing settings
2 parents 919d018 + 0c52bc7 commit b7730c8

File tree

2 files changed

+46
-23
lines changed

2 files changed

+46
-23
lines changed

‎src/vs/workbench/services/configuration/common/configurationEditingService.ts‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment'
1515
import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
1616
import { IConfigurationService, IConfigurationUpdateOverrides } from 'vs/platform/configuration/common/configuration';
1717
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';
18-
import { IFileService } from 'vs/platform/files/common/files';
18+
import { FileOperationError, FileOperationResult, IFileService } from 'vs/platform/files/common/files';
1919
import { IResolvedTextEditorModel, ITextModelService } from 'vs/editor/common/services/resolverService';
2020
import { IConfigurationRegistry, Extensions as ConfigurationExtensions, ConfigurationScope, keyFromOverrideIdentifiers, OVERRIDE_PROPERTY_REGEX } from 'vs/platform/configuration/common/configurationRegistry';
2121
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
@@ -90,7 +90,12 @@ export const enum ConfigurationEditingErrorCode {
9090
/**
9191
* Error when trying to write to a configuration file that contains JSON errors.
9292
*/
93-
ERROR_INVALID_CONFIGURATION
93+
ERROR_INVALID_CONFIGURATION,
94+
95+
/**
96+
* Internal Error.
97+
*/
98+
ERROR_INTERNAL
9499
}
95100

96101
export class ConfigurationEditingError extends Error {
@@ -161,16 +166,19 @@ export class ConfigurationEditingService {
161166
});
162167
}
163168

164-
writeConfiguration(target: EditableConfigurationTarget, value: IConfigurationValue, options: IConfigurationEditingOptions = {}): Promise<void> {
169+
async writeConfiguration(target: EditableConfigurationTarget, value: IConfigurationValue, options: IConfigurationEditingOptions = {}): Promise<void> {
165170
const operation = this.getConfigurationEditOperation(target, value, options.scopes || {});
166-
return Promise.resolve(this.queue.queue(() => this.doWriteConfiguration(operation, options) // queue up writes to prevent race conditions
167-
.then(() => { },
168-
async error => {
169-
if (!options.donotNotifyError) {
170-
await this.onError(error, operation, options.scopes);
171-
}
172-
return Promise.reject(error);
173-
})));
171+
// queue up writes to prevent race conditions
172+
return this.queue.queue(async () => {
173+
try {
174+
await this.doWriteConfiguration(operation, options);
175+
} catch (error) {
176+
if (options.donotNotifyError) {
177+
throw error;
178+
}
179+
await this.onError(error, operation, options.scopes);
180+
}
181+
});
174182
}
175183

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

193201
const edit = this.getEdits(operation, model.getValue(), formattingOptions)[0];
194202
if (edit && this.applyEditsToBuffer(edit, model)) {
195-
await this.textFileService.save(model.uri);
203+
try {
204+
await this.textFileService.save(model.uri, { ignoreErrorHandler: true });
205+
} catch (error) {
206+
if ((<FileOperationError>error).fileOperationResult === FileOperationResult.FILE_MODIFIED_SINCE) {
207+
try { await this.textFileService.revert(model.uri); } catch (error) { /* Ignore */ }
208+
throw this.toConfigurationEditingError(ConfigurationEditingErrorCode.ERROR_CONFIGURATION_FILE_MODIFIED_SINCE, operation.target, operation);
209+
}
210+
throw this.toConfigurationEditingError(ConfigurationEditingErrorCode.ERROR_INTERNAL, operation.target, operation);
211+
}
196212
}
197213
}
198214

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

‎src/vs/workbench/services/configuration/test/browser/configurationEditingService.test.ts‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -113,52 +113,57 @@ suite('ConfigurationEditingService', () => {
113113

114114
test('errors cases - invalid key', async () => {
115115
try {
116-
await testObject.writeConfiguration(EditableConfigurationTarget.WORKSPACE, { key: 'unknown.key', value: 'value' });
117-
assert.fail('Should fail with ERROR_UNKNOWN_KEY');
116+
await testObject.writeConfiguration(EditableConfigurationTarget.WORKSPACE, { key: 'unknown.key', value: 'value' }, { donotNotifyError: true });
118117
} catch (error) {
119118
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_UNKNOWN_KEY);
119+
return;
120120
}
121+
assert.fail('Should fail with ERROR_UNKNOWN_KEY');
121122
});
122123

123124
test('errors cases - no workspace', async () => {
124125
await workspaceService.initialize({ id: uuid.generateUuid() });
125126
try {
126-
await testObject.writeConfiguration(EditableConfigurationTarget.WORKSPACE, { key: 'configurationEditing.service.testSetting', value: 'value' });
127-
assert.fail('Should fail with ERROR_NO_WORKSPACE_OPENED');
127+
await testObject.writeConfiguration(EditableConfigurationTarget.WORKSPACE, { key: 'configurationEditing.service.testSetting', value: 'value' }, { donotNotifyError: true });
128128
} catch (error) {
129129
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_NO_WORKSPACE_OPENED);
130+
return;
130131
}
132+
assert.fail('Should fail with ERROR_NO_WORKSPACE_OPENED');
131133
});
132134

133135
test('errors cases - invalid configuration', async () => {
134136
await fileService.writeFile(environmentService.settingsResource, VSBuffer.fromString(',,,,,,,,,,,,,,'));
135137
try {
136-
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'configurationEditing.service.testSetting', value: 'value' });
137-
assert.fail('Should fail with ERROR_INVALID_CONFIGURATION');
138+
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'configurationEditing.service.testSetting', value: 'value' }, { donotNotifyError: true });
138139
} catch (error) {
139140
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_INVALID_CONFIGURATION);
141+
return;
140142
}
143+
assert.fail('Should fail with ERROR_INVALID_CONFIGURATION');
141144
});
142145

143146
test('errors cases - invalid global tasks configuration', async () => {
144147
const resource = joinPath(environmentService.userRoamingDataHome, USER_STANDALONE_CONFIGURATIONS['tasks']);
145148
await fileService.writeFile(resource, VSBuffer.fromString(',,,,,,,,,,,,,,'));
146149
try {
147-
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'tasks.configurationEditing.service.testSetting', value: 'value' });
148-
assert.fail('Should fail with ERROR_INVALID_CONFIGURATION');
150+
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'tasks.configurationEditing.service.testSetting', value: 'value' }, { donotNotifyError: true });
149151
} catch (error) {
150152
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_INVALID_CONFIGURATION);
153+
return;
151154
}
155+
assert.fail('Should fail with ERROR_INVALID_CONFIGURATION');
152156
});
153157

154158
test('errors cases - dirty', async () => {
155159
instantiationService.stub(ITextFileService, 'isDirty', true);
156160
try {
157-
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'configurationEditing.service.testSetting', value: 'value' });
158-
assert.fail('Should fail with ERROR_CONFIGURATION_FILE_DIRTY error.');
161+
await testObject.writeConfiguration(EditableConfigurationTarget.USER_LOCAL, { key: 'configurationEditing.service.testSetting', value: 'value' }, { donotNotifyError: true });
159162
} catch (error) {
160163
assert.strictEqual(error.code, ConfigurationEditingErrorCode.ERROR_CONFIGURATION_FILE_DIRTY);
164+
return;
161165
}
166+
assert.fail('Should fail with ERROR_CONFIGURATION_FILE_DIRTY error.');
162167
});
163168

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

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

0 commit comments

Comments
 (0)