Skip to content

Commit 2f0369f

Browse files
amungeramunger
authored
return file Operation error rather than throwing (microsoft#259654)
send file op error object across in full
1 parent 5d0e1fa commit 2f0369f

File tree

4 files changed

+114
-66
lines changed

4 files changed

+114
-66
lines changed

‎src/vs/workbench/api/browser/mainThreadNotebook.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { revive } from '../../../base/common/marshalling.js';
2424
import { INotebookFileMatchNoModel } from '../../contrib/search/common/searchNotebookHelpers.js';
2525
import { NotebookPriorityInfo } from '../../contrib/search/common/search.js';
2626
import { coalesce } from '../../../base/common/arrays.js';
27+
import { FileOperationError } from '../../../platform/files/common/files.js';
2728

2829
@extHostNamedCustomer(MainContext.MainThreadNotebook)
2930
export class MainThreadNotebooks implements MainThreadNotebookShape {
@@ -80,6 +81,9 @@ export class MainThreadNotebooks implements MainThreadNotebookShape {
8081
},
8182
save: async (uri, versionId, options, token) => {
8283
const stat = await this._proxy.$saveNotebook(handle, uri, versionId, options, token);
84+
if (isFileOperationError(stat)) {
85+
throw new FileOperationError(stat.message, stat.fileOperationResult, stat.options);
86+
}
8387
return {
8488
...stat,
8589
children: undefined,
@@ -222,3 +226,7 @@ CommandsRegistry.registerCommand('_executeNotebookToData', async (accessor, ...a
222226
const bytes = await info.serializer.notebookToData(data);
223227
return bytes;
224228
});
229+
230+
function isFileOperationError(error: any): error is FileOperationError {
231+
return typeof error.fileOperationResult === 'number' && typeof error.message === 'string';
232+
}

‎src/vs/workbench/api/common/extHost.protocol.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2846,7 +2846,7 @@ export interface ExtHostNotebookShape extends ExtHostNotebookDocumentsAndEditors
28462846

28472847
$dataToNotebook(handle: number, data: VSBuffer, token: CancellationToken): Promise<SerializableObjectWithBuffers<NotebookDataDto>>;
28482848
$notebookToData(handle: number, data: SerializableObjectWithBuffers<NotebookDataDto>, token: CancellationToken): Promise<VSBuffer>;
2849-
$saveNotebook(handle: number, uri: UriComponents, versionId: number, options: files.IWriteFileOptions, token: CancellationToken): Promise<INotebookPartialFileStatsWithMetadata>;
2849+
$saveNotebook(handle: number, uri: UriComponents, versionId: number, options: files.IWriteFileOptions, token: CancellationToken): Promise<INotebookPartialFileStatsWithMetadata | files.FileOperationError>;
28502850

28512851
$searchInNotebooks(handle: number, textQuery: search.ITextQuery, viewTypeFileTargets: NotebookPriorityInfo[], otherViewTypeFileTargets: NotebookPriorityInfo[], token: CancellationToken): Promise<{ results: IRawClosedNotebookFileMatch[]; limitHit: boolean }>;
28522852
}

‎src/vs/workbench/api/common/extHostNotebook.ts

Lines changed: 71 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -313,83 +313,91 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
313313
return VSBuffer.wrap(bytes);
314314
}
315315

316-
async $saveNotebook(handle: number, uriComponents: UriComponents, versionId: number, options: files.IWriteFileOptions, token: CancellationToken): Promise<INotebookPartialFileStatsWithMetadata> {
316+
async $saveNotebook(handle: number, uriComponents: UriComponents, versionId: number, options: files.IWriteFileOptions, token: CancellationToken): Promise<INotebookPartialFileStatsWithMetadata | files.FileOperationError> {
317317
const uri = URI.revive(uriComponents);
318318
const serializer = this._notebookSerializer.get(handle);
319319
this.trace(`enter saveNotebook(versionId: ${versionId}, ${uri.toString()})`);
320320

321-
if (!serializer) {
322-
throw new NotebookSaveError('NO serializer found');
323-
}
321+
try {
322+
if (!serializer) {
323+
throw new NotebookSaveError('NO serializer found');
324+
}
324325

325-
const document = this._documents.get(uri);
326-
if (!document) {
327-
throw new NotebookSaveError('Document NOT found');
328-
}
326+
const document = this._documents.get(uri);
327+
if (!document) {
328+
throw new NotebookSaveError('Document NOT found');
329+
}
329330

330-
if (document.versionId !== versionId) {
331-
throw new NotebookSaveError('Document version mismatch, expected: ' + versionId + ', actual: ' + document.versionId);
332-
}
331+
if (document.versionId !== versionId) {
332+
throw new NotebookSaveError('Document version mismatch, expected: ' + versionId + ', actual: ' + document.versionId);
333+
}
333334

334-
if (!this._extHostFileSystem.value.isWritableFileSystem(uri.scheme)) {
335-
throw new files.FileOperationError(localize('err.readonly', "Unable to modify read-only file '{0}'", this._resourceForError(uri)), files.FileOperationResult.FILE_PERMISSION_DENIED);
336-
}
335+
if (!this._extHostFileSystem.value.isWritableFileSystem(uri.scheme)) {
336+
throw new files.FileOperationError(localize('err.readonly', "Unable to modify read-only file '{0}'", this._resourceForError(uri)), files.FileOperationResult.FILE_PERMISSION_DENIED);
337+
}
337338

338-
const data: vscode.NotebookData = {
339-
metadata: filter(document.apiNotebook.metadata, key => !(serializer.options?.transientDocumentMetadata ?? {})[key]),
340-
cells: [],
341-
};
339+
const data: vscode.NotebookData = {
340+
metadata: filter(document.apiNotebook.metadata, key => !(serializer.options?.transientDocumentMetadata ?? {})[key]),
341+
cells: [],
342+
};
342343

343-
// this data must be retrieved before any async calls to ensure the data is for the correct version
344-
for (const cell of document.apiNotebook.getCells()) {
345-
const cellData = new extHostTypes.NotebookCellData(
346-
cell.kind,
347-
cell.document.getText(),
348-
cell.document.languageId,
349-
cell.mime,
350-
!(serializer.options?.transientOutputs) ? [...cell.outputs] : [],
351-
cell.metadata,
352-
cell.executionSummary
353-
);
354-
355-
cellData.metadata = filter(cell.metadata, key => !(serializer.options?.transientCellMetadata ?? {})[key]);
356-
data.cells.push(cellData);
357-
}
344+
// this data must be retrieved before any async calls to ensure the data is for the correct version
345+
for (const cell of document.apiNotebook.getCells()) {
346+
const cellData = new extHostTypes.NotebookCellData(
347+
cell.kind,
348+
cell.document.getText(),
349+
cell.document.languageId,
350+
cell.mime,
351+
!(serializer.options?.transientOutputs) ? [...cell.outputs] : [],
352+
cell.metadata,
353+
cell.executionSummary
354+
);
358355

359-
// validate write
360-
await this._validateWriteFile(uri, options);
356+
cellData.metadata = filter(cell.metadata, key => !(serializer.options?.transientCellMetadata ?? {})[key]);
357+
data.cells.push(cellData);
358+
}
361359

362-
if (token.isCancellationRequested) {
363-
throw new CancellationError();
364-
}
365-
const bytes = await serializer.serializer.serializeNotebook(data, token);
366-
if (token.isCancellationRequested) {
367-
throw new CancellationError();
368-
}
360+
// validate write
361+
await this._validateWriteFile(uri, options);
369362

370-
// Don't accept any cancellation beyond this point, we need to report the result of the file write
371-
this.trace(`serialized versionId: ${versionId} ${uri.toString()}`);
372-
await this._extHostFileSystem.value.writeFile(uri, bytes);
373-
this.trace(`Finished write versionId: ${versionId} ${uri.toString()}`);
374-
const providerExtUri = this._extHostFileSystem.getFileSystemProviderExtUri(uri.scheme);
375-
const stat = await this._extHostFileSystem.value.stat(uri);
363+
if (token.isCancellationRequested) {
364+
throw new CancellationError();
365+
}
366+
const bytes = await serializer.serializer.serializeNotebook(data, token);
367+
if (token.isCancellationRequested) {
368+
throw new CancellationError();
369+
}
376370

377-
const fileStats = {
378-
name: providerExtUri.basename(uri),
379-
isFile: (stat.type & files.FileType.File) !== 0,
380-
isDirectory: (stat.type & files.FileType.Directory) !== 0,
381-
isSymbolicLink: (stat.type & files.FileType.SymbolicLink) !== 0,
382-
mtime: stat.mtime,
383-
ctime: stat.ctime,
384-
size: stat.size,
385-
readonly: Boolean((stat.permissions ?? 0) & files.FilePermission.Readonly) || !this._extHostFileSystem.value.isWritableFileSystem(uri.scheme),
386-
locked: Boolean((stat.permissions ?? 0) & files.FilePermission.Locked),
387-
etag: files.etag({ mtime: stat.mtime, size: stat.size }),
388-
children: undefined
389-
};
371+
// Don't accept any cancellation beyond this point, we need to report the result of the file write
372+
this.trace(`serialized versionId: ${versionId} ${uri.toString()}`);
373+
await this._extHostFileSystem.value.writeFile(uri, bytes);
374+
this.trace(`Finished write versionId: ${versionId} ${uri.toString()}`);
375+
const providerExtUri = this._extHostFileSystem.getFileSystemProviderExtUri(uri.scheme);
376+
const stat = await this._extHostFileSystem.value.stat(uri);
377+
378+
const fileStats = {
379+
name: providerExtUri.basename(uri),
380+
isFile: (stat.type & files.FileType.File) !== 0,
381+
isDirectory: (stat.type & files.FileType.Directory) !== 0,
382+
isSymbolicLink: (stat.type & files.FileType.SymbolicLink) !== 0,
383+
mtime: stat.mtime,
384+
ctime: stat.ctime,
385+
size: stat.size,
386+
readonly: Boolean((stat.permissions ?? 0) & files.FilePermission.Readonly) || !this._extHostFileSystem.value.isWritableFileSystem(uri.scheme),
387+
locked: Boolean((stat.permissions ?? 0) & files.FilePermission.Locked),
388+
etag: files.etag({ mtime: stat.mtime, size: stat.size }),
389+
children: undefined
390+
};
390391

391-
this.trace(`exit saveNotebook(versionId: ${versionId}, ${uri.toString()})`);
392-
return fileStats;
392+
this.trace(`exit saveNotebook(versionId: ${versionId}, ${uri.toString()})`);
393+
return fileStats;
394+
} catch (error) {
395+
// return fileOperationsErrors to keep the whole object across serialization, these errors are handled specially by the WCS
396+
if (error instanceof files.FileOperationError) {
397+
return { ...error, message: error.message };
398+
}
399+
throw error;
400+
}
393401
}
394402

395403
/**

‎src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { Schemas } from '../../../../base/common/network.js';
1313
import { assertType } from '../../../../base/common/types.js';
1414
import { URI } from '../../../../base/common/uri.js';
1515
import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
16-
import { IWriteFileOptions, IFileStatWithMetadata } from '../../../../platform/files/common/files.js';
16+
import { IWriteFileOptions, IFileStatWithMetadata, FileOperationError, FileOperationResult } from '../../../../platform/files/common/files.js';
1717
import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js';
1818
import { IRevertOptions, ISaveOptions, IUntypedEditorInput } from '../../../common/editor.js';
1919
import { EditorModel } from '../../../common/editor/editorModel.js';
@@ -276,7 +276,7 @@ export class NotebookFileWorkingCopyModel extends Disposable implements IStoredF
276276
error: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; comment: 'Info about the error that occurred' };
277277
};
278278
const isIPynb = this._notebookModel.viewType === 'jupyter-notebook' || this._notebookModel.viewType === 'interactive';
279-
const errorMessage = error.name === 'NotebookSaveError' ? error.message : 'Unknown error';
279+
const errorMessage = getSaveErrorMessage(error);
280280
this._telemetryService.publicLogError2<notebookSaveErrorData, notebookSaveErrorClassification>('notebook/SaveError', {
281281
isRemote: this._notebookModel.uri.scheme === Schemas.vscodeRemote,
282282
isIPyNbWorkerSerializer: isIPynb && this._configurationService.getValue<boolean>('ipynb.experimental.serialization'),
@@ -362,3 +362,35 @@ class NotebookSaveError extends Error {
362362
this.name = 'NotebookSaveError';
363363
}
364364
}
365+
366+
function getSaveErrorMessage(error: Error): string {
367+
if (error.name === 'NotebookSaveError') {
368+
return error.message;
369+
} else if (error instanceof FileOperationError) {
370+
switch (error.fileOperationResult) {
371+
case FileOperationResult.FILE_IS_DIRECTORY:
372+
return 'File is a directory';
373+
case FileOperationResult.FILE_NOT_FOUND:
374+
return 'File not found';
375+
case FileOperationResult.FILE_NOT_MODIFIED_SINCE:
376+
return 'File not modified since';
377+
case FileOperationResult.FILE_MODIFIED_SINCE:
378+
return 'File modified since';
379+
case FileOperationResult.FILE_MOVE_CONFLICT:
380+
return 'File move conflict';
381+
case FileOperationResult.FILE_WRITE_LOCKED:
382+
return 'File write locked';
383+
case FileOperationResult.FILE_PERMISSION_DENIED:
384+
return 'File permission denied';
385+
case FileOperationResult.FILE_TOO_LARGE:
386+
return 'File too large';
387+
case FileOperationResult.FILE_INVALID_PATH:
388+
return 'File invalid path';
389+
case FileOperationResult.FILE_NOT_DIRECTORY:
390+
return 'File not directory';
391+
case FileOperationResult.FILE_OTHER_ERROR:
392+
return 'File other error';
393+
}
394+
}
395+
return 'Unknown error';
396+
}

0 commit comments

Comments
 (0)