FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: yield empty maybes for failed AsyncWrap::MakeCallback calls · nodejs/node@198cf41 · GitHub

/ node Public

Commit 198cf41

Browse files
addaleax
authored and
targos
committed
src: yield empty maybes for failed AsyncWrap::MakeCallback calls
Use an empty `MaybeLocal<Value>` as the return value for `AsyncWrap::MakeCallback()` if an exception occurred, regardless of `MakeCallback` depth. Unfortunately, the public API can not make this switch without a breaking change (and possibly some kind of deprecation/renaming). PR-URL: #22078 Backport-PR-URL: #22505 Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 02e3daa commit 198cf41

6 files changed

Lines changed: 22 additions & 19 deletions

File tree

‎src/callback_scope.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
5959
AsyncWrap::EmitBefore(env, asyncContext.async_id);
6060
}
6161

62-
if (!IsInnerMakeCallback()) {
62+
CHECK_GE(env->makecallback_depth(), 1);
63+
if (env->makecallback_depth() == 1) {
6364
env->tick_info()->set_has_thrown(false);
6465
}
6566

@@ -91,7 +92,7 @@ void InternalCallbackScope::Close() {
9192
AsyncWrap::EmitAfter(env_, async_context_.async_id);
9293
}
9394

94-
if (IsInnerMakeCallback()) {
95+
if (env_->makecallback_depth() > 1) {
9596
return;
9697
}
9798

‎src/env-inl.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,8 @@ inline Environment::AsyncCallbackScope::~AsyncCallbackScope() {
218218
env_->makecallback_cntr_--;
219219
}
220220

221-
inline bool Environment::AsyncCallbackScope::in_makecallback() const {
222-
return env_->makecallback_cntr_ > 1;
221+
inline size_t Environment::makecallback_depth() const {
222+
return makecallback_cntr_;
223223
}
224224

225225
inline Environment::ImmediateInfo::ImmediateInfo(v8::Isolate* isolate)

‎src/env.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,14 +503,15 @@ class Environment {
503503
AsyncCallbackScope() = delete;
504504
explicit AsyncCallbackScope(Environment* env);
505505
~AsyncCallbackScope();
506-
inline bool in_makecallback() const;
507506

508507
private:
509508
Environment* env_;
510509

511510
DISALLOW_COPY_AND_ASSIGN(AsyncCallbackScope);
512511
};
513512

513+
inline size_t makecallback_depth() const;
514+
514515
class ImmediateInfo {
515516
public:
516517
inline AliasedBuffer<uint32_t, v8::Uint32Array>& fields();

‎src/node.cc‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ MaybeLocal<Value> InternalMakeCallback(Environment* env,
758758
CHECK(!recv.IsEmpty());
759759
InternalCallbackScope scope(env, recv, asyncContext);
760760
if (scope.Failed()) {
761-
return Undefined(env->isolate());
761+
return MaybeLocal<Value>();
762762
}
763763

764764
Local<Function> domain_cb = env->domain_callback();
@@ -773,15 +773,13 @@ MaybeLocal<Value> InternalMakeCallback(Environment* env,
773773
}
774774

775775
if (ret.IsEmpty()) {
776-
// NOTE: For backwards compatibility with public API we return Undefined()
777-
// if the top level call threw.
778776
scope.MarkAsFailed();
779-
return scope.IsInnerMakeCallback() ? ret : Undefined(env->isolate());
777+
return MaybeLocal<Value>();
780778
}
781779

782780
scope.Close();
783781
if (scope.Failed()) {
784-
return Undefined(env->isolate());
782+
return MaybeLocal<Value>();
785783
}
786784

787785
return ret;
@@ -833,8 +831,14 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
833831
// the two contexts need not be the same.
834832
Environment* env = Environment::GetCurrent(callback->CreationContext());
835833
Context::Scope context_scope(env->context());
836-
return InternalMakeCallback(env, recv, callback,
837-
argc, argv, asyncContext);
834+
MaybeLocal<Value> ret = InternalMakeCallback(env, recv, callback,
835+
argc, argv, asyncContext);
836+
if (ret.IsEmpty() && env->makecallback_depth() == 0) {
837+
// This is only for legacy compatiblity and we may want to look into
838+
// removing/adjusting it.
839+
return Undefined(env->isolate());
840+
}
841+
return ret;
838842
}
839843

840844

‎src/node_internals.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -543,9 +543,6 @@ class InternalCallbackScope {
543543

544544
inline bool Failed() const { return failed_; }
545545
inline void MarkAsFailed() { failed_ = true; }
546-
inline bool IsInnerMakeCallback() const {
547-
return callback_scope_.in_makecallback();
548-
}
549546

550547
private:
551548
Environment* env_;

‎src/timer_wrap.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ using v8::FunctionTemplate;
3737
using v8::HandleScope;
3838
using v8::Integer;
3939
using v8::Local;
40+
using v8::MaybeLocal;
4041
using v8::Object;
4142
using v8::String;
4243
using v8::Value;
@@ -138,13 +139,12 @@ class TimerWrap : public HandleWrap {
138139
Environment* env = wrap->env();
139140
HandleScope handle_scope(env->isolate());
140141
Context::Scope context_scope(env->context());
141-
Local<Value> ret;
142+
MaybeLocal<Value> ret;
142143
Local<Value> args[1];
143144
do {
144145
args[0] = env->GetNow();
145-
ret = wrap->MakeCallback(env->timers_callback_function(), 1, args)
146-
.ToLocalChecked();
147-
} while (ret->IsUndefined() &&
146+
ret = wrap->MakeCallback(env->timers_callback_function(), 1, args);
147+
} while ((ret.IsEmpty() || ret.ToLocalChecked()->IsUndefined()) &&
148148
!env->tick_info()->has_thrown() &&
149149
env->can_call_into_js() &&
150150
wrap->object()->Get(env->context(),

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL