add logs for failed reverts

Author: jankuca

src/notebooks/deepnote/deepnoteActivationService.ts

@@ -2,6 +2,7 @@ import { injectable, inject } from 'inversify';
 import { workspace } from 'vscode';
 import { IExtensionSyncActivationService } from '../../platform/activation/types';
 import { IExtensionContext } from '../../platform/common/types';
+import { ILogger } from '../../platform/logging/types';
 import { IDeepnoteNotebookManager } from '../types';
 import { DeepnoteNotebookSerializer } from './deepnoteSerializer';
 import { DeepnoteExplorerView } from './deepnoteExplorerView';
@@ -25,7 +26,8 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic
     constructor(
         @inject(IExtensionContext) private extensionContext: IExtensionContext,
         @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager,
-        @inject(IIntegrationManager) integrationManager: IIntegrationManager
+        @inject(IIntegrationManager) integrationManager: IIntegrationManager,
+        @inject(ILogger) private readonly logger: ILogger
     ) {
         this.integrationManager = integrationManager;
     }
@@ -37,7 +39,7 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic
     public activate() {
         this.serializer = new DeepnoteNotebookSerializer(this.notebookManager);
         this.explorerView = new DeepnoteExplorerView(this.extensionContext, this.notebookManager);
-        this.editProtection = new DeepnoteInputBlockEditProtection();
+        this.editProtection = new DeepnoteInputBlockEditProtection(this.logger);
 
         this.extensionContext.subscriptions.push(workspace.registerNotebookSerializer('deepnote', this.serializer));
         this.extensionContext.subscriptions.push(this.editProtection);

src/notebooks/deepnote/deepnoteActivationService.unit.test.ts

@@ -3,13 +3,15 @@ import { assert } from 'chai';
 import { DeepnoteActivationService } from './deepnoteActivationService';
 import { DeepnoteNotebookManager } from './deepnoteNotebookManager';
 import { IExtensionContext } from '../../platform/common/types';
+import { ILogger } from '../../platform/logging/types';
 import { IIntegrationManager } from './integrations/types';
 
 suite('DeepnoteActivationService', () => {
     let activationService: DeepnoteActivationService;
     let mockExtensionContext: IExtensionContext;
     let manager: DeepnoteNotebookManager;
     let mockIntegrationManager: IIntegrationManager;
+    let mockLogger: ILogger;
 
     setup(() => {
         mockExtensionContext = {
@@ -22,7 +24,20 @@ suite('DeepnoteActivationService', () => {
                 return;
             }
         };
-        activationService = new DeepnoteActivationService(mockExtensionContext, manager, mockIntegrationManager);
+        mockLogger = {
+            error: () => {},
+            warn: () => {},
+            info: () => {},
+            debug: () => {},
+            trace: () => {},
+            ci: () => {}
+        } as ILogger;
+        activationService = new DeepnoteActivationService(
+            mockExtensionContext,
+            manager,
+            mockIntegrationManager,
+            mockLogger
+        );
     });
 
     suite('constructor', () => {
@@ -92,8 +107,24 @@ suite('DeepnoteActivationService', () => {
                     return;
                 }
             };
-            const service1 = new DeepnoteActivationService(context1, manager1, mockIntegrationManager1);
-            const service2 = new DeepnoteActivationService(context2, manager2, mockIntegrationManager2);
+            const mockLogger1: ILogger = {
+                error: () => {},
+                warn: () => {},
+                info: () => {},
+                debug: () => {},
+                trace: () => {},
+                ci: () => {}
+            } as ILogger;
+            const mockLogger2: ILogger = {
+                error: () => {},
+                warn: () => {},
+                info: () => {},
+                debug: () => {},
+                trace: () => {},
+                ci: () => {}
+            } as ILogger;
+            const service1 = new DeepnoteActivationService(context1, manager1, mockIntegrationManager1, mockLogger1);
+            const service2 = new DeepnoteActivationService(context2, manager2, mockIntegrationManager2, mockLogger2);
 
             // Verify each service has its own context
             assert.strictEqual((service1 as any).extensionContext, context1);
@@ -128,8 +159,24 @@ suite('DeepnoteActivationService', () => {
                     return;
                 }
             };
-            new DeepnoteActivationService(context1, manager1, mockIntegrationManager1);
-            new DeepnoteActivationService(context2, manager2, mockIntegrationManager2);
+            const mockLogger3: ILogger = {
+                error: () => {},
+                warn: () => {},
+                info: () => {},
+                debug: () => {},
+                trace: () => {},
+                ci: () => {}
+            } as ILogger;
+            const mockLogger4: ILogger = {
+                error: () => {},
+                warn: () => {},
+                info: () => {},
+                debug: () => {},
+                trace: () => {},
+                ci: () => {}
+            } as ILogger;
+            new DeepnoteActivationService(context1, manager1, mockIntegrationManager1, mockLogger3);
+            new DeepnoteActivationService(context2, manager2, mockIntegrationManager2, mockLogger4);
 
             assert.strictEqual(context1.subscriptions.length, 0);
             assert.strictEqual(context2.subscriptions.length, 1);

src/notebooks/deepnote/deepnoteInputBlockEditProtection.ts

@@ -1,3 +1,4 @@
+import { injectable, inject } from 'inversify';
 import {
     Disposable,
     NotebookCell,
@@ -10,13 +11,15 @@ import {
     workspace,
     WorkspaceEdit
 } from 'vscode';
+import { ILogger } from '../../platform/logging/types';
 import { formatInputBlockCellContent } from './inputBlockContentFormatter';
 
 /**
  * Protects readonly input blocks from being edited by reverting changes.
  * Also protects the language ID of all input blocks.
  * This is needed because VSCode doesn't support the `editable: false` metadata property.
  */
+@injectable()
 export class DeepnoteInputBlockEditProtection implements Disposable {
     private readonly disposables: Disposable[] = [];
 
@@ -55,7 +58,7 @@ export class DeepnoteInputBlockEditProtection implements Disposable {
         ['button', 'python']
     ]);
 
-    constructor() {
+    constructor(@inject(ILogger) private readonly logger: ILogger) {
         // Listen for notebook document changes
         this.disposables.push(
             workspace.onDidChangeNotebookDocument((e) => {
@@ -125,7 +128,14 @@ export class DeepnoteInputBlockEditProtection implements Disposable {
                 new Position(lastLine, cell.document.lineAt(lastLine).text.length)
             );
             edit.replace(cell.document.uri, fullRange, correctContent);
-            await workspace.applyEdit(edit);
+            const success = await workspace.applyEdit(edit);
+            if (!success) {
+                this.logger.error(
+                    `Failed to revert cell content for input block type '${blockType}' at cell index ${
+                        cell.index
+                    } in notebook ${cell.notebook.uri.toString()}`
+                );
+            }
         }
     }
 
@@ -168,7 +178,15 @@ export class DeepnoteInputBlockEditProtection implements Disposable {
             workspaceEdit.set(uri, edits);
         }
 
-        await workspace.applyEdit(workspaceEdit);
+        const success = await workspace.applyEdit(workspaceEdit);
+        if (!success) {
+            const cellInfo = cellsToFix
+                .map(({ cell, blockType }) => `cell ${cell.index} (type: ${blockType})`)
+                .join(', ');
+            this.logger.error(
+                `Failed to protect cell languages for ${cellsToFix.length} cell(s): ${cellInfo} in notebook(s)`
+            );
+        }
     }
 
     dispose(): void {