Skip to content

Commit cccef20

Browse files
jasnelladuh95
authored andcommitted
src: improve error handling in callback.cc
Avoiding use of v8 `Checked()` APIs where appropriate. Few other style cleanups PR-URL: #57758 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 6718abb commit cccef20

File tree

1 file changed

+28
-15
lines changed

1 file changed

+28
-15
lines changed

src/api/callback.cc

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ using v8::HandleScope;
1313
using v8::Isolate;
1414
using v8::Local;
1515
using v8::MaybeLocal;
16+
using v8::Number;
1617
using v8::Object;
1718
using v8::String;
1819
using v8::Undefined;
@@ -51,7 +52,7 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
5152
Local<Object> object,
5253
const async_context& asyncContext,
5354
int flags,
54-
v8::Local<v8::Value> context_frame)
55+
Local<Value> context_frame)
5556
: env_(env),
5657
async_context_(asyncContext),
5758
object_(object),
@@ -216,7 +217,7 @@ MaybeLocal<Value> InternalMakeCallback(Environment* env,
216217
Local<Context> context = env->context();
217218
if (use_async_hooks_trampoline) {
218219
MaybeStackBuffer<Local<Value>, 16> args(3 + argc);
219-
args[0] = v8::Number::New(env->isolate(), asyncContext.async_id);
220+
args[0] = Number::New(env->isolate(), asyncContext.async_id);
220221
args[1] = resource;
221222
args[2] = callback;
222223
for (int i = 0; i < argc; i++) {
@@ -248,8 +249,10 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
248249
int argc,
249250
Local<Value> argv[],
250251
async_context asyncContext) {
251-
Local<String> method_string =
252-
String::NewFromUtf8(isolate, method).ToLocalChecked();
252+
Local<String> method_string;
253+
if (!String::NewFromUtf8(isolate, method).ToLocal(&method_string)) {
254+
return {};
255+
}
253256
return MakeCallback(isolate, recv, method_string, argc, argv, asyncContext);
254257
}
255258

@@ -260,13 +263,18 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
260263
Local<Value> argv[],
261264
async_context asyncContext) {
262265
// Check can_call_into_js() first because calling Get() might do so.
263-
Environment* env = Environment::GetCurrent(recv->GetCreationContextChecked());
266+
Local<Context> context;
267+
if (!recv->GetCreationContext().ToLocal(&context)) {
268+
return {};
269+
}
270+
Environment* env = Environment::GetCurrent(context);
264271
CHECK_NOT_NULL(env);
265-
if (!env->can_call_into_js()) return Local<Value>();
272+
if (!env->can_call_into_js()) return {};
266273

267274
Local<Value> callback_v;
268-
if (!recv->Get(isolate->GetCurrentContext(), symbol).ToLocal(&callback_v))
269-
return Local<Value>();
275+
if (!recv->Get(isolate->GetCurrentContext(), symbol).ToLocal(&callback_v)) {
276+
return {};
277+
}
270278
if (!callback_v->IsFunction()) {
271279
// This used to return an empty value, but Undefined() makes more sense
272280
// since no exception is pending here.
@@ -300,8 +308,11 @@ MaybeLocal<Value> InternalMakeCallback(Isolate* isolate,
300308
//
301309
// Because of the AssignToContext() call in src/node_contextify.cc,
302310
// the two contexts need not be the same.
303-
Environment* env =
304-
Environment::GetCurrent(callback->GetCreationContextChecked());
311+
Local<Context> context;
312+
if (!callback->GetCreationContext().ToLocal(&context)) {
313+
return {};
314+
}
315+
Environment* env = Environment::GetCurrent(context);
305316
CHECK_NOT_NULL(env);
306317
Context::Scope context_scope(env->context());
307318
MaybeLocal<Value> ret = InternalMakeCallback(
@@ -323,12 +334,14 @@ MaybeLocal<Value> MakeSyncCallback(Isolate* isolate,
323334
Local<Function> callback,
324335
int argc,
325336
Local<Value> argv[]) {
326-
Environment* env =
327-
Environment::GetCurrent(callback->GetCreationContextChecked());
337+
Local<Context> context;
338+
if (!callback->GetCreationContext().ToLocal(&context)) {
339+
return {};
340+
}
341+
Environment* env = Environment::GetCurrent(context);
328342
CHECK_NOT_NULL(env);
329-
if (!env->can_call_into_js()) return Local<Value>();
343+
if (!env->can_call_into_js()) return {};
330344

331-
Local<Context> context = env->context();
332345
Context::Scope context_scope(context);
333346
if (env->async_callback_scope_depth()) {
334347
// There's another MakeCallback() on the stack, piggy back on it.
@@ -345,7 +358,7 @@ MaybeLocal<Value> MakeSyncCallback(Isolate* isolate,
345358
argc,
346359
argv,
347360
async_context{0, 0},
348-
v8::Undefined(isolate));
361+
Undefined(isolate));
349362
return ret;
350363
}
351364

0 commit comments

Comments
 (0)