From 0f471f4e1256cc5f306571bb65428eff74a4f872 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Tue, 5 Oct 2021 02:30:52 +0530 Subject: [PATCH 1/4] Fail suspected API requests with 404, not 503 Non-running user servers making requests is a fairly common occurance - user servers get culled while their browser tabs are left open. So we now have a background level of 503s responses on the hub *all* the time, making it very difficult to detect *real* 503s, which should ideally be closely monitored and alerted on. I *think* 404 is a more appropriate response, as the resource (API) being requested is no longer present. --- jupyterhub/handlers/base.py | 2 +- jupyterhub/tests/test_pages.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index b0e926ec..024f4a9e 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -1349,7 +1349,7 @@ class UserUrlHandler(BaseHandler): self.log.warning( "Failing suspected API request to not-running server: %s", self.request.path ) - self.set_status(503) + self.set_status(404) self.set_header("Content-Type", "application/json") spawn_url = urlparse(self.request.full_url())._replace(query="") diff --git a/jupyterhub/tests/test_pages.py b/jupyterhub/tests/test_pages.py index 7140b823..133aa426 100644 --- a/jupyterhub/tests/test_pages.py +++ b/jupyterhub/tests/test_pages.py @@ -1001,7 +1001,7 @@ async def test_token_page(app): async def test_server_not_running_api_request(app): cookies = await app.login_user("bees") r = await get_page("user/bees/api/status", app, hub=False, cookies=cookies) - assert r.status_code == 503 + assert r.status_code == 404 assert r.headers["content-type"] == "application/json" message = r.json()['message'] assert ujoin(app.base_url, "hub/spawn/bees") in message From 9cb19cc3426e7bfbb713e8a8166aa9c8e5d2af7e Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Tue, 5 Oct 2021 17:25:14 +0530 Subject: [PATCH 2/4] Use 424 rather than 404 to indicate non-running server 404 is also used to identify that a particular resource (like a kernel or terminal) is not present, maybe because it is deleted. That comes from the notebook server, while here we are responding from JupyterHub. Saying that the user server they are trying to request the resource (kernel, etc) from does not exist seems right. --- jupyterhub/handlers/base.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 024f4a9e..0f40b477 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -1330,7 +1330,7 @@ class UserUrlHandler(BaseHandler): **Changed Behavior as of 1.0** This handler no longer triggers a spawn. Instead, it checks if: - 1. server is not active, serve page prompting for spawn (status: 503) + 1. server is not active, serve page prompting for spawn (status: 424) 2. server is ready (This shouldn't happen! Proxy isn't updated yet. Wait a bit and redirect.) 3. server is active, redirect to /hub/spawn-pending to monitor launch progress (will redirect back when finished) @@ -1349,7 +1349,11 @@ class UserUrlHandler(BaseHandler): self.log.warning( "Failing suspected API request to not-running server: %s", self.request.path ) - self.set_status(404) + + # If we got here, the server is not running. To differentiate + # that the *server* itself is not running, rather than just the particular + # resource *in* the server is not found, we return a 424 instead of a 404. + self.set_status(424) self.set_header("Content-Type", "application/json") spawn_url = urlparse(self.request.full_url())._replace(query="") @@ -1514,15 +1518,14 @@ class UserUrlHandler(BaseHandler): self.redirect(pending_url, status=303) return - # if we got here, the server is not running - # serve a page prompting for spawn and 503 error - # visiting /user/:name no longer triggers implicit spawn - # without explicit user action + # If we got here, the server is not running. To differentiate + # that the *server* itself is not running, rather than just the particular + # page *in* the server is not found, we return a 424 instead of a 404. spawn_url = url_concat( url_path_join(self.hub.base_url, "spawn", user.escaped_name, server_name), {"next": self.request.uri}, ) - self.set_status(503) + self.set_status(424) auth_state = await user.get_auth_state() html = await self.render_template( From 6007ba78b015633a7850507fa81e7717e44065ef Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Tue, 5 Oct 2021 17:56:51 +0530 Subject: [PATCH 3/4] Preserve older 503 behavior behind a flag --- jupyterhub/app.py | 27 +++++++++++++++++++++++---- jupyterhub/handlers/base.py | 10 ++++++++-- jupyterhub/tests/test_pages.py | 9 ++++++++- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 4522c7a7..174f5b2b 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -369,7 +369,7 @@ class JupyterHub(Application): even if your Hub authentication is still valid. If your Hub authentication is valid, logging in may be a transparent redirect as you refresh the page. - + This does not affect JupyterHub API tokens in general, which do not expire by default. Only tokens issued during the oauth flow @@ -862,7 +862,7 @@ class JupyterHub(Application): "/", help=""" The routing prefix for the Hub itself. - + Override to send only a subset of traffic to the Hub. Default is to use the Hub as the default route for all requests. @@ -874,7 +874,7 @@ class JupyterHub(Application): may want to handle these events themselves, in which case they can register their own default target with the proxy and set e.g. `hub_routespec = /hub/` to serve only the hub's own pages, or even `/hub/api/` for api-only operation. - + Note: hub_routespec must include the base_url, if any. .. versionadded:: 1.4 @@ -1457,7 +1457,7 @@ class JupyterHub(Application): Can be a Unicode string (e.g. '/hub/home') or a callable based on the handler object: :: - + def default_url_fn(handler): user = handler.current_user if user and user.admin: @@ -1485,6 +1485,25 @@ class JupyterHub(Application): """, ).tag(config=True) + use_legacy_stopped_server_status_code = Bool( + False, + help=""" + Return 503 rather than 424 when request comes in for a non-running server. + + Prior to JupyterHub 2.0, we returned a 503 when any request came in for + a user server that was currently not running. By default, JupyterHub 2.0 + will return a 424 - this makes operational metric dashboards more useful. + + JupyterLab < 3.2 expected the 503 to know if the user server is no longer + running, and prompted the user to start their server. Set this config to + true to retain the old behavior, so JupyterLab < 3.2 can continue to show + the appropriate UI when the user server is stopped. + + This option will be removed in a future release. + """, + config=True, + ) + def init_handlers(self): h = [] # load handlers from the authenticator diff --git a/jupyterhub/handlers/base.py b/jupyterhub/handlers/base.py index 0f40b477..34ca2c1b 100644 --- a/jupyterhub/handlers/base.py +++ b/jupyterhub/handlers/base.py @@ -1353,7 +1353,10 @@ class UserUrlHandler(BaseHandler): # If we got here, the server is not running. To differentiate # that the *server* itself is not running, rather than just the particular # resource *in* the server is not found, we return a 424 instead of a 404. - self.set_status(424) + # We allow retaining the old behavior to support older JupyterLab versions + self.set_status( + 424 if not self.app.use_legacy_stopped_server_status_code else 503 + ) self.set_header("Content-Type", "application/json") spawn_url = urlparse(self.request.full_url())._replace(query="") @@ -1521,11 +1524,14 @@ class UserUrlHandler(BaseHandler): # If we got here, the server is not running. To differentiate # that the *server* itself is not running, rather than just the particular # page *in* the server is not found, we return a 424 instead of a 404. + # We allow retaining the old behavior to support older JupyterLab versions spawn_url = url_concat( url_path_join(self.hub.base_url, "spawn", user.escaped_name, server_name), {"next": self.request.uri}, ) - self.set_status(424) + self.set_status( + 424 if not self.app.use_legacy_stopped_server_status_code else 503 + ) auth_state = await user.get_auth_state() html = await self.render_template( diff --git a/jupyterhub/tests/test_pages.py b/jupyterhub/tests/test_pages.py index 133aa426..76853cd7 100644 --- a/jupyterhub/tests/test_pages.py +++ b/jupyterhub/tests/test_pages.py @@ -1001,13 +1001,20 @@ async def test_token_page(app): async def test_server_not_running_api_request(app): cookies = await app.login_user("bees") r = await get_page("user/bees/api/status", app, hub=False, cookies=cookies) - assert r.status_code == 404 + assert r.status_code == 424 assert r.headers["content-type"] == "application/json" message = r.json()['message'] assert ujoin(app.base_url, "hub/spawn/bees") in message assert " /user/bees" in message +async def test_server_not_running_api_request_legacy_status(app): + app.use_legacy_stopped_server_status_code = True + cookies = await app.login_user("bees") + r = await get_page("user/bees/api/status", app, hub=False, cookies=cookies) + assert r.status_code == 503 + + async def test_metrics_no_auth(app): r = await get_page("metrics", app) assert r.status_code == 403 From 3b2a1a37f97a4c744d05075928116225578dd24b Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Tue, 5 Oct 2021 18:02:35 +0530 Subject: [PATCH 4/4] Update tests that were looking for 503s --- jupyterhub/tests/test_pages.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/jupyterhub/tests/test_pages.py b/jupyterhub/tests/test_pages.py index 76853cd7..0f1c2e97 100644 --- a/jupyterhub/tests/test_pages.py +++ b/jupyterhub/tests/test_pages.py @@ -53,8 +53,8 @@ async def test_root_redirect(app): r = await get_page(url, app, cookies=cookies) path = urlparse(r.url).path assert path == ujoin(app.base_url, 'hub/user/%s/test.ipynb' % name) - # serve "server not running" page, which has status 503 - assert r.status_code == 503 + # serve "server not running" page, which has status 424 + assert r.status_code == 424 async def test_root_default_url_noauth(app): @@ -169,7 +169,7 @@ async def test_spawn_redirect(app): r = await get_page('user/' + name, app, hub=False, cookies=cookies) path = urlparse(r.url).path assert path == ujoin(app.base_url, 'hub/user/%s/' % name) - assert r.status_code == 503 + assert r.status_code == 424 async def test_spawn_handler_access(app): @@ -504,13 +504,13 @@ async def test_user_redirect_deprecated(app, username): print(urlparse(r.url)) path = urlparse(r.url).path assert path == ujoin(app.base_url, 'hub/user/%s/' % name) - assert r.status_code == 503 + assert r.status_code == 424 r = await get_page('/user/baduser/test.ipynb', app, cookies=cookies, hub=False) print(urlparse(r.url)) path = urlparse(r.url).path assert path == ujoin(app.base_url, 'hub/user/%s/test.ipynb' % name) - assert r.status_code == 503 + assert r.status_code == 424 r = await get_page('/user/baduser/test.ipynb', app, hub=False) r.raise_for_status()