From 5328a3f143db47ee44cc698712d66cde99a5d20f Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Tue, 21 May 2024 15:23:34 -0700 Subject: [PATCH] Save on EH diagnostics (#213171) * added trace logs * another trace message * fix test ctors --- src/vs/workbench/api/common/extHost.api.impl.ts | 2 +- .../api/common/extHostDocumentContentProviders.ts | 5 +++++ src/vs/workbench/api/common/extHostNotebook.ts | 13 ++++++++++++- .../api/test/browser/extHostNotebook.test.ts | 2 +- .../api/test/browser/extHostNotebookKernel.test.ts | 2 +- .../contrib/notebook/common/notebookEditorModel.ts | 6 +++--- .../workingCopy/common/storedFileWorkingCopy.ts | 10 ++++++++++ 7 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index a2cbad65c65..8c3aa184689 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -178,7 +178,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I const extHostDocuments = rpcProtocol.set(ExtHostContext.ExtHostDocuments, new ExtHostDocuments(rpcProtocol, extHostDocumentsAndEditors)); const extHostDocumentContentProviders = rpcProtocol.set(ExtHostContext.ExtHostDocumentContentProviders, new ExtHostDocumentContentProvider(rpcProtocol, extHostDocumentsAndEditors, extHostLogService)); const extHostDocumentSaveParticipant = rpcProtocol.set(ExtHostContext.ExtHostDocumentSaveParticipant, new ExtHostDocumentSaveParticipant(extHostLogService, extHostDocuments, rpcProtocol.getProxy(MainContext.MainThreadBulkEdits))); - const extHostNotebook = rpcProtocol.set(ExtHostContext.ExtHostNotebook, new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments, extHostConsumerFileSystem, extHostSearch)); + const extHostNotebook = rpcProtocol.set(ExtHostContext.ExtHostNotebook, new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments, extHostConsumerFileSystem, extHostSearch, extHostLogService)); const extHostNotebookDocuments = rpcProtocol.set(ExtHostContext.ExtHostNotebookDocuments, new ExtHostNotebookDocuments(extHostNotebook)); const extHostNotebookEditors = rpcProtocol.set(ExtHostContext.ExtHostNotebookEditors, new ExtHostNotebookEditors(extHostLogService, extHostNotebook)); const extHostNotebookKernels = rpcProtocol.set(ExtHostContext.ExtHostNotebookKernels, new ExtHostNotebookKernels(rpcProtocol, initData, extHostNotebook, extHostCommands, extHostLogService)); diff --git a/src/vs/workbench/api/common/extHostDocumentContentProviders.ts b/src/vs/workbench/api/common/extHostDocumentContentProviders.ts index daca96f1913..9ea63aa8eb0 100644 --- a/src/vs/workbench/api/common/extHostDocumentContentProviders.ts +++ b/src/vs/workbench/api/common/extHostDocumentContentProviders.ts @@ -37,6 +37,11 @@ export class ExtHostDocumentContentProvider implements ExtHostDocumentContentPro throw new Error(`scheme '${scheme}' already registered`); } + this._logService.warn('TEST WARNING'); + this._logService.error('TEST ERROR'); + this._logService.info('TEST INFO'); + this._logService.trace('TEST TRACE'); + const handle = ExtHostDocumentContentProvider._handlePool++; this._documentContentProviders.set(handle, provider); diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index 29e72e73a07..0657c297678 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -37,6 +37,7 @@ import { CellSearchModel } from 'vs/workbench/contrib/search/common/cellSearchMo import { INotebookCellMatchNoModel, INotebookFileMatchNoModel, IRawClosedNotebookFileMatch, genericCellMatchesToTextSearchMatches } from 'vs/workbench/contrib/search/common/searchNotebookHelpers'; import { NotebookPriorityInfo } from 'vs/workbench/contrib/search/common/search'; import { globMatchesResource } from 'vs/workbench/services/editor/common/editorResolverService'; +import { ILogService } from 'vs/platform/log/common/log'; export class ExtHostNotebookController implements ExtHostNotebookShape { private static _notebookStatusBarItemProviderHandlePool: number = 0; @@ -78,7 +79,8 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { private _textDocumentsAndEditors: ExtHostDocumentsAndEditors, private _textDocuments: ExtHostDocuments, private _extHostFileSystem: IExtHostConsumerFileSystem, - private _extHostSearch: IExtHostSearch + private _extHostSearch: IExtHostSearch, + private _logService: ILogService ) { this._notebookProxy = mainContext.getProxy(MainContext.MainThreadNotebook); this._notebookDocumentsProxy = mainContext.getProxy(MainContext.MainThreadNotebookDocuments); @@ -314,6 +316,8 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { async $saveNotebook(handle: number, uriComponents: UriComponents, versionId: number, options: files.IWriteFileOptions, token: CancellationToken): Promise { const uri = URI.revive(uriComponents); const serializer = this._notebookSerializer.get(handle); + this.trace(`enter saveNotebook(versionId: ${versionId}, ${uri.toString()})`); + if (!serializer) { throw new Error('NO serializer found'); } @@ -357,7 +361,9 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { await this._validateWriteFile(uri, options); const bytes = await serializer.serializer.serializeNotebook(data, token); + this.trace(`serialized versionId: ${versionId} ${uri.toString()}`); await this._extHostFileSystem.value.writeFile(uri, bytes); + this.trace(`Finished write versionId: ${versionId} ${uri.toString()}`); const providerExtUri = this._extHostFileSystem.getFileSystemProviderExtUri(uri.scheme); const stat = await this._extHostFileSystem.value.stat(uri); @@ -375,6 +381,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { children: undefined }; + this.trace(`exit saveNotebook(versionId: ${versionId}, ${uri.toString()})`); return fileStats; } @@ -718,4 +725,8 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { extHostCommands.registerApiCommand(commandDataToNotebook); extHostCommands.registerApiCommand(commandNotebookToData); } + + private trace(msg: string): void { + this._logService.trace(`[Extension Host Notebook] ${msg}`); + } } diff --git a/src/vs/workbench/api/test/browser/extHostNotebook.test.ts b/src/vs/workbench/api/test/browser/extHostNotebook.test.ts index 806a6b685dd..5a7ed7e4e70 100644 --- a/src/vs/workbench/api/test/browser/extHostNotebook.test.ts +++ b/src/vs/workbench/api/test/browser/extHostNotebook.test.ts @@ -66,7 +66,7 @@ suite('NotebookCell#Document', function () { override onExtensionError(): boolean { return true; } - }), extHostDocumentsAndEditors, extHostDocuments, extHostConsumerFileSystem, extHostSearch); + }), extHostDocumentsAndEditors, extHostDocuments, extHostConsumerFileSystem, extHostSearch, new NullLogService()); extHostNotebookDocuments = new ExtHostNotebookDocuments(extHostNotebooks); const reg = extHostNotebooks.registerNotebookSerializer(nullExtensionDescription, 'test', new class extends mock() { }); diff --git a/src/vs/workbench/api/test/browser/extHostNotebookKernel.test.ts b/src/vs/workbench/api/test/browser/extHostNotebookKernel.test.ts index 8af3ece68d9..5a7e6f434c2 100644 --- a/src/vs/workbench/api/test/browser/extHostNotebookKernel.test.ts +++ b/src/vs/workbench/api/test/browser/extHostNotebookKernel.test.ts @@ -105,7 +105,7 @@ suite('NotebookKernel', function () { }); extHostConsumerFileSystem = new ExtHostConsumerFileSystem(rpcProtocol, new ExtHostFileSystemInfo()); extHostSearch = new ExtHostSearch(rpcProtocol, new URITransformerService(null), new NullLogService()); - extHostNotebooks = new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments, extHostConsumerFileSystem, extHostSearch); + extHostNotebooks = new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors, extHostDocuments, extHostConsumerFileSystem, extHostSearch, new NullLogService()); extHostNotebookDocuments = new ExtHostNotebookDocuments(extHostNotebooks); diff --git a/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts b/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts index 5974292f2e5..3c26eff229a 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts @@ -250,17 +250,17 @@ export class NotebookFileWorkingCopyModel extends Disposable implements IStoredF if (!token.isCancellationRequested) { type notebookSaveErrorData = { isRemote: boolean; - versionMismatch: boolean; + error: Error; }; type notebookSaveErrorClassification = { owner: 'amunger'; comment: 'Detect if we are having issues saving a notebook on the Extension Host'; isRemote: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; comment: 'Whether the save is happening on a remote file system' }; - versionMismatch: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; comment: 'If the error was because of a version mismatch' }; + error: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; comment: 'Info about the error that occurred' }; }; this._telemetryService.publicLogError2('notebook/SaveError', { isRemote: this._notebookModel.uri.scheme === Schemas.vscodeRemote, - versionMismatch: error instanceof Error && error.message === 'Document version mismatch' + error: error }); } diff --git a/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts b/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts index 2fd6bad674e..f6a173613ac 100644 --- a/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts +++ b/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts @@ -35,6 +35,16 @@ import { IMarkdownString } from 'vs/base/common/htmlContent'; */ export interface IStoredFileWorkingCopyModelFactory extends IFileWorkingCopyModelFactory { } +export async function createOptionalResult(callback: (token: CancellationToken) => Promise, token: CancellationToken): Promise { + const result = await callback(token); + if (result === undefined && token.isCancellationRequested) { + return undefined; + } + else { + return assertIsDefined(result); + } +} + /** * The underlying model of a stored file working copy provides some * methods for the stored file working copy to function. The model is