Forward errors when performing View actions (#4122)

This PR addresses #3653 by
forwarding the error when opening the Viewer to the user (using a
notification) and suggesting to restart the session. It will also
forward possible errors when performing `view` on Connections objects.

extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_variables.py

@@ -696,6 +696,46 @@ def test_view_error(variables_comm: DummyComm) -> None:
     ]
 
 
+def test_view_error_when_pandas_not_loaded(
+    shell: PositronShell, variables_comm: DummyComm, mock_dataexplorer_service: Mock
+) -> None:
+    # regression test for https://github.com/posit-dev/positron/issues/3653
+    shell.user_ns["x"] = pd.DataFrame({"a": [0]})
+
+    # Cases where the object has a viewer action, but no service reports it as
+    # supported.
+    def not_supported(value):
+        return False
+
+    mock_dataexplorer_service.is_supported = not_supported
+
+    path = _encode_path(["x"])
+    msg = json_rpc_request("view", {"path": path}, comm_id="dummy_comm_id")
+    variables_comm.handle_msg(msg, raise_errors=False)
+
+    assert variables_comm.messages == [
+        json_rpc_error(
+            JsonRpcErrorCode.INTERNAL_ERROR,
+            f"Error opening viewer for variable at '{path}'. Object not supported. Try restarting the session.",
+        )
+    ]
+
+    # Case where the object has a viewer, but somehting wrong happens when checking
+    # if the object is supported.
+    def fail_is_supported(value):
+        raise TypeError("Not supported")
+
+    mock_dataexplorer_service.is_supported = fail_is_supported
+    variables_comm.handle_msg(msg, raise_errors=False)
+
+    assert [variables_comm.messages[-1]] == [
+        json_rpc_error(
+            JsonRpcErrorCode.INTERNAL_ERROR,
+            f"Error opening viewer for variable at '{path}'. Try restarting the session.",
+        )
+    ]
+
+
 def test_unknown_method(variables_comm: DummyComm) -> None:
     msg = json_rpc_request("unknown_method", comm_id="dummy_comm_id")
     variables_comm.handle_msg(msg, raise_errors=False)

extensions/positron-python/python_files/positron/positron_ipykernel/variables.py

@@ -576,10 +576,22 @@ def _perform_view_action(self, path: List[str]) -> None:
                 f"Cannot find variable at '{path}' to view",
             )
 
-        if self.kernel.connections_service.object_is_supported(value):
-            self._open_connections_pane(path, value)
-        elif self.kernel.data_explorer_service.is_supported(value):
-            self._open_data_explorer(path, value)
+        try:
+            if self.kernel.connections_service.object_is_supported(value):
+                self._open_connections_pane(path, value)
+            elif self.kernel.data_explorer_service.is_supported(value):
+                self._open_data_explorer(path, value)
+            else:
+                self._send_error(
+                    JsonRpcErrorCode.INTERNAL_ERROR,
+                    f"Error opening viewer for variable at '{path}'. Object not supported. Try restarting the session.",
+                )
+        except Exception as err:
+            self._send_error(
+                JsonRpcErrorCode.INTERNAL_ERROR,
+                f"Error opening viewer for variable at '{path}'. Try restarting the session.",
+            )
+            logger.error(err, exc_info=True)
 
     def _open_data_explorer(self, path: List[str], value: Any) -> None:
         """Opens a DataExplorer comm for the variable at the requested

src/vs/workbench/contrib/positronVariables/browser/components/variableItem.tsx

@@ -123,7 +123,15 @@ export const VariableItem = (props: VariableItemProps) => {
 			instance.requestFocus();
 		} else {
 			// Open a viewer for the variable item.
-			const viewerId = await item.view();
+			let viewerId: string | undefined;
+			try {
+				viewerId = await item.view();
+			} catch (err) {
+				positronVariablesContext.notificationService.error(localize(
+					'positron.variables.viewerError',
+					"An error occurred while opening the viewer. Try restarting your session."
+				));
+			}
 
 			// If a binding was returned, save the binding between the viewer and the variable item.
 			if (viewerId) {

src/vs/workbench/contrib/positronVariables/browser/positronVariablesState.tsx

@@ -13,6 +13,7 @@ import { IPositronVariablesInstance } from 'vs/workbench/services/positronVariab
 import { PositronActionBarServices } from 'vs/platform/positronActionBar/browser/positronActionBarState';
 import { IRuntimeSessionService } from 'vs/workbench/services/runtimeSession/common/runtimeSessionService';
 import { IPositronDataExplorerService } from 'vs/workbench/services/positronDataExplorer/browser/interfaces/positronDataExplorerService';
+import { INotificationService } from 'vs/platform/notification/common/notification';
 
 /**
  * PositronVariablesServices interface.
@@ -24,6 +25,7 @@ export interface PositronVariablesServices extends PositronActionBarServices {
 	readonly positronVariablesService: IPositronVariablesService;
 	readonly reactComponentContainer: IReactComponentContainer;
 	readonly dataExplorerService: IPositronDataExplorerService;
+	readonly notificationService: INotificationService;
 }
 
 /**

src/vs/workbench/contrib/positronVariables/browser/positronVariablesView.tsx

@@ -29,6 +29,7 @@ import { IRuntimeSessionService } from 'vs/workbench/services/runtimeSession/com
 import { PositronViewPane } from 'vs/workbench/browser/positronViewPane/positronViewPane';
 import { IHoverService } from 'vs/platform/hover/browser/hover';
 import { IPositronDataExplorerService } from 'vs/workbench/services/positronDataExplorer/browser/interfaces/positronDataExplorerService';
+import { INotificationService } from 'vs/platform/notification/common/notification';
 
 /**
  * PositronVariablesViewPane class.
@@ -198,7 +199,8 @@ export class PositronVariablesViewPane extends PositronViewPane implements IReac
 		@ITelemetryService telemetryService: ITelemetryService,
 		@IThemeService themeService: IThemeService,
 		@IViewDescriptorService viewDescriptorService: IViewDescriptorService,
-		@IWorkbenchLayoutService private readonly _layoutService: IWorkbenchLayoutService
+		@IWorkbenchLayoutService private readonly _layoutService: IWorkbenchLayoutService,
+		@INotificationService private readonly _notificationService: INotificationService
 	) {
 		// Call the base class's constructor.
 		super(
@@ -268,6 +270,7 @@ export class PositronVariablesViewPane extends PositronViewPane implements IReac
 				contextKeyService={this.contextKeyService}
 				contextMenuService={this.contextMenuService}
 				dataExplorerService={this._dataExplorerService}
+				notificationService={this._notificationService}
 				hoverService={this.hoverService}
 				keybindingService={this.keybindingService}
 				runtimeSessionService={this._runtimeSessionService}