add defensive handling of vscode api proposal usage (#174)

* add defensive handling of vscode api proposal usage

* add tests

* fix test

Author: jankuca

src/notebooks/controllers/vscodeNotebookController.ts

@@ -173,10 +173,19 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont
             displayDataProvider
         );
 
-        try {
-            controller.controller.variableProvider = jupyterVairablesProvider;
-        } catch (ex) {
-            logger.warn('Failed to attach variable provider', ex);
+        // Only attach variable provider if the API is available
+        // The notebookVariableProvider is a proposed API that:
+        // - Works in extension development mode (F5 debugging)
+        // - Does NOT work in published extensions from the Marketplace
+        // - Requires users to manually enable it with --enable-proposed-api flag
+        // See: https://code.visualstudio.com/api/advanced-topics/using-proposed-api
+        // This check allows the extension to gracefully degrade when the API is unavailable
+        if ('variableProvider' in controller.controller) {
+            try {
+                controller.controller.variableProvider = jupyterVairablesProvider;
+            } catch (ex) {
+                logger.warn('Failed to attach variable provider', ex);
+            }
         }
 
         return controller;

src/notebooks/controllers/vscodeNotebookController.unit.test.ts

@@ -37,11 +37,12 @@ import { KernelConnector } from './kernelConnector';
 import { ITrustedKernelPaths } from '../../kernels/raw/finder/types';
 import { IInterpreterService } from '../../platform/interpreter/contracts';
 import { PythonEnvironment } from '../../platform/pythonEnvironments/info';
-import { IConnectionDisplayDataProvider } from './types';
+import { IConnectionDisplayData, IConnectionDisplayDataProvider } from './types';
 import { ConnectionDisplayDataProvider } from './connectionDisplayData.node';
 import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock';
 import { Environment, PythonExtension } from '@vscode/python-extension';
 import { crateMockedPythonApi, whenResolveEnvironment } from '../../kernels/helpers.unit.test';
+import { IJupyterVariablesProvider } from '../../kernels/variables/types';
 
 suite(`Notebook Controller`, function () {
     let controller: NotebookController;
@@ -544,4 +545,215 @@ suite(`Notebook Controller`, function () {
             });
         });
     });
+
+    suite('VSCodeNotebookController.create', function () {
+        let kernelConnection: KernelConnectionMetadata;
+        let kernelProvider: IKernelProvider;
+        let context: IExtensionContext;
+        let languageService: NotebookCellLanguageService;
+        let configService: IConfigurationService;
+        let extensionChecker: IPythonExtensionChecker;
+        let serviceContainer: IServiceContainer;
+        let displayDataProvider: IConnectionDisplayDataProvider;
+        let jupyterVariablesProvider: IJupyterVariablesProvider;
+        let disposables: IDisposable[] = [];
+        let controller: NotebookController;
+        let onDidChangeSelectedNotebooks: EventEmitter<{
+            readonly notebook: NotebookDocument;
+            readonly selected: boolean;
+        }>;
+
+        setup(function () {
+            resetVSCodeMocks();
+            disposables.push(new Disposable(() => resetVSCodeMocks()));
+            kernelConnection = mock<KernelConnectionMetadata>();
+            kernelProvider = mock<IKernelProvider>();
+            context = mock<IExtensionContext>();
+            languageService = mock<NotebookCellLanguageService>();
+            configService = mock<IConfigurationService>();
+            extensionChecker = mock<IPythonExtensionChecker>();
+            serviceContainer = mock<IServiceContainer>();
+            displayDataProvider = mock<IConnectionDisplayDataProvider>();
+            jupyterVariablesProvider = mock<IJupyterVariablesProvider>();
+            controller = mock<NotebookController>();
+            onDidChangeSelectedNotebooks = new EventEmitter<{
+                readonly notebook: NotebookDocument;
+                readonly selected: boolean;
+            }>();
+            disposables.push(onDidChangeSelectedNotebooks);
+
+            when(context.extensionUri).thenReturn(Uri.file('extension'));
+            when(controller.onDidChangeSelectedNotebooks).thenReturn(onDidChangeSelectedNotebooks.event);
+            when(displayDataProvider.getDisplayData(anything())).thenReturn({
+                label: 'Test Kernel',
+                description: 'Test Description',
+                detail: 'Test Detail',
+                category: 'Test Category',
+                serverDisplayName: 'Test Server',
+                onDidChange: new EventEmitter<IConnectionDisplayData>().event,
+                dispose: () => {
+                    /* noop */
+                }
+            });
+            when(
+                mockedVSCodeNamespaces.notebooks.createNotebookController(
+                    anything(),
+                    anything(),
+                    anything(),
+                    anything(),
+                    anything()
+                )
+            ).thenReturn(instance(controller));
+        });
+
+        teardown(() => (disposables = dispose(disposables)));
+
+        test('Should attach variable provider when API is available', function () {
+            // Arrange: Mock controller with variableProvider property
+            const controllerWithApi = mock<NotebookController>();
+            when(controllerWithApi.onDidChangeSelectedNotebooks).thenReturn(onDidChangeSelectedNotebooks.event);
+            (instance(controllerWithApi) as any).variableProvider = undefined;
+
+            when(
+                mockedVSCodeNamespaces.notebooks.createNotebookController(
+                    anything(),
+                    anything(),
+                    anything(),
+                    anything(),
+                    anything()
+                )
+            ).thenReturn(instance(controllerWithApi));
+
+            // Act
+            const result = VSCodeNotebookController.create(
+                instance(kernelConnection),
+                'test-id',
+                'jupyter-notebook',
+                instance(kernelProvider),
+                instance(context),
+                disposables,
+                instance(languageService),
+                instance(configService),
+                instance(extensionChecker),
+                instance(serviceContainer),
+                instance(displayDataProvider),
+                instance(jupyterVariablesProvider)
+            );
+
+            // Assert
+            assert.isDefined(result);
+            assert.strictEqual(
+                (result.controller as any).variableProvider,
+                instance(jupyterVariablesProvider),
+                'Variable provider should be attached when API is available'
+            );
+        });
+
+        test('Should not attach variable provider when API is not available', function () {
+            // Arrange: Create a plain object without variableProvider property
+            const controllerWithoutApi = {
+                onDidChangeSelectedNotebooks: onDidChangeSelectedNotebooks.event,
+                id: 'test-id',
+                notebookType: 'jupyter-notebook',
+                supportedLanguages: [],
+                supportsExecutionOrder: true,
+                description: '',
+                detail: '',
+                label: 'Test Kernel',
+                dispose: () => {
+                    /* noop */
+                },
+                createNotebookCellExecution: () => ({}) as any,
+                createNotebookExecution: () => ({}) as any,
+                executeHandler: () => {
+                    /* noop */
+                },
+                interruptHandler: undefined,
+                updateNotebookAffinity: () => {
+                    /* noop */
+                },
+                rendererScripts: [],
+                onDidReceiveMessage: new EventEmitter<any>().event,
+                postMessage: () => Promise.resolve(true),
+                asWebviewUri: (uri: Uri) => uri
+                // Note: no variableProvider property to simulate API not being available
+            } as NotebookController;
+
+            when(
+                mockedVSCodeNamespaces.notebooks.createNotebookController(
+                    anything(),
+                    anything(),
+                    anything(),
+                    anything(),
+                    anything()
+                )
+            ).thenReturn(controllerWithoutApi);
+
+            // Act
+            const result = VSCodeNotebookController.create(
+                instance(kernelConnection),
+                'test-id',
+                'jupyter-notebook',
+                instance(kernelProvider),
+                instance(context),
+                disposables,
+                instance(languageService),
+                instance(configService),
+                instance(extensionChecker),
+                instance(serviceContainer),
+                instance(displayDataProvider),
+                instance(jupyterVariablesProvider)
+            );
+
+            // Assert
+            assert.isDefined(result);
+            assert.isFalse(
+                'variableProvider' in result.controller,
+                'Variable provider property should not exist when API is not available'
+            );
+        });
+
+        test('Should handle errors when attaching variable provider', function () {
+            // Arrange: Mock controller that throws when setting variableProvider
+            const controllerWithError = mock<NotebookController>();
+            when(controllerWithError.onDidChangeSelectedNotebooks).thenReturn(onDidChangeSelectedNotebooks.event);
+
+            const controllerInstance = instance(controllerWithError);
+            Object.defineProperty(controllerInstance, 'variableProvider', {
+                set: () => {
+                    throw new Error('API not supported');
+                },
+                configurable: true
+            });
+
+            when(
+                mockedVSCodeNamespaces.notebooks.createNotebookController(
+                    anything(),
+                    anything(),
+                    anything(),
+                    anything(),
+                    anything()
+                )
+            ).thenReturn(controllerInstance);
+
+            // Act - should not throw
+            const result = VSCodeNotebookController.create(
+                instance(kernelConnection),
+                'test-id',
+                'jupyter-notebook',
+                instance(kernelProvider),
+                instance(context),
+                disposables,
+                instance(languageService),
+                instance(configService),
+                instance(extensionChecker),
+                instance(serviceContainer),
+                instance(displayDataProvider),
+                instance(jupyterVariablesProvider)
+            );
+
+            // Assert
+            assert.isDefined(result);
+        });
+    });
 });