From 04a8a51a889aa8ebe796d00593528e7a056bd5f4 Mon Sep 17 00:00:00 2001
From: Kenton Varda <kenton@cloudflare.com>
Date: Fri, 13 Dec 2024 09:47:56 -0600
Subject: [PATCH] Undo tightening of rules around default entrypoints in #3193.

Say you have a module-syntax Worker that has no `export default`. You then refer to that worker from elsewhere in the workerd config, but you do not specify an entrypoint. What should happen?

In the midst of other changes, PR #3193 turned this into an error, becaues it seemed like obviously it should be. This turns out to have broken Miniflare tests, which often use an empty string as a placeholder Worker that is referenced but never invoked.

So, this PR restores the old behavior.

It turns out, incidentally, that the old behavior is crazier than you'd imagine. See the comments in the test case for details.
---
 src/workerd/server/server-test.c++ | 62 ++++++++++++++++++++++++++++++
 src/workerd/server/server.c++      | 14 ++++++-
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/src/workerd/server/server-test.c++ b/src/workerd/server/server-test.c++
index 54c8ea19748..7754ea18d09 100644
--- a/src/workerd/server/server-test.c++
+++ b/src/workerd/server/server-test.c++
@@ -1441,6 +1441,68 @@ KJ_TEST("Server: invalid entrypoint") {
       "has no such named entrypoint.\n");
 }
 
+KJ_TEST("Server: referencing non-extant default entrypoint is not an error") {
+  // For historical reasons, it's not a config error to refer to to the default entrypoint of
+  // a service that has no default export.
+  TestServer test(R"((
+    services = [
+      ( name = "hello",
+        worker = (
+          compatibilityDate = "2022-08-17",
+          modules = [
+            ( name = "main.js",
+              esModule =
+                `export let alt = {
+                `  async fetch(request, env) {
+                `    return new Response("OK");
+                `  }
+                `}
+            )
+          ],
+        )
+      ),
+    ],
+    sockets = [
+      ( name = "main", address = "test-addr", service = "hello" ),
+    ]
+  ))"_kj);
+  test.start();
+
+  // A request will still fail at runtime, but we shouldn't have seen startup/config errors.
+  auto conn = test.connect("test-addr");
+  conn.sendHttpGet("/");
+
+  // Due to the Deep Magic (bugs) going back to the dawn of Module Workers, if an HTTP request is
+  // delivered to the default entrypoint of a module worker that has no default export, then the
+  // system will fall back to calling event handlers registered with addEventListener("fetch").
+  //
+  // There is a magic deeper still in which, due to mistakes introduced in the stillness and the
+  // darkness before Module Workers dawned, if none of those event listeners call
+  // `event.respondWith()` (perhaps because *there are no event listeners*), then the request falls
+  // back to default handling, in which it simply passes through to fetch() and makes a subrequest.
+  //
+  // So... we expect... a subrequest...
+  {
+    auto subreq = test.receiveSubrequest("foo", {"public"});
+    subreq.recv(R"(
+      GET / HTTP/1.1
+      Host: foo
+
+    )"_blockquote);
+    subreq.send(R"(
+      HTTP/1.1 200 OK
+      Content-Length: 3
+
+      wat)"_blockquote);
+  }
+
+  conn.recv(R"(
+    HTTP/1.1 200 OK
+    Content-Length: 3
+
+    wat)"_blockquote);
+}
+
 KJ_TEST("Server: call queue handler on service binding") {
   TestServer test(R"((
     services = [
diff --git a/src/workerd/server/server.c++ b/src/workerd/server/server.c++
index 9be9dc570ec..dfa4db174e5 100644
--- a/src/workerd/server/server.c++
+++ b/src/workerd/server/server.c++
@@ -1630,7 +1630,19 @@ class Server::WorkerService final: public Service,
       name = entry.key;  // replace with more-permanent string
       handlers = &entry.value;
     } else {
-      handlers = &KJ_UNWRAP_OR_RETURN(defaultEntrypointHandlers, kj::none);
+      KJ_IF_SOME(d, defaultEntrypointHandlers) {
+        handlers = &d;
+      } else {
+        // It would appear that there is no default export, therefore this refers to an entrypoint
+        // that doesn't exist! However, this was historically allowed. For backwards-compatibility,
+        // we preserve this behavior, by returning a reference to the WorkerService itself, whose
+        // startRequest() will fail.
+        //
+        // What will happen if you invoke this entrypoint? Not what you think. Check out the
+        // test case in server-test.c++ entitled "referencing non-extant default entrypoint is not
+        // an error" for the sordid details.
+        return fakeOwn(*this);
+      }
     }
     return kj::heap<EntrypointService>(*this, name, propsJson, *handlers);
   }