ContentsHandler return 404 rather than raise exc (#1357)

Author: bloomsa

jupyter_server/services/contents/handlers.py

@@ -5,6 +5,7 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 import json
+from http import HTTPStatus
 
 try:
     from jupyter_client.jsonutil import json_default
@@ -91,6 +92,12 @@ def _finish_model(self, model, location=True):
         self.set_header("Content-Type", "application/json")
         self.finish(json.dumps(model, default=json_default))
 
+    async def _finish_error(self, code, message):
+        """Finish a JSON request with an error code and descriptive message"""
+        self.set_status(code)
+        self.write(message)
+        await self.finish()
+
     @web.authenticated
     @authorized
     async def get(self, path=""):
@@ -116,18 +123,27 @@ async def get(self, path=""):
         content = int(content_str or "")
 
         if not cm.allow_hidden and await ensure_async(cm.is_hidden(path)):
-            raise web.HTTPError(404, f"file or directory {path!r} does not exist")
-
-        model = await ensure_async(
-            self.contents_manager.get(
-                path=path,
-                type=type,
-                format=format,
-                content=content,
+            await self._finish_error(
+                HTTPStatus.NOT_FOUND, f"file or directory {path!r} does not exist"
             )
-        )
-        validate_model(model, expect_content=content)
-        self._finish_model(model, location=False)
+        try:
+            model = await ensure_async(
+                self.contents_manager.get(
+                    path=path,
+                    type=type,
+                    format=format,
+                    content=content,
+                )
+            )
+            validate_model(model, expect_content=content)
+            self._finish_model(model, location=False)
+        except web.HTTPError as exc:
+            # 404 is okay in this context, catch exception and return 404 code to prevent stack trace on client
+            if exc.status_code == HTTPStatus.NOT_FOUND:
+                await self._finish_error(
+                    HTTPStatus.NOT_FOUND, f"file or directory {path!r} does not exist"
+                )
+            raise
 
     @web.authenticated
     @authorized

tests/services/contents/test_manager.py

@@ -406,35 +406,44 @@ async def test_400(jp_file_contents_manager_class, tmp_path):  # noqa
 
 
 async def test_404(jp_file_contents_manager_class, tmp_path):
+    # setup
+    td = str(tmp_path)
+    cm = jp_file_contents_manager_class(root_dir=td)
+
     # Test visible file in hidden folder
-    with pytest.raises(HTTPError) as excinfo:
-        td = str(tmp_path)
-        cm = jp_file_contents_manager_class(root_dir=td)
-        hidden_dir = ".hidden"
-        file_in_hidden_path = os.path.join(hidden_dir, "visible.txt")
-        _make_dir(cm, hidden_dir)
-        model = await ensure_async(cm.new(path=file_in_hidden_path))
-        os_path = cm._get_os_path(model["path"])
+    cm.allow_hidden = True
+    hidden_dir = ".hidden"
+    file_in_hidden_path = os.path.join(hidden_dir, "visible.txt")
+    _make_dir(cm, hidden_dir)
+    model = await ensure_async(cm.new(path=file_in_hidden_path))
+    os_path = cm._get_os_path(model["path"])
+    cm.allow_hidden = False
 
-        try:
-            result = await ensure_async(cm.get(os_path, "w"))
-        except HTTPError as e:
-            assert e.status_code == 404
+    with pytest.raises(HTTPError) as excinfo:
+        await ensure_async(cm.get(os_path))
+        assert excinfo.value.status_code == 404
 
     # Test hidden file in visible folder
+    cm.allow_hidden = True
+    hidden_dir = "visible"
+    file_in_hidden_path = os.path.join(hidden_dir, ".hidden.txt")
+    _make_dir(cm, hidden_dir)
+    model = await ensure_async(cm.new(path=file_in_hidden_path))
+    os_path = cm._get_os_path(model["path"])
+    cm.allow_hidden = False
+
     with pytest.raises(HTTPError) as excinfo:
-        td = str(tmp_path)
-        cm = jp_file_contents_manager_class(root_dir=td)
-        hidden_dir = "visible"
-        file_in_hidden_path = os.path.join(hidden_dir, ".hidden.txt")
-        _make_dir(cm, hidden_dir)
-        model = await ensure_async(cm.new(path=file_in_hidden_path))
-        os_path = cm._get_os_path(model["path"])
+        await ensure_async(cm.get(os_path))
+        assert excinfo.value.status_code == 404
 
-        try:
-            result = await ensure_async(cm.get(os_path, "w"))
-        except HTTPError as e:
-            assert e.status_code == 404
+    # Test file not found
+    td = str(tmp_path)
+    cm = jp_file_contents_manager_class(root_dir=td)
+    not_a_file = "foo.bar"
+
+    with pytest.raises(HTTPError) as excinfo:
+        await ensure_async(cm.get(not_a_file))
+        assert excinfo.value.status_code == 404
 
 
 async def test_escape_root(jp_file_contents_manager_class, tmp_path):