timers: refactor error handling

Instead of using nextTick to process failed lists, just attempt to
process them again from C++ if the process is still alive.

This also allows the removal of domain specific code in timers.

The current behaviour is not quite ideal as it means that all lists
after the failed one will process on an arbitrary nextTick, even if
they're — say — not due to fire for another 2 days...

PR-URL: https://github.com/nodejs/node/pull/18486
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

4 files changed, 24 insertions, 1 deletions

diff --git a/src/env-inl.h b/src/env-inl.h
index 545a92ba17..5643fffb6f 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -264,6 +264,10 @@ inline bool Environment::TickInfo::has_scheduled() const {
   return fields_[kHasScheduled] == 1;
 }
 
+inline bool Environment::TickInfo::has_thrown() const {
+  return fields_[kHasThrown] == 1;
+}
+
 inline bool Environment::TickInfo::has_promise_rejections() const {
   return fields_[kHasPromiseRejections] == 1;
 }
@@ -272,6 +276,10 @@ inline void Environment::TickInfo::promise_rejections_toggle_on() {
   fields_[kHasPromiseRejections] = 1;
 }
 
+inline void Environment::TickInfo::set_has_thrown(bool state) {
+  fields_[kHasThrown] = state ? 1 : 0;
+}
+
 inline void Environment::AssignToContext(v8::Local<v8::Context> context,
                                          const ContextInfo& info) {
   context->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex, this);
diff --git a/src/env.h b/src/env.h
index 2ce7069f47..7e01529161 100644
--- a/src/env.h
+++ b/src/env.h
@@ -484,8 +484,10 @@ class Environment {
     inline AliasedBuffer<uint8_t, v8::Uint8Array>& fields();
     inline bool has_scheduled() const;
     inline bool has_promise_rejections() const;
+    inline bool has_thrown() const;
 
     inline void promise_rejections_toggle_on();
+    inline void set_has_thrown(bool state);
 
    private:
     friend class Environment;  // So we can call the constructor.
@@ -494,6 +496,7 @@ class Environment {
     enum Fields {
       kHasScheduled,
       kHasPromiseRejections,
+      kHasThrown,
       kFieldsCount
     };
 
diff --git a/src/node.cc b/src/node.cc
index dfe4125bd0..4e3638b886 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -937,6 +937,10 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
     AsyncWrap::EmitBefore(env, asyncContext.async_id);
   }
 
+  if (!IsInnerMakeCallback()) {
+    env->tick_info()->set_has_thrown(false);
+  }
+
   env->async_hooks()->push_async_ids(async_context_.async_id,
                                async_context_.trigger_async_id);
   pushed_ids_ = true;
@@ -984,6 +988,7 @@ void InternalCallbackScope::Close() {
   Local<Object> process = env_->process_object();
 
   if (env_->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) {
+    env_->tick_info()->set_has_thrown(true);
     failed_ = true;
   }
 }
diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc
index 1725cf30e7..8b596eb435 100644
--- a/src/timer_wrap.cc
+++ b/src/timer_wrap.cc
@@ -138,7 +138,14 @@ class TimerWrap : public HandleWrap {
     Environment* env = wrap->env();
     HandleScope handle_scope(env->isolate());
     Context::Scope context_scope(env->context());
-    wrap->MakeCallback(kOnTimeout, 0, nullptr);
+    Local<Value> ret;
+    do {
+      ret = wrap->MakeCallback(kOnTimeout, 0, nullptr).ToLocalChecked();
+    } while (ret->IsUndefined() &&
+             !env->tick_info()->has_thrown() &&
+             wrap->object()->Get(env->context(),
+                                 env->owner_string()).ToLocalChecked()
+                                                     ->IsUndefined());
   }
 
   static void Now(const FunctionCallbackInfo<Value>& args) {