Skip to content

Commit 3641d33

Browse files
jasnelljasnell
committed
src: improve error handling in multiple files
* node_http_parser * node_http2 * node_builtins PR-URL: #57507 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
1 parent df883b0 commit 3641d33

File tree

3 files changed

+43
-47
lines changed

3 files changed

+43
-47
lines changed

‎src/node_builtins.cc

+28-37
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,18 @@
1010
namespace node {
1111
namespace builtins {
1212

13+
using v8::Boolean;
1314
using v8::Context;
1415
using v8::EscapableHandleScope;
16+
using v8::Exception;
1517
using v8::Function;
1618
using v8::FunctionCallbackInfo;
1719
using v8::IntegrityLevel;
1820
using v8::Isolate;
1921
using v8::Local;
2022
using v8::MaybeLocal;
2123
using v8::Name;
24+
using v8::NewStringType;
2225
using v8::None;
2326
using v8::Object;
2427
using v8::ObjectTemplate;
@@ -28,6 +31,7 @@ using v8::ScriptOrigin;
2831
using v8::Set;
2932
using v8::SideEffectType;
3033
using v8::String;
34+
using v8::TryCatch;
3135
using v8::Undefined;
3236
using v8::Value;
3337

@@ -200,11 +204,11 @@ MaybeLocal<String> BuiltinLoader::LoadBuiltinSource(Isolate* isolate,
200204
uv_strerror(r),
201205
filename);
202206
Local<String> message = OneByteString(isolate, buf);
203-
isolate->ThrowException(v8::Exception::Error(message));
207+
isolate->ThrowException(Exception::Error(message));
204208
return MaybeLocal<String>();
205209
}
206210
return String::NewFromUtf8(
207-
isolate, contents.c_str(), v8::NewStringType::kNormal, contents.length());
211+
isolate, contents.c_str(), NewStringType::kNormal, contents.length());
208212
#endif // NODE_BUILTIN_MODULES_PATH
209213
}
210214

@@ -528,7 +532,7 @@ bool BuiltinLoader::CompileAllBuiltinsAndCopyCodeCache(
528532
to_eager_compile_.emplace(id);
529533
}
530534

531-
v8::TryCatch bootstrapCatch(context->GetIsolate());
535+
TryCatch bootstrapCatch(context->GetIsolate());
532536
auto fn = LookupAndCompile(context, id.data(), nullptr);
533537
if (bootstrapCatch.HasCaught()) {
534538
per_process::Debug(DebugCategory::CODE_CACHE,
@@ -581,18 +585,15 @@ void BuiltinLoader::GetBuiltinCategories(
581585
Local<Value> can_be_required_js;
582586

583587
if (!ToV8Value(context, builtin_categories.cannot_be_required)
584-
.ToLocal(&cannot_be_required_js))
585-
return;
586-
if (result
588+
.ToLocal(&cannot_be_required_js) ||
589+
result
587590
->Set(context,
588591
FIXED_ONE_BYTE_STRING(isolate, "cannotBeRequired"),
589592
cannot_be_required_js)
590-
.IsNothing())
591-
return;
592-
if (!ToV8Value(context, builtin_categories.can_be_required)
593-
.ToLocal(&can_be_required_js))
594-
return;
595-
if (result
593+
.IsNothing() ||
594+
!ToV8Value(context, builtin_categories.can_be_required)
595+
.ToLocal(&can_be_required_js) ||
596+
result
596597
->Set(context,
597598
FIXED_ONE_BYTE_STRING(isolate, "canBeRequired"),
598599
can_be_required_js)
@@ -612,34 +613,22 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
612613
Local<Value> builtins_without_cache_js;
613614
Local<Value> builtins_in_snapshot_js;
614615
if (!ToV8Value(context, realm->builtins_with_cache)
615-
.ToLocal(&builtins_with_cache_js)) {
616-
return;
617-
}
618-
if (result
616+
.ToLocal(&builtins_with_cache_js) ||
617+
result
619618
->Set(context,
620619
FIXED_ONE_BYTE_STRING(isolate, "compiledWithCache"),
621620
builtins_with_cache_js)
622-
.IsNothing()) {
623-
return;
624-
}
625-
626-
if (!ToV8Value(context, realm->builtins_without_cache)
627-
.ToLocal(&builtins_without_cache_js)) {
628-
return;
629-
}
630-
if (result
621+
.IsNothing() ||
622+
!ToV8Value(context, realm->builtins_without_cache)
623+
.ToLocal(&builtins_without_cache_js) ||
624+
result
631625
->Set(context,
632626
FIXED_ONE_BYTE_STRING(isolate, "compiledWithoutCache"),
633627
builtins_without_cache_js)
634-
.IsNothing()) {
635-
return;
636-
}
637-
638-
if (!ToV8Value(context, realm->builtins_in_snapshot)
639-
.ToLocal(&builtins_in_snapshot_js)) {
640-
return;
641-
}
642-
if (result
628+
.IsNothing() ||
629+
!ToV8Value(context, realm->builtins_in_snapshot)
630+
.ToLocal(&builtins_in_snapshot_js) ||
631+
result
643632
->Set(context,
644633
FIXED_ONE_BYTE_STRING(isolate, "compiledInSnapshot"),
645634
builtins_in_snapshot_js)
@@ -656,8 +645,10 @@ void BuiltinLoader::BuiltinIdsGetter(Local<Name> property,
656645
Isolate* isolate = env->isolate();
657646

658647
auto ids = env->builtin_loader()->GetBuiltinIds();
659-
info.GetReturnValue().Set(
660-
ToV8Value(isolate->GetCurrentContext(), ids).ToLocalChecked());
648+
Local<Value> ret;
649+
if (ToV8Value(isolate->GetCurrentContext(), ids).ToLocal(&ret)) {
650+
info.GetReturnValue().Set(ret);
651+
}
661652
}
662653

663654
void BuiltinLoader::ConfigStringGetter(
@@ -693,7 +684,7 @@ void BuiltinLoader::CompileFunction(const FunctionCallbackInfo<Value>& args) {
693684
void BuiltinLoader::HasCachedBuiltins(const FunctionCallbackInfo<Value>& args) {
694685
auto instance = Environment::GetCurrent(args)->builtin_loader();
695686
RwLock::ScopedReadLock lock(instance->code_cache_->mutex);
696-
args.GetReturnValue().Set(v8::Boolean::New(
687+
args.GetReturnValue().Set(Boolean::New(
697688
args.GetIsolate(), instance->code_cache_->has_code_cache));
698689
}
699690

‎src/node_http2.cc

+10-6
Original file line numberDiff line numberDiff line change
@@ -2928,9 +2928,11 @@ void Http2Session::UpdateChunksSent(const FunctionCallbackInfo<Value>& args) {
29282928

29292929
uint32_t length = session->chunks_sent_since_last_write_;
29302930

2931-
session->object()->Set(env->context(),
2932-
env->chunks_sent_since_last_write_string(),
2933-
Integer::NewFromUnsigned(isolate, length)).Check();
2931+
if (session->object()->Set(env->context(),
2932+
env->chunks_sent_since_last_write_string(),
2933+
Integer::NewFromUnsigned(isolate, length)).IsNothing()) {
2934+
return;
2935+
}
29342936

29352937
args.GetReturnValue().Set(length);
29362938
}
@@ -3109,11 +3111,13 @@ void Http2Session::AltSvc(const FunctionCallbackInfo<Value>& args) {
31093111
int32_t id = args[0]->Int32Value(env->context()).ToChecked();
31103112

31113113
// origin and value are both required to be ASCII, handle them as such.
3112-
Local<String> origin_str = args[1]->ToString(env->context()).ToLocalChecked();
3113-
Local<String> value_str = args[2]->ToString(env->context()).ToLocalChecked();
3114+
Local<String> origin_str;
3115+
Local<String> value_str;
31143116

3115-
if (origin_str.IsEmpty() || value_str.IsEmpty())
3117+
if (!args[1]->ToString(env->context()).ToLocal(&origin_str) ||
3118+
!args[2]->ToString(env->context()).ToLocal(&value_str)) {
31163119
return;
3120+
}
31173121

31183122
size_t origin_len = origin_str->Length();
31193123
size_t value_len = value_str->Length();

‎src/node_http_parser.cc

+5-4
Original file line numberDiff line numberDiff line change
@@ -736,12 +736,13 @@ class Parser : public AsyncWrap, public StreamListener {
736736
Parser* parser;
737737
ASSIGN_OR_RETURN_UNWRAP(&parser, args.This());
738738

739-
Local<Object> ret = Buffer::Copy(
739+
Local<Object> ret;
740+
if (Buffer::Copy(
740741
parser->env(),
741742
parser->current_buffer_data_,
742-
parser->current_buffer_len_).ToLocalChecked();
743-
744-
args.GetReturnValue().Set(ret);
743+
parser->current_buffer_len_).ToLocal(&ret)) {
744+
args.GetReturnValue().Set(ret);
745+
}
745746
}
746747

747748
static void Duration(const FunctionCallbackInfo<Value>& args) {

0 commit comments

Comments
 (0)