Merge pull request #232 from jitseniesen/kernel-id

PR: Make .get_kernel_id() more robust

spyder_notebook/widgets/client.py

@@ -34,7 +34,6 @@
 # Local imports
 from ..widgets.dom import DOMWidget
 
-
 # -----------------------------------------------------------------------------
 # Templates
 # -----------------------------------------------------------------------------
@@ -226,11 +225,26 @@ def get_kernel_id(self):
         """
         Get the kernel id of the client.
 
-        Return a str with the kernel id or None.
+        Return a str with the kernel id or None. On error, display a dialog
+        box and return None.
         """
         sessions_url = self.get_session_url()
-        sessions_req = requests.get(sessions_url).content.decode()
-        sessions = json.loads(sessions_req)
+        try:
+            sessions_response = requests.get(sessions_url)
+        except requests.exceptions.RequestException as exception:
+            msg = _('Spyder could not get a list of sessions '
+                    'from the Jupyter Notebook server. '
+                    'Message: {}').format(exception)
+            QMessageBox.warning(self, _('Server error'), msg)
+            return None
+
+        sessions = json.loads(sessions_response.content.decode())
+        if sessions_response.status_code != requests.codes.ok:
+            msg = _('Spyder could not get a list of sessions '
+                    'from the Jupyter Notebook server. '
+                    'Message: {}').format(sessions.get('message'))
+            QMessageBox.warning(self, _('Server error'), msg)
+            return None
 
         if os.name == 'nt':
             path = self.path.replace('\\', '/')

spyder_notebook/widgets/tests/test_client.py

@@ -8,6 +8,7 @@
 # Third-party imports
 import pytest
 from qtpy.QtWidgets import QWidget
+import requests
 
 # Local imports
 from spyder_notebook.widgets.client import NotebookClient
@@ -17,38 +18,70 @@ class MockPlugin(QWidget):
     def get_plugin_actions(self):
         return []
 
-
 @pytest.fixture
-def client(qtbot):
-    """Construct a NotebookClient for use in tests."""
+def plugin(qtbot):
+    """
+    Construct mock plugin with NotebookClient for use in tests.
+
+    Use `plugin.client` to access the client.
+    """
     plugin = MockPlugin()
+    qtbot.addWidget(plugin)
     client = NotebookClient(plugin, '/path/notebooks/ham.ipynb')
+    plugin.client = client
     server_info = {'notebook_dir': '/path/notebooks',
                    'url': 'fake_url',
                    'token': 'fake_token'}
     client.register(server_info)
-    return client
+    return plugin
 
 
-def test_notebookclient_get_kernel_id(client, mocker):
+def test_notebookclient_get_kernel_id(plugin, mocker):
     """Basic unit test for NotebookClient.get_kernel_id()."""
     response = mocker.Mock()
     content = b'[{"kernel": {"id": "42"}, "notebook": {"path": "ham.ipynb"}}]'
     response.content = content
+    response.status_code = requests.codes.ok
     mocker.patch('requests.get', return_value=response)
 
-    kernel_id = client.get_kernel_id()
+    kernel_id = plugin.client.get_kernel_id()
     assert kernel_id == '42'
 
 
-def test_notebookclient_get_kernel_id_with_fields_missing(client, mocker):
+def test_notebookclient_get_kernel_id_with_fields_missing(plugin, mocker):
     """Test NotebookClient.get_kernel_id() if response has fields missing."""
     response = mocker.Mock()
     content = (b'[{"kernel": {"id": "1"}, "notebook": {"spam": "eggs"}},'
                b' {"kernel": {"id": "2"}},'
                b' {"kernel": {"id": "3"}, "notebook": {"path": "ham.ipynb"}}]')
     response.content = content
+    response.status_code = requests.codes.ok
     mocker.patch('requests.get', return_value=response)
 
-    kernel_id = client.get_kernel_id()
+    kernel_id = plugin.client.get_kernel_id()
     assert kernel_id == '3'
+
+
+def test_notebookclient_get_kernel_id_with_error_status(plugin, mocker):
+    """Test NotebookClient.get_kernel_id() when response has error status."""
+    response = mocker.Mock()
+    content = b'{"message": "error"}'
+    response.content = content
+    response.status_code = requests.codes.forbidden
+    mocker.patch('requests.get', return_value=response)
+    MockMessageBox = mocker.patch('spyder_notebook.widgets.client.QMessageBox')
+
+    plugin.client.get_kernel_id()
+
+    MockMessageBox.warning.assert_called()
+
+
+def test_notebookclient_get_kernel_id_with_exception(plugin, mocker):
+    """Test NotebookClient.get_kernel_id() when request raises an exception."""
+    exception = requests.exceptions.ProxyError('kaboom')
+    mocker.patch('requests.get', side_effect=exception)
+    MockMessageBox = mocker.patch('spyder_notebook.widgets.client.QMessageBox')
+
+    plugin.client.get_kernel_id()
+
+    MockMessageBox.warning.assert_called()