Merge pull request #372 from cloudflare/bcaimano/alarm-in-ctor

Allow setAlarm to be invoked in Durable Object constructors

Author: bcaimano

src/workerd/api/actor-state.c++

@@ -231,11 +231,8 @@ jsg::Promise<jsg::Value> DurableObjectStorageOperations::getOne(
 
 jsg::Promise<kj::Maybe<double>> DurableObjectStorageOperations::getAlarm(
     jsg::Optional<GetAlarmOptions> maybeOptions, v8::Isolate* isolate) {
-
-  if (!IoContext::current().getActorOrThrow().hasAlarmHandler()) {
-    return jsg::resolvedPromise<kj::Maybe<double>>(isolate, nullptr);
-  }
-
+  // Even if we do not have an alarm handler, we might once have had one. It's fine to return
+  // whatever a previous alarm setting or a falsy result.
   auto options = configureOptions(maybeOptions.map([](auto& o) {
     return GetOptions {
       .allowConcurrency = o.allowConcurrency,
@@ -400,8 +397,11 @@ jsg::Promise<void> DurableObjectStorageOperations::setAlarm(kj::Date scheduledTi
   JSG_REQUIRE(scheduledTime > kj::origin<kj::Date>(), TypeError,
     "setAlarm() cannot be called with an alarm time <= 0");
 
-  JSG_REQUIRE(IoContext::current().getActorOrThrow().hasAlarmHandler(), TypeError,
-    "Your Durable Object class must have an alarm() handler in order to call setAlarm()");
+  // This doesn't check if we have an alarm handler per say. It checks if we have an initialized
+  // (post-ctor) JS durable object with an alarm handler. Notably, this means this won't throw if
+  // `setAlarm` is invoked in the DO ctor even if the DO class does not have an alarm handler. This
+  // is better than throwing even if we do have an alarm handler.
+  IoContext::current().getActorOrThrow().assertCanSetAlarm();
 
   auto options = configureOptions(maybeOptions.map([](auto& o) {
     return PutOptions {
@@ -470,6 +470,8 @@ kj::OneOf<jsg::Promise<bool>, jsg::Promise<int>> DurableObjectStorageOperations:
 
 jsg::Promise<void> DurableObjectStorageOperations::deleteAlarm(
     jsg::Optional<SetAlarmOptions> maybeOptions, v8::Isolate* isolate) {
+  // Even if we do not have an alarm handler, we might once have had one. It's fine to remove that
+  // alarm or noop on the absence of one.
   auto options = configureOptions(maybeOptions.map([](auto& o) {
     return PutOptions {
       .allowConcurrency = o.allowConcurrency,

src/workerd/io/worker.c++

@@ -2821,8 +2821,33 @@ kj::Maybe<jsg::Ref<api::DurableObjectStorage>>
   });
 }
 
-bool Worker::Actor::hasAlarmHandler() {
-  return getHandler().map([](auto& h) { return h.alarm != nullptr; }).orDefault(false);
+void Worker::Actor::assertCanSetAlarm() {
+  KJ_SWITCH_ONEOF(impl->classInstance) {
+    KJ_CASE_ONEOF(_, Impl::NoClass) {
+      // Once upon a time, we allowed actors without classes. Let's make a nicer message if we
+      // we somehow see a classless actor attempt to run an alarm in the wild.
+      JSG_FAIL_REQUIRE(TypeError,
+          "Your Durable Object must be class-based in order to call setAlarm()");
+    }
+    KJ_CASE_ONEOF(_, DurableObjectConstructor*) {
+      KJ_FAIL_ASSERT("setAlarm() invoked before Durable Object ctor");
+    }
+    KJ_CASE_ONEOF(_, Impl::Initializing) {
+      // We don't explicitly know if we have an alarm handler or not, so just let it happen. We'll
+      // handle it when we go to run the alarm.
+      return;
+    }
+    KJ_CASE_ONEOF(handler, api::ExportedHandler) {
+      JSG_REQUIRE(handler.alarm != nullptr, TypeError,
+          "Your Durable Object class must have an alarm() handler in order to call setAlarm()");
+      return;
+    }
+    KJ_CASE_ONEOF(exception, kj::Exception) {
+      // We've failed in the ctor, might as well just throw that exception for now.
+      kj::throwFatalException(kj::cp(exception));
+    }
+  }
+  KJ_UNREACHABLE;
 }
 
 kj::Promise<void> Worker::Actor::makeAlarmTaskForPreview(kj::Date scheduledTime) {

src/workerd/io/worker.h

@@ -703,7 +703,7 @@ class Worker::Actor final: public kj::Refcounted {
 
   const Worker& getWorker() { return *worker; }
 
-  bool hasAlarmHandler();
+  void assertCanSetAlarm();
   kj::Promise<void> makeAlarmTaskForPreview(kj::Date scheduledTime);
 
   kj::Promise<WorkerInterface::AlarmResult> dedupAlarm(