Skip to content

Commit

Permalink
Allow referencing empty-object exports as entrypoints.
Browse files Browse the repository at this point in the history
This fixes a regression in miniflare around named entrypoints: If a script had a top-level export that was an object with no instance properties, miniflare would treat it as a named entrypoint (configuring a socket for it), but workerd would consider this config invalid because the export didn't appear to actually be able to handle events.

It would be hard to make miniflare be able to detect the types of exports, so instead we make workerd more lenient.
  • Loading branch information
kentonv committed Apr 4, 2024
1 parent 084b006 commit 8184033
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/workerd/io/worker.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1957,15 +1957,21 @@ void Worker::Lock::validateHandlers(ValidationErrorReporter& errorReporter) {
// hence we will see it here. Rather than try to correct this inconsistency between
// struct and dict handling (which could have unintended consequences), let's just
// work around by ignoring arrays here.
errorReporter.addEmptyExport(name);
return;
}

auto dict = js.toDict(handle);
bool empty = true;
for (auto& field: dict.fields) {
if (!ignoredHandlers.contains(field.name)) {
errorReporter.addHandler(name, field.name);
empty = false;
}
}
if (empty) {
errorReporter.addEmptyExport(name);
}
};

auto getEntrypointName = [&](kj::StringPtr key) -> kj::Maybe<kj::StringPtr> {
Expand Down
4 changes: 4 additions & 0 deletions src/workerd/io/worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ class Worker: public kj::AtomicRefcounted {
public:
virtual void addError(kj::String error) = 0;
virtual void addHandler(kj::Maybe<kj::StringPtr> exportName, kj::StringPtr type) = 0;

// Called when an export is encountered that defines no handlers, thus isn't useful for
// anything.
virtual void addEmptyExport(kj::Maybe<kj::StringPtr> exportName) {}
};

class LockType;
Expand Down
16 changes: 15 additions & 1 deletion src/workerd/server/server-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,13 @@ KJ_TEST("Server: named entrypoints") {
` return new Response("hello from bar entrypoint");
` }
`}
`
`// Also exprot some symbols that aren't valid entrypoints, but we should still
`// be allowed to point sockets at them. (Sending any actual requests to them
`// will still fail.)
`export let invalidObj = {}; // no handlers
`export let invalidArray = [1, 2];
`export let invalidMap = new Map();
)
]
)
Expand All @@ -1354,7 +1361,14 @@ KJ_TEST("Server: named entrypoints") {
sockets = [
( name = "main", address = "test-addr", service = "hello" ),
( name = "alt1", address = "foo-addr", service = (name = "hello", entrypoint = "foo")),
( name = "alt2", address = "bar-addr", service = (name = "hello", entrypoint = "bar"))
( name = "alt2", address = "bar-addr", service = (name = "hello", entrypoint = "bar")),
( name = "invalid1", address = "invalid1-addr",
service = (name = "hello", entrypoint = "invalidObj")),
( name = "invalid2", address = "invalid2-addr",
service = (name = "hello", entrypoint = "invalidArray")),
( name = "invalid3", address = "invalid3-addr",
service = (name = "hello", entrypoint = "invalidMap")),
]
))"_kj);

Expand Down
17 changes: 17 additions & 0 deletions src/workerd/server/server.c++
Original file line number Diff line number Diff line change
Expand Up @@ -2580,6 +2580,23 @@ kj::Own<Server::Service> Server::makeWorker(kj::StringPtr name, config::Worker::
}
set->insert(kj::str(type));
}

void addEmptyExport(kj::Maybe<kj::StringPtr> exportName) override {
// Even though the export has no handlers, we want to add it to the namedEntrypoints map,
// so that other parts of the config are allowed to refer to this entrypoint. Otherwise,
// if anything in the config refers to it, we'll treat the config as invalid. Of course, if
// any actual requests are sent to an empty handler, they will fail at runtime.
//
// The reason we want the config to be considered valid even if it refers to empty
// entrypoints is so that it's safe to auto-generate a config that binds every exprot to
// a socket, without checking the types of all the exports. Miniflare does this.
KJ_IF_SOME(e, exportName) {
namedEntrypoints.findOrCreate(e,
[&]() -> decltype(namedEntrypoints)::Entry { return { kj::str(e), {} }; });
} else {
defaultEntrypoint.emplace();
}
}
};

ErrorReporter errorReporter(*this, name);
Expand Down

0 comments on commit 8184033

Please sign in to comment.