ALEPH-512 ensure we always return json for error

src/aleph/vm/orchestrator/resources.py

@@ -166,7 +166,7 @@ async def about_certificates(request: web.Request):
     """Public endpoint to expose platform certificates for confidential computing."""
 
     if not settings.ENABLE_CONFIDENTIAL_COMPUTING:
-        return web.HTTPBadRequest(reason="Confidential computing setting not enabled on that server")
+        return web.HTTPServiceUnavailable(text="Confidential computing setting not enabled on that server")
 
     sev_client: SevClient = request.app["sev_client"]
 

src/aleph/vm/orchestrator/supervisor.py

@@ -1,18 +1,18 @@
 """
 The VM Supervisor is in charge of executing code, starting and stopping VMs and provides
-and API to launch these operations.
+an API to launch these operations.
 
-At it's core, it is currently an asynchronous HTTP server using aiohttp, but this may
+At its core, it is currently an asynchronous HTTP server using aiohttp, but this may
 evolve in the future.
 """
 
 import asyncio
 import logging
-from collections.abc import Awaitable, Callable
 from pathlib import Path
 from secrets import token_urlsafe
 
-from aiohttp import web
+from aiohttp import hdrs, web
+from aiohttp.web_exceptions import HTTPException
 from aiohttp_cors import ResourceOptions, setup
 
 from aleph.vm.conf import settings
@@ -61,16 +61,37 @@
 
 
 @web.middleware
-async def server_version_middleware(
-    request: web.Request,
-    handler: Callable[[web.Request], Awaitable[web.StreamResponse]],
-) -> web.StreamResponse:
+async def error_middleware(request, handler) -> web.Response:
+    "Ensure we always return a JSON response for errors."
+    try:
+        response = await handler(request)
+        if response.status == 404:
+            message = response.text
+            status = response.status
+            return web.json_response({"error": message}, status=status)
+        if isinstance(response, HTTPException):
+            if response.headers[hdrs.CONTENT_TYPE] != "application/json":
+                message = response.text or response.reason
+                status = response.status
+                return web.json_response(
+                    {"error": message},
+                    status=status,
+                )
+        return response
+    except web.HTTPException as exc:
+        message = exc.text or exc.reason
+        status = exc.status
+        return web.json_response({"error": message}, status=status)
+    except Exception as exc:
+        message = str(exc)
+        status = 500
+        return web.json_response({"error": message, "error_type": str(type(exc))}, status=status)
+    assert False, "unreachable"
+
+
+async def on_prepare_server_version(request: web.Request, response: web.Response) -> None:
     """Add the version of Aleph-VM in the HTTP headers of the responses."""
-    resp: web.StreamResponse = await handler(request)
-    resp.headers.update(
-        {"Server": f"aleph-vm/{__version__}"},
-    )
-    return resp
+    response.headers["Server"] = f"aleph-vm/{__version__}"
 
 
 async def http_not_found(request: web.Request):
@@ -79,7 +100,8 @@ async def http_not_found(request: web.Request):
 
 
 def setup_webapp():
-    app = web.Application(middlewares=[server_version_middleware])
+    app = web.Application(middlewares=[error_middleware])
+    app.on_response_prepare.append(on_prepare_server_version)
     cors = setup(
         app,
         defaults={

src/aleph/vm/orchestrator/views/__init__.py

@@ -148,6 +148,7 @@
 
 @cors_allow_all
 async def about_executions(request: web.Request) -> web.Response:
+    "/about/executions/details Debugging endpoint with full execution details."
     authenticate_request(request)
     pool: VmPool = request.app["vm_pool"]
     return web.json_response(
@@ -399,7 +400,7 @@
     pool: VmPool = request.app["vm_pool"]
 
     async with allocation_lock:
-        # First free resources from persistent programs and instances that are not scheduled anymore.
+        # First, free resources from persistent programs and instances that are not scheduled anymore.
         allocations = allocation.persistent_vms | allocation.instances
         # Make a copy since the pool is modified
         for execution in list(pool.get_persistent_executions()):
@@ -494,7 +495,7 @@
         data = await request.json()
         vm_notification = VMNotification.model_validate(data)
     except JSONDecodeError:
-        return web.HTTPBadRequest(reason="Body is not valid JSON")
+        return web.HTTPBadRequest(text="Body is not valid JSON")
     except ValidationError as error:
         return web.json_response(data=error.json(), status=web.HTTPBadRequest.status_code)
 
@@ -639,7 +640,7 @@
         data = await request.json()
         message = InstanceContent.model_validate(data)
     except JSONDecodeError:
-        return web.HTTPBadRequest(reason="Body is not valid JSON")
+        return web.HTTPBadRequest(text="Body is not valid JSON")
     except ValidationError as error:
         return web.json_response(data=error.json(), status=web.HTTPBadRequest.status_code)
 

src/aleph/vm/orchestrator/views/operator.py

@@ -364,7 +364,7 @@
         data = await request.json()
         params = InjectSecretParams.model_validate(data)
     except json.JSONDecodeError:
-        return web.HTTPBadRequest(reason="Body is not valid JSON")
+        return web.HTTPBadRequest(text="Body is not valid JSON")
     except pydantic.ValidationError as error:
         return web.json_response(data=error.json(), status=web.HTTPBadRequest.status_code)
 

tests/supervisor/test_views.py

@@ -97,7 +97,7 @@ async def test_allocation_invalid_auth_token(aiohttp_client):
         headers={"X-Auth-Signature": "notTest"},
     )
     assert response.status == 401
-    assert await response.text() == "Authentication token received is invalid"
+    assert await response.json() == {"error": "Authentication token received is invalid"}
 
 
 @pytest.mark.asyncio
@@ -110,7 +110,7 @@ async def test_allocation_missing_auth_token(aiohttp_client):
         json={"persistent_vms": []},
     )
     assert response.status == 401
-    assert await response.text() == "Authentication token is missing"
+    assert await response.json() == {"error": "Authentication token is missing"}
 
 
 @pytest.mark.asyncio
@@ -147,8 +147,8 @@ async def test_about_certificates_missing_setting(aiohttp_client):
     app["sev_client"] = SevClient(Path().resolve(), Path("/opt/sevctl").resolve())
     client = await aiohttp_client(app)
     response: web.Response = await client.get("/about/certificates")
-    assert response.status == 400
-    assert await response.text() == "400: Confidential computing setting not enabled on that server"
+    assert response.status == 503
+    assert await response.json() == {"error": "Confidential computing setting not enabled on that server"}
 
 
 @pytest.mark.asyncio

tests/supervisor/views/test_view_errors.py

@@ -0,0 +1,26 @@
+import pytest
+from aiohttp.test_utils import TestClient
+
+from aleph.vm.orchestrator.supervisor import setup_webapp
+
+
+@pytest.mark.asyncio
+async def test_json_404_about(aiohttp_client, mocker):
+    app = setup_webapp()
+    client: TestClient = await aiohttp_client(app)
+    response = await client.get(
+        "/about/non_existing_path",
+    )
+    assert response.status == 404
+    assert response.content_type == "application/json"
+    assert await response.json() == {"error": "404: Not Found"}
+
+
+@pytest.mark.asyncio
+async def test_json_err_allocation_notify(aiohttp_client, mocker):
+    app = setup_webapp()
+    client: TestClient = await aiohttp_client(app)
+    response = await client.post("/control/allocation/notify", data="invalid_json")
+    assert response.status == 400
+    assert response.content_type == "application/json"
+    assert await response.json() == {"error": "Body is not valid JSON"}