From b51f49efffbabc43b9a38e94ab403eee43fdcbd2 Mon Sep 17 00:00:00 2001
From: Hood Chatham <roberthoodchatham@gmail.com>
Date: Fri, 7 Feb 2025 22:49:05 +0100
Subject: [PATCH] Python workers: Better error handling when the handler is not
 defined (#3495)

---
 src/pyodide/python-entrypoint-helper.ts       |  9 ++++-
 src/workerd/server/tests/python/BUILD.bazel   |  9 +++++
 .../tests/python/undefined-handler/main.py    |  2 +
 .../tests/python/undefined-handler/server.mjs | 11 ++++++
 .../undefined-handler.wd-test                 | 37 +++++++++++++++++++
 .../undefined-handler/undefined_handler.py    |  2 +
 6 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 src/workerd/server/tests/python/undefined-handler/main.py
 create mode 100644 src/workerd/server/tests/python/undefined-handler/server.mjs
 create mode 100644 src/workerd/server/tests/python/undefined-handler/undefined-handler.wd-test
 create mode 100644 src/workerd/server/tests/python/undefined-handler/undefined_handler.py

diff --git a/src/pyodide/python-entrypoint-helper.ts b/src/pyodide/python-entrypoint-helper.ts
index 5aa8c42b0f3..bf8c727d8d5 100644
--- a/src/pyodide/python-entrypoint-helper.ts
+++ b/src/pyodide/python-entrypoint-helper.ts
@@ -131,9 +131,14 @@ function makeHandler(pyHandlerName: string): Handler {
         'prep_python',
         async () => await preparePython()
       );
-
+      const handler = mainModule[pyHandlerName];
+      if (!handler) {
+        throw new Error(
+          `Python entrypoint "${MAIN_MODULE_NAME}" does not export a handler named "${pyHandlerName}"`
+        );
+      }
       const result = await enterJaegerSpan('python_code', () => {
-        return mainModule[pyHandlerName].callRelaxed(...args);
+        return handler.callRelaxed(...args);
       });
 
       // Support returning a pyodide.ffi.FetchResponse.
diff --git a/src/workerd/server/tests/python/BUILD.bazel b/src/workerd/server/tests/python/BUILD.bazel
index f680f186cb1..5b3634fe6f3 100644
--- a/src/workerd/server/tests/python/BUILD.bazel
+++ b/src/workerd/server/tests/python/BUILD.bazel
@@ -57,3 +57,12 @@ gen_import_tests(
         ],
     },
 )
+
+py_wd_test(
+    "undefined-handler",
+    # TODO: Requires a new bundle deploy
+    skip_python_flags = [
+        "0.26.0a2",
+        "0.27.1",
+    ],
+)
diff --git a/src/workerd/server/tests/python/undefined-handler/main.py b/src/workerd/server/tests/python/undefined-handler/main.py
new file mode 100644
index 00000000000..f174823854e
--- /dev/null
+++ b/src/workerd/server/tests/python/undefined-handler/main.py
@@ -0,0 +1,2 @@
+def test():
+    pass
diff --git a/src/workerd/server/tests/python/undefined-handler/server.mjs b/src/workerd/server/tests/python/undefined-handler/server.mjs
new file mode 100644
index 00000000000..4472a71070e
--- /dev/null
+++ b/src/workerd/server/tests/python/undefined-handler/server.mjs
@@ -0,0 +1,11 @@
+import { rejects } from 'node:assert';
+
+export default {
+  async test(ctrl, env) {
+    await rejects(env.pythonWorker.fetch(new Request('http://example.com')), {
+      name: 'Error',
+      message:
+        'Python entrypoint "undefined_handler.py" does not export a handler named "on_fetch"',
+    });
+  },
+};
diff --git a/src/workerd/server/tests/python/undefined-handler/undefined-handler.wd-test b/src/workerd/server/tests/python/undefined-handler/undefined-handler.wd-test
new file mode 100644
index 00000000000..8e6740a684a
--- /dev/null
+++ b/src/workerd/server/tests/python/undefined-handler/undefined-handler.wd-test
@@ -0,0 +1,37 @@
+using Workerd = import "/workerd/workerd.capnp";
+
+const undefinedHandler :Workerd.Worker = (
+  modules = [
+    (name = "undefined_handler.py", pythonModule = embed "undefined_handler.py")
+  ],
+  compatibilityDate = "2024-10-01",
+  compatibilityFlags = [%PYTHON_FEATURE_FLAGS],
+);
+
+const server :Workerd.Worker = (
+  modules = [
+    (name = "server.js", esModule = embed "server.mjs")
+  ],
+  bindings = [
+    ( name = "pythonWorker", service = "undefinedHandler" ),
+  ],
+  compatibilityDate = "2024-10-01",
+  compatibilityFlags = ["nodejs_compat"],
+);
+
+
+const unitTests :Workerd.Config = (
+  services = [
+    ( name = "external",
+      network = (
+        tlsOptions = (trustBrowserCas = true)
+      )
+    ),
+    ( name = "server",
+      worker = .server
+    ),
+    ( name = "undefinedHandler",
+      worker = .undefinedHandler
+    ),
+  ],
+);
diff --git a/src/workerd/server/tests/python/undefined-handler/undefined_handler.py b/src/workerd/server/tests/python/undefined-handler/undefined_handler.py
new file mode 100644
index 00000000000..f174823854e
--- /dev/null
+++ b/src/workerd/server/tests/python/undefined-handler/undefined_handler.py
@@ -0,0 +1,2 @@
+def test():
+    pass