Return HTTP 400 when attempting to post an event with an unregistered schema (#1463)

Co-authored-by: Zachary Sailer <zachsailer@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: 3 people

jupyter_server/services/events/handlers.py

@@ -71,12 +71,25 @@ def on_close(self):
         self.event_logger.remove_listener(listener=self.event_listener)
 
 
-def validate_model(data: dict[str, Any]) -> None:
-    """Validates for required fields in the JSON request body"""
+def validate_model(
+    data: dict[str, Any], registry: jupyter_events.schema_registry.SchemaRegistry
+) -> None:
+    """Validates for required fields in the JSON request body and verifies that
+    a registered schema/version exists"""
     required_keys = {"schema_id", "version", "data"}
     for key in required_keys:
         if key not in data:
-            raise web.HTTPError(400, f"Missing `{key}` in the JSON request body.")
+            message = f"Missing `{key}` in the JSON request body."
+            raise Exception(message)
+    schema_id = cast(str, data.get("schema_id"))
+    # The case where a given schema_id isn't found,
+    # jupyter_events raises a useful error, so there's no need to
+    # handle that case here.
+    schema = registry.get(schema_id)
+    version = int(cast(int, data.get("version")))
+    if schema.version != version:
+        message = f"Unregistered version: {version}≠{schema.version} for `{schema_id}`"
+        raise Exception(message)
 
 
 def get_timestamp(data: dict[str, Any]) -> Optional[datetime]:
@@ -111,18 +124,18 @@ async def post(self):
             raise web.HTTPError(400, "No JSON data provided")
 
         try:
-            validate_model(payload)
+            validate_model(payload, self.event_logger.schemas)
             self.event_logger.emit(
                 schema_id=cast(str, payload.get("schema_id")),
                 data=cast("Dict[str, Any]", payload.get("data")),
                 timestamp_override=get_timestamp(payload),
             )
             self.set_status(204)
             self.finish()
-        except web.HTTPError:
-            raise
         except Exception as e:
-            raise web.HTTPError(500, str(e)) from e
+            # All known exceptions are raised by bad requests, e.g., bad
+            # version, unregistered schema, invalid emission data payload, etc.
+            raise web.HTTPError(400, str(e)) from e
 
 
 default_handlers = [

tests/services/events/test_api.py

@@ -117,16 +117,17 @@ async def test_post_event(jp_fetch, event_logger_sink, payload):
 }
 """
 
-
-@pytest.mark.parametrize("payload", [payload_3, payload_4, payload_5, payload_6])
-async def test_post_event_400(jp_fetch, event_logger, payload):
-    with pytest.raises(tornado.httpclient.HTTPClientError) as e:
-        await jp_fetch("api", "events", method="POST", body=payload)
-
-    assert expected_http_error(e, 400)
-
-
 payload_7 = """\
+{
+    "schema_id": "http://event.mock.jupyter.org/UNREGISTERED-SCHEMA",
+    "version": 1,
+    "data": {
+        "event_message": "Hello, world!"
+    }
+}
+"""
+
+payload_8 = """\
 {
     "schema_id": "http://event.mock.jupyter.org/message",
     "version": 1,
@@ -136,20 +137,23 @@ async def test_post_event_400(jp_fetch, event_logger, payload):
 }
 """
 
-payload_8 = """\
+payload_9 = """\
 {
     "schema_id": "http://event.mock.jupyter.org/message",
     "version": 2,
     "data": {
-        "message": "Hello, world!"
+        "event_message": "Hello, world!"
     }
 }
 """
 
 
-@pytest.mark.parametrize("payload", [payload_7, payload_8])
-async def test_post_event_500(jp_fetch, event_logger, payload):
+@pytest.mark.parametrize(
+    "payload",
+    [payload_3, payload_4, payload_5, payload_6, payload_7, payload_8, payload_9],
+)
+async def test_post_event_400(jp_fetch, event_logger, payload):
     with pytest.raises(tornado.httpclient.HTTPClientError) as e:
         await jp_fetch("api", "events", method="POST", body=payload)
 
-    assert expected_http_error(e, 500)
+    assert expected_http_error(e, 400)