Merge pull request #147 from minrk/failed-spawn

prevent inconsistent state if Spawner.start fails

jupyterhub/apihandlers/users.py

@@ -14,12 +14,18 @@ from .base import APIHandler
 class BaseUserHandler(APIHandler):
     
     def user_model(self, user):
-        return {
+        model = {
             'name': user.name,
             'admin': user.admin,
-            'server': user.server.base_url if user.server and not (user.spawn_pending or user.stop_pending) else None,
+            'server': user.server.base_url if user.running else None,
+            'pending': None,
             'last_activity': user.last_activity.isoformat(),
         }
+        if user.spawn_pending:
+            model['pending'] = 'spawn'
+        elif user.stop_pending:
+            model['pending'] = 'stop'
+        return model
     
     _model_types = {
         'name': str,
@@ -150,7 +156,7 @@ class UserServerAPIHandler(BaseUserHandler):
         if user.stop_pending:
             self.set_status(202)
             return
-        if user.spawner is None:
+        if not user.running:
             raise web.HTTPError(400, "%s's server is not running" % name)
         status = yield user.spawner.poll()
         if status is not None:
@@ -175,8 +181,8 @@ class UserAdminAccessAPIHandler(BaseUserHandler):
         user = self.find_user(name)
         if user is None:
             raise web.HTTPError(404)
-        if user.server is None:
+        if not user.running:
-            raise web.HTTPError(400, "%s has no server running" % name)
+            raise web.HTTPError(400, "%s's server is not running" % name)
         self.set_server_cookie(user)
 
 

jupyterhub/orm.py

@@ -267,6 +267,15 @@ class User(Base):
                 name=self.name,
             )
     
+    @property
+    def running(self):
+        """property for whether a user has a running server"""
+        if self.spawner is None:
+            return False
+        if self.server is None:
+            return False
+        return True
+    
     def new_api_token(self):
         """Create a new API token"""
         assert self.id is not None
@@ -302,7 +311,6 @@ class User(Base):
         api_token = self.new_api_token()
         db.commit()
         
-        
         spawner = self.spawner = spawner_class(
             config=config,
             user=self,
@@ -314,21 +322,26 @@ class User(Base):
         spawner.api_token = api_token
         
         self.spawn_pending = True
-        f = spawner.start()
         # wait for spawner.start to return
         try:
+            f = spawner.start()
             yield gen.with_timeout(timedelta(seconds=spawner.start_timeout), f)
-        except gen.TimeoutError as e:
+        except Exception as e:
-            self.log.warn("{user}'s server failed to start in {s} seconds, giving up".format(
+            if isinstance(e, gen.TimeoutError):
-                user=self.name, s=spawner.start_timeout,
+                self.log.warn("{user}'s server failed to start in {s} seconds, giving up".format(
-            ))
+                    user=self.name, s=spawner.start_timeout,
+                ))
+            else:
+                self.log.error("Unhandled error starting {user}'s server: {error}".format(
+                    user=self.name, error=e,
+                ))
             try:
                 yield self.stop()
             except Exception:
                 self.log.error("Failed to cleanup {user}'s server that failed to start".format(
                     user=self.name,
                 ), exc_info=True)
-            # raise original TimeoutError
+            # raise original exception
             raise e
         spawner.start_polling()
 
@@ -338,10 +351,15 @@ class User(Base):
         db.commit()
         try:
             yield self.server.wait_up(http=True)
-        except TimeoutError as e:
+        except Exception as e:
-            self.log.warn("{user}'s server never showed up at {url}, giving up".format(
+            if isinstance(e, TimeoutError):
-                user=self.name, url=self.server.url,
+                self.log.warn("{user}'s server never showed up at {url}, giving up".format(
-            ))
+                    user=self.name, url=self.server.url,
+                ))
+            else:
+                self.log.error("Unhandled error waiting for {user}'s server to show up at {url}: {error}".format(
+                    user=self.name, url=self.server.url, error=e,
+                ))
             try:
                 yield self.stop()
             except Exception:

jupyterhub/tests/test_api.py

@@ -104,11 +104,13 @@ def test_get_users(app):
             'name': 'admin',
             'admin': True,
             'server': None,
+            'pending': None,
         },
         {
             'name': 'user',
             'admin': False,
             'server': None,
+            'pending': None,
         }
     ]
 

jupyterhub/tests/test_orm.py

@@ -3,7 +3,11 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 
+import pytest
+from tornado import gen
+
 from .. import orm
+from .mocking import MockSpawner
 
 
 def test_server(db):
@@ -82,3 +86,20 @@ def test_tokens(db):
     assert found.match(token)
     found = orm.APIToken.find(db, 'something else')
     assert found is None
+
+
+def test_spawn_fails(db, io_loop):
+    user = orm.User(name='aeofel')
+    db.add(user)
+    db.commit()
+    
+    class BadSpawner(MockSpawner):
+        @gen.coroutine
+        def start(self):
+            raise RuntimeError("Split the party")
+    
+    with pytest.raises(Exception) as exc:
+        io_loop.run_sync(lambda : user.spawn(BadSpawner))
+    assert user.server is None
+    assert not user.running
+

share/jupyter/hub/templates/admin.html

@@ -42,11 +42,11 @@
       <td class="admin-col col-sm-2">{% if u.admin %}admin{% endif %}</td>
       <td class="time-col col-sm-3">{{u.last_activity.isoformat() + 'Z'}}</td>
       <td class="server-col col-sm-3 text-center">
-        <span class="stop-server btn btn-xs btn-danger {% if not u.server %}hidden{% endif %}">stop server</span>
+        <span class="stop-server btn btn-xs btn-danger {% if not u.running %}hidden{% endif %}">stop server</span>
         {% if admin_access %}
-        <span class="access-server btn btn-xs btn-success {% if not u.server %}hidden{% endif %}">access server</span>
+        <span class="access-server btn btn-xs btn-success {% if not u.running %}hidden{% endif %}">access server</span>
         {% endif %}
-        <span class="start-server btn btn-xs btn-success {% if u.server %}hidden{% endif %}">start server</span>
+        <span class="start-server btn btn-xs btn-success {% if u.running %}hidden{% endif %}">start server</span>
       </td>
       <td class="edit-col col-sm-2">
         <span class="edit-user btn btn-xs btn-primary">edit</span>

share/jupyter/hub/templates/home.html

@@ -5,7 +5,7 @@
 <div class="container">
   <div class="row">
     <div class="text-center">
-      {% if user.server %}
+      {% if user.running %}
       <a id="stop" class="btn btn-lg btn-danger">Stop My Server</a>
       {% endif %}
       <a id="start" class="btn btn-lg btn-success"