Skip to content

Commit 9611980

Browse files
jasnelladuh95
jasnell
authored and
aduh95
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 d0c96c4 commit 9611980

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

@@ -201,11 +205,11 @@ MaybeLocal<String> BuiltinLoader::LoadBuiltinSource(Isolate* isolate,
201205
uv_strerror(r),
202206
filename);
203207
Local<String> message = OneByteString(isolate, buf);
204-
isolate->ThrowException(v8::Exception::Error(message));
208+
isolate->ThrowException(Exception::Error(message));
205209
return MaybeLocal<String>();
206210
}
207211
return String::NewFromUtf8(
208-
isolate, contents.c_str(), v8::NewStringType::kNormal, contents.length());
212+
isolate, contents.c_str(), NewStringType::kNormal, contents.length());
209213
#endif // NODE_BUILTIN_MODULES_PATH
210214
}
211215

@@ -529,7 +533,7 @@ bool BuiltinLoader::CompileAllBuiltinsAndCopyCodeCache(
529533
to_eager_compile_.emplace(id);
530534
}
531535

532-
v8::TryCatch bootstrapCatch(context->GetIsolate());
536+
TryCatch bootstrapCatch(context->GetIsolate());
533537
auto fn = LookupAndCompile(context, id.data(), nullptr);
534538
if (bootstrapCatch.HasCaught()) {
535539
per_process::Debug(DebugCategory::CODE_CACHE,
@@ -582,18 +586,15 @@ void BuiltinLoader::GetBuiltinCategories(
582586
Local<Value> can_be_required_js;
583587

584588
if (!ToV8Value(context, builtin_categories.cannot_be_required)
585-
.ToLocal(&cannot_be_required_js))
586-
return;
587-
if (result
589+
.ToLocal(&cannot_be_required_js) ||
590+
result
588591
->Set(context,
589592
FIXED_ONE_BYTE_STRING(isolate, "cannotBeRequired"),
590593
cannot_be_required_js)
591-
.IsNothing())
592-
return;
593-
if (!ToV8Value(context, builtin_categories.can_be_required)
594-
.ToLocal(&can_be_required_js))
595-
return;
596-
if (result
594+
.IsNothing() ||
595+
!ToV8Value(context, builtin_categories.can_be_required)
596+
.ToLocal(&can_be_required_js) ||
597+
result
597598
->Set(context,
598599
FIXED_ONE_BYTE_STRING(isolate, "canBeRequired"),
599600
can_be_required_js)
@@ -613,34 +614,22 @@ void BuiltinLoader::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
613614
Local<Value> builtins_without_cache_js;
614615
Local<Value> builtins_in_snapshot_js;
615616
if (!ToV8Value(context, realm->builtins_with_cache)
616-
.ToLocal(&builtins_with_cache_js)) {
617-
return;
618-
}
619-
if (result
617+
.ToLocal(&builtins_with_cache_js) ||
618+
result
620619
->Set(context,
621620
FIXED_ONE_BYTE_STRING(isolate, "compiledWithCache"),
622621
builtins_with_cache_js)
623-
.IsNothing()) {
624-
return;
625-
}
626-
627-
if (!ToV8Value(context, realm->builtins_without_cache)
628-
.ToLocal(&builtins_without_cache_js)) {
629-
return;
630-
}
631-
if (result
622+
.IsNothing() ||
623+
!ToV8Value(context, realm->builtins_without_cache)
624+
.ToLocal(&builtins_without_cache_js) ||
625+
result
632626
->Set(context,
633627
FIXED_ONE_BYTE_STRING(isolate, "compiledWithoutCache"),
634628
builtins_without_cache_js)
635-
.IsNothing()) {
636-
return;
637-
}
638-
639-
if (!ToV8Value(context, realm->builtins_in_snapshot)
640-
.ToLocal(&builtins_in_snapshot_js)) {
641-
return;
642-
}
643-
if (result
629+
.IsNothing() ||
630+
!ToV8Value(context, realm->builtins_in_snapshot)
631+
.ToLocal(&builtins_in_snapshot_js) ||
632+
result
644633
->Set(context,
645634
FIXED_ONE_BYTE_STRING(isolate, "compiledInSnapshot"),
646635
builtins_in_snapshot_js)
@@ -657,8 +646,10 @@ void BuiltinLoader::BuiltinIdsGetter(Local<Name> property,
657646
Isolate* isolate = env->isolate();
658647

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

664655
void BuiltinLoader::ConfigStringGetter(
@@ -694,7 +685,7 @@ void BuiltinLoader::CompileFunction(const FunctionCallbackInfo<Value>& args) {
694685
void BuiltinLoader::HasCachedBuiltins(const FunctionCallbackInfo<Value>& args) {
695686
auto instance = Environment::GetCurrent(args)->builtin_loader();
696687
RwLock::ScopedReadLock lock(instance->code_cache_->mutex);
697-
args.GetReturnValue().Set(v8::Boolean::New(
688+
args.GetReturnValue().Set(Boolean::New(
698689
args.GetIsolate(), instance->code_cache_->has_code_cache));
699690
}
700691

‎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)