diff --git a/jupyterhub/handlers/pages.py b/jupyterhub/handlers/pages.py index 2aa2c3e8..68264afc 100644 --- a/jupyterhub/handlers/pages.py +++ b/jupyterhub/handlers/pages.py @@ -172,7 +172,19 @@ class SpawnHandler(BaseHandler): spawner._spawn_future = None # not running, no form. Trigger spawn and redirect back to /user/:name f = asyncio.ensure_future(self.spawn_single_user(user, server_name)) - await asyncio.wait([f], timeout=1) + done, pending = await asyncio.wait([f], timeout=1) + # If spawn_single_user throws an exception, raise a 500 error + # otherwise it may cause a redirect loop + if f in done: + future, = done + exc = future.exception() + if exc: + raise web.HTTPError( + 500, + "Error in Authenticator.pre_spawn_start: %s %s" + % (type(exc).__name__, str(exc)), + ) + return self.redirect(pending_url) @web.authenticated diff --git a/jupyterhub/user.py b/jupyterhub/user.py index 97a3543f..407310db 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -529,17 +529,19 @@ class User: # trigger pre-spawn hook on authenticator authenticator = self.authenticator - if authenticator: - await maybe_future(authenticator.pre_spawn_start(self, spawner)) - - spawner._start_pending = True - # update spawner start time, and activity for both spawner and user - self.last_activity = ( - spawner.orm_spawner.started - ) = spawner.orm_spawner.last_activity = datetime.utcnow() - db.commit() - # wait for spawner.start to return try: + if authenticator: + # pre_spawn_start can thow errors that can lead to a redirect loop + # if left uncaught (see https://github.com/jupyterhub/jupyterhub/issues/2683) + await maybe_future(authenticator.pre_spawn_start(self, spawner)) + + spawner._start_pending = True + # update spawner start time, and activity for both spawner and user + self.last_activity = ( + spawner.orm_spawner.started + ) = spawner.orm_spawner.last_activity = datetime.utcnow() + db.commit() + # wait for spawner.start to return # run optional preparation work to bootstrap the notebook await maybe_future(spawner.run_pre_spawn_hook()) if self.settings.get('internal_ssl'):