Commit 353587f
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 92428c2 commit 353587f
1 file changed
Lines changed: 28 additions & 15 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -13,6 +13,7 @@ using v8::HandleScope; | |||
| 13 | 13 | using v8::Isolate; | |
| 14 | 14 | using v8::Local; | |
| 15 | 15 | using v8::MaybeLocal; | |
| 16 | + using v8::Number; | ||
| 16 | 17 | using v8::Object; | |
| 17 | 18 | using v8::String; | |
| 18 | 19 | using v8::Undefined; | |
@@ -51,7 +52,7 @@ InternalCallbackScope::InternalCallbackScope(Environment* env, | |||
| 51 | 52 | Local<Object> object, | |
| 52 | 53 | const async_context& asyncContext, | |
| 53 | 54 | int flags, | |
| 54 | - v8::Local<v8::Value> context_frame) | ||
| 55 | + Local<Value> context_frame) | ||
| 55 | 56 | : env_(env), | |
| 56 | 57 | async_context_(asyncContext), | |
| 57 | 58 | object_(object), | |
@@ -216,7 +217,7 @@ MaybeLocal<Value> InternalMakeCallback(Environment* env, | |||
| 216 | 217 | Local<Context> context = env->context(); | |
| 217 | 218 | if (use_async_hooks_trampoline) { | |
| 218 | 219 | 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); | ||
| 220 | 221 | args[1] = resource; | |
| 221 | 222 | args[2] = callback; | |
| 222 | 223 | for (int i = 0; i < argc; i++) { | |
@@ -248,8 +249,10 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate, | |||
| 248 | 249 | int argc, | |
| 249 | 250 | Local<Value> argv[], | |
| 250 | 251 | 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 | + } | ||
| 253 | 256 | return MakeCallback(isolate, recv, method_string, argc, argv, asyncContext); | |
| 254 | 257 | } | |
| 255 | 258 | ||
@@ -260,13 +263,18 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate, | |||
| 260 | 263 | Local<Value> argv[], | |
| 261 | 264 | async_context asyncContext) { | |
| 262 | 265 | // 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); | ||
| 264 | 271 | CHECK_NOT_NULL(env); | |
| 265 | - if (!env->can_call_into_js()) return Local<Value>(); | ||
| 272 | + if (!env->can_call_into_js()) return {}; | ||
| 266 | 273 | ||
| 267 | 274 | 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 | + } | ||
| 270 | 278 | if (!callback_v->IsFunction()) { | |
| 271 | 279 | // This used to return an empty value, but Undefined() makes more sense | |
| 272 | 280 | // since no exception is pending here. | |
@@ -300,8 +308,11 @@ MaybeLocal<Value> InternalMakeCallback(Isolate* isolate, | |||
| 300 | 308 | // | |
| 301 | 309 | // Because of the AssignToContext() call in src/node_contextify.cc, | |
| 302 | 310 | // 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); | ||
| 305 | 316 | CHECK_NOT_NULL(env); | |
| 306 | 317 | Context::Scope context_scope(env->context()); | |
| 307 | 318 | MaybeLocal<Value> ret = InternalMakeCallback( | |
@@ -323,12 +334,14 @@ MaybeLocal<Value> MakeSyncCallback(Isolate* isolate, | |||
| 323 | 334 | Local<Function> callback, | |
| 324 | 335 | int argc, | |
| 325 | 336 | 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); | ||
| 328 | 342 | CHECK_NOT_NULL(env); | |
| 329 | - if (!env->can_call_into_js()) return Local<Value>(); | ||
| 343 | + if (!env->can_call_into_js()) return {}; | ||
| 330 | 344 | ||
| 331 | - Local<Context> context = env->context(); | ||
| 332 | 345 | Context::Scope context_scope(context); | |
| 333 | 346 | if (env->async_callback_scope_depth()) { | |
| 334 | 347 | // There's another MakeCallback() on the stack, piggy back on it. | |
@@ -345,7 +358,7 @@ MaybeLocal<Value> MakeSyncCallback(Isolate* isolate, | |||
| 345 | 358 | argc, | |
| 346 | 359 | argv, | |
| 347 | 360 | async_context{0, 0}, | |
| 348 | - v8::Undefined(isolate)); | ||
| 361 | + Undefined(isolate)); | ||
| 349 | 362 | return ret; | |
| 350 | 363 | } | |
| 351 | 364 | ||
0 commit comments