Skip to content

Commit

Permalink
Undo tightening of rules around default entrypoints in #3193.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kentonv committed Dec 13, 2024
1 parent ace145c commit 04a8a51
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 1 deletion.
62 changes: 62 additions & 0 deletions src/workerd/server/server-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
14 changes: 13 additions & 1 deletion src/workerd/server/server.c++
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 04a8a51

Please sign in to comment.