CapabilityServerSet::getLocalServer() really needs to throw if given a broken promise capability.

c++/src/capnp/capability-test.c++

@@ -864,7 +864,10 @@ TEST(Capability, CapabilityServerSet) {
   auto paf = kj::newPromiseAndFulfiller<test::TestInterface::Client>();
   test::TestInterface::Client clientPromise = kj::mv(paf.promise);
 
-  bool resolved1 = false, resolved2 = false;
+  auto errorPaf = kj::newPromiseAndFulfiller<test::TestInterface::Client>();
+  test::TestInterface::Client errorPromise = kj::mv(errorPaf.promise);
+
+  bool resolved1 = false, resolved2 = false, resolved3 = false;
   auto promise1 = set1.getLocalServer(clientPromise)
       .then([&](kj::Maybe<test::TestInterface::Server&> server) {
     resolved1 = true;
@@ -875,6 +878,13 @@ TEST(Capability, CapabilityServerSet) {
     resolved2 = true;
     EXPECT_TRUE(server == nullptr);
   });
+  auto promise3 = set1.getLocalServer(errorPromise)
+      .then([&](kj::Maybe<test::TestInterface::Server&> server) {
+    KJ_FAIL_EXPECT("getLocalServer() on error promise should have thrown");
+  }, [&](kj::Exception&& e) {
+    resolved3 = true;
+    KJ_EXPECT(e.getDescription().endsWith("foo"), e.getDescription());
+  });
 
   kj::evalLater([](){}).wait(waitScope);
   kj::evalLater([](){}).wait(waitScope);
@@ -883,14 +893,18 @@ TEST(Capability, CapabilityServerSet) {
 
   EXPECT_FALSE(resolved1);
   EXPECT_FALSE(resolved2);
+  EXPECT_FALSE(resolved3);
 
   paf.fulfiller->fulfill(kj::cp(client1));
+  errorPaf.fulfiller->reject(KJ_EXCEPTION(FAILED, "foo"));
 
   promise1.wait(waitScope);
   promise2.wait(waitScope);
+  promise3.wait(waitScope);
 
   EXPECT_TRUE(resolved1);
   EXPECT_TRUE(resolved2);
+  EXPECT_TRUE(resolved3);
 
   // Check weak pointer.
   {

c++/src/capnp/capability.c++

@@ -53,6 +53,8 @@ public:
 
 static BrokenCapFactoryImpl brokenCapFactory;
 
+static kj::Own<ClientHook> newNullCap();
+
 }  // namespace
 
 ClientHook::ClientHook() {
@@ -71,7 +73,7 @@ void MessageReader::initCapTable(kj::Array<kj::Maybe<kj::Own<ClientHook>>> capTa
 // =======================================================================================
 
 Capability::Client::Client(decltype(nullptr))
-    : hook(newBrokenCap("Called null capability.")) {}
+    : hook(newNullCap()) {}
 
 Capability::Client::Client(kj::Exception&& exception)
     : hook(newBrokenCap(kj::mv(exception))) {}
@@ -603,9 +605,10 @@ public:
 
 class BrokenClient final: public ClientHook, public kj::Refcounted {
 public:
-  BrokenClient(const kj::Exception& exception): exception(exception) {}
-  BrokenClient(const kj::StringPtr description)
-      : exception(kj::Exception::Type::FAILED, "", 0, kj::str(description)) {}
+  BrokenClient(const kj::Exception& exception, bool resolved)
+      : exception(exception), resolved(resolved) {}
+  BrokenClient(const kj::StringPtr description, bool resolved)
+      : exception(kj::Exception::Type::FAILED, "", 0, kj::str(description)), resolved(resolved) {}
 
   Request<AnyPointer, AnyPointer> newCall(
       uint64_t interfaceId, uint16_t methodId, kj::Maybe<MessageSize> sizeHint) override {
@@ -622,8 +625,12 @@ public:
   }
 
   kj::Maybe<kj::Promise<kj::Own<ClientHook>>> whenMoreResolved() override {
+    if (resolved) {
+      return nullptr;
+    } else {
       return kj::Promise<kj::Own<ClientHook>>(kj::cp(exception));
     }
+  }
 
   kj::Own<ClientHook> addRef() override {
     return kj::addRef(*this);
@@ -635,20 +642,26 @@ public:
 
 private:
   kj::Exception exception;
+  bool resolved;
 };
 
 kj::Own<ClientHook> BrokenPipeline::getPipelinedCap(kj::ArrayPtr<const PipelineOp> ops) {
-  return kj::refcounted<BrokenClient>(exception);
+  return kj::refcounted<BrokenClient>(exception, false);
+}
+
+kj::Own<ClientHook> newNullCap() {
+  // A null capability, unlike other broken capabilities, is considered resolved.
+  return kj::refcounted<BrokenClient>("Called null capability.", true);
 }
 
 }  // namespace
 
 kj::Own<ClientHook> newBrokenCap(kj::StringPtr reason) {
-  return kj::refcounted<BrokenClient>(reason);
+  return kj::refcounted<BrokenClient>(reason, false);
 }
 
 kj::Own<ClientHook> newBrokenCap(kj::Exception&& reason) {
-  return kj::refcounted<BrokenClient>(kj::mv(reason));
+  return kj::refcounted<BrokenClient>(kj::mv(reason), false);
 }
 
 kj::Own<PipelineHook> newBrokenPipeline(kj::Exception&& reason) {
@@ -705,9 +718,6 @@ kj::Promise<void*> CapabilityServerSetBase::getLocalServerInternal(Capability::C
         .then([this](kj::Own<ClientHook>&& resolved) {
       Capability::Client client(kj::mv(resolved));
       return getLocalServerInternal(client);
-    }, [](kj::Exception&&) -> void* {
-      // A broken promise is simply not a local capability.
-      return nullptr;
     });
   } else {
     return hook->getLocalServer(*this);