Skip to content

Commit 8704996

Browse files
committed
Fix exporting a class instance as an entrypoint.
This should work: ``` class Foo { async fetch(req, env, ctx) { ... } } export default new Foo(); ``` But it had a slight bug: `validateHandlers()` did not report the `fetch` method as existing. This meant you couldn't actually deploy a worker defined this way as the upload validator on the server side would reject it for not implementing `fetch`. I asked Claude Code to fix it (as a test... I knew what the problem was). Transcript: https://claude-workerd-transcript.pages.dev/validate-handlers
1 parent c03a25b commit 8704996

File tree

1 file changed

+59
-37
lines changed

1 file changed

+59
-37
lines changed

src/workerd/io/worker.c++

+59-37
Original file line numberDiff line numberDiff line change
@@ -2156,6 +2156,44 @@ void Worker::Lock::validateHandlers(ValidationErrorReporter& errorReporter) {
21562156
ignoredHandlers.insert("unhandledrejection"_kj);
21572157
ignoredHandlers.insert("rejectionhandled"_kj);
21582158

2159+
// Helper function to collect methods from a prototype chain
2160+
auto collectMethodsFromPrototypeChain = [&](jsg::JsValue startProto,
2161+
kj::HashSet<kj::String>& seenNames) {
2162+
// Find the prototype for `Object` by creating one.
2163+
auto obj = js.obj();
2164+
jsg::JsValue prototypeOfObject = obj.getPrototype(js);
2165+
2166+
// Walk the prototype chain.
2167+
jsg::JsValue proto = startProto;
2168+
for (;;) {
2169+
auto protoObj = JSG_REQUIRE_NONNULL(proto.tryCast<jsg::JsObject>(), TypeError,
2170+
"Exported value's prototype chain does not end in Object.");
2171+
if (protoObj == prototypeOfObject) {
2172+
// Reached the prototype for `Object`. Stop here.
2173+
break;
2174+
}
2175+
2176+
// Awkwardly, the prototype's members are not typically enumerable, so we have to
2177+
// enumerate them rather directly.
2178+
jsg::JsArray properties = protoObj.getPropertyNames(js, jsg::KeyCollectionFilter::OWN_ONLY,
2179+
jsg::PropertyFilter::SKIP_SYMBOLS, jsg::IndexFilter::SKIP_INDICES);
2180+
for (auto i: kj::zeroTo(properties.size())) {
2181+
auto name = properties.get(js, i).toString(js);
2182+
if (name == "constructor"_kj) {
2183+
// Don't treat special method `constructor` as an exported handler.
2184+
continue;
2185+
}
2186+
2187+
if (!ignoredHandlers.contains(name)) {
2188+
// Only report each method name once, even if it overrides a method in a superclass.
2189+
seenNames.upsert(kj::mv(name), [&](auto&, auto&&) {});
2190+
}
2191+
}
2192+
2193+
proto = protoObj.getPrototype(js);
2194+
}
2195+
};
2196+
21592197
KJ_IF_SOME(c, worker.impl->context) {
21602198
// Service workers syntax.
21612199
auto handlerNames = c->getHandlerNames();
@@ -2172,7 +2210,6 @@ void Worker::Lock::validateHandlers(ValidationErrorReporter& errorReporter) {
21722210
errorReporter.addEntrypoint(kj::none, handlers.releaseAsArray());
21732211
} else {
21742212
auto report = [&](kj::Maybe<kj::StringPtr> name, api::ExportedHandler& exported) {
2175-
kj::Vector<kj::String> methods;
21762213
auto handle = exported.self.getHandle(js);
21772214
if (handle->IsArray()) {
21782215
// HACK: toDict() will throw a TypeError if given an array, because jsg::DictWrapper is
@@ -2182,15 +2219,28 @@ void Worker::Lock::validateHandlers(ValidationErrorReporter& errorReporter) {
21822219
// hence we will see it here. Rather than try to correct this inconsistency between
21832220
// struct and dict handling (which could have unintended consequences), let's just
21842221
// work around by ignoring arrays here.
2222+
errorReporter.addEntrypoint(name, kj::Array<kj::String>());
21852223
} else {
2224+
// Use a HashSet to avoid duplicates when methods exist both as own properties
2225+
// and in the prototype chain
2226+
kj::HashSet<kj::String> methodSet;
2227+
2228+
// First, check for own properties (like a plain object literal)
21862229
auto dict = js.toDict(handle);
21872230
for (auto& field: dict.fields) {
21882231
if (!ignoredHandlers.contains(field.name)) {
2189-
methods.add(kj::mv(field.name));
2232+
methodSet.upsert(kj::mv(field.name), [&](auto&, auto&&) {});
21902233
}
21912234
}
2235+
2236+
// Then, check for methods in the prototype chain (like a class instance)
2237+
js.withinHandleScope([&]() {
2238+
collectMethodsFromPrototypeChain(jsg::JsObject(handle).getPrototype(js), methodSet);
2239+
});
2240+
2241+
// Convert HashSet to Array for reporting
2242+
errorReporter.addEntrypoint(name, KJ_MAP(n, methodSet) { return kj::mv(n); });
21922243
}
2193-
errorReporter.addEntrypoint(name, methods.releaseAsArray());
21942244
};
21952245

21962246
auto getEntrypointName = [&](kj::StringPtr key) -> kj::Maybe<kj::StringPtr> {
@@ -2224,44 +2274,16 @@ void Worker::Lock::validateHandlers(ValidationErrorReporter& errorReporter) {
22242274
// prototype, and its prototype's prototype, and so on, until we get to Object's
22252275
// prototype, which we ignore.
22262276
auto entrypointName = getEntrypointName(entry.key);
2227-
js.withinHandleScope([&]() {
2228-
// Find the prototype for `Object` by creating one.
2229-
auto obj = js.obj();
2230-
jsg::JsValue prototypeOfObject = obj.getPrototype(js);
2277+
kj::HashSet<kj::String> seenNames;
22312278

2232-
// Walk the prototype chain.
2279+
js.withinHandleScope([&]() {
2280+
// For stateless classes, we need to get the class's prototype property
22332281
jsg::JsObject ctor(KJ_ASSERT_NONNULL(entry.value.tryGetHandle(js.v8Isolate)));
22342282
jsg::JsValue proto = ctor.get(js, "prototype");
2235-
kj::HashSet<kj::String> seenNames;
2236-
for (;;) {
2237-
auto protoObj = JSG_REQUIRE_NONNULL(proto.tryCast<jsg::JsObject>(), TypeError,
2238-
"Exported entrypoint class's prototype chain does not end in Object.");
2239-
if (protoObj == prototypeOfObject) {
2240-
// Reached the prototype for `Object`. Stop here.
2241-
break;
2242-
}
2243-
2244-
// Awkwardly, the prototype's members are not typically enumerable, so we have to
2245-
// enumerate them rather directly.
2246-
jsg::JsArray properties =
2247-
protoObj.getPropertyNames(js, jsg::KeyCollectionFilter::OWN_ONLY,
2248-
jsg::PropertyFilter::SKIP_SYMBOLS, jsg::IndexFilter::SKIP_INDICES);
2249-
for (auto i: kj::zeroTo(properties.size())) {
2250-
auto name = properties.get(js, i).toString(js);
2251-
if (name == "constructor"_kj) {
2252-
// Don't treat special method `constructor` as an exported handler.
2253-
continue;
2254-
}
2255-
2256-
// Only report each method name once, even if it overrides a method in a superclass.
2257-
seenNames.upsert(kj::mv(name), [&](auto&, auto&&) {});
2258-
}
2259-
2260-
proto = protoObj.getPrototype(js);
2261-
}
2262-
2263-
errorReporter.addEntrypoint(entrypointName, KJ_MAP(n, seenNames) { return kj::mv(n); });
2283+
collectMethodsFromPrototypeChain(proto, seenNames);
22642284
});
2285+
2286+
errorReporter.addEntrypoint(entrypointName, KJ_MAP(n, seenNames) { return kj::mv(n); });
22652287
}
22662288
}
22672289
});

0 commit comments

Comments
 (0)