try/catch in HandleOKCallback

@mapsam

We should add try/catch around code in the HandleOKCallback like

node-cpp-skel/src/object_async/hello_async.cpp

Lines 179 to 186 in c3d13e7

    
           Nan::HandleScope scope; 
        
           const auto argc = 2u; 
        
           v8::Local<v8::Value> argv[argc] = { 
        
               Nan::Null(), Nan::New<v8::String>(result_).ToLocalChecked()}; 
        
           // Static cast done here to avoid 'cppcoreguidelines-pro-bounds-array-to-pointer-decay' warning with clang-tidy 
        
           callback->Call(argc, static_cast<v8::Local<v8::Value>*>(argv));

. Otherwise down stream developers are likely to:

add new code to the HandleOKCallback
in many instances that code might throw
a throw will not be caught automatically and will instead crash the process with an abort

An example is mapbox/vtquery#69 /cc @mapsam

We must protect against crashes like this, so I think its worth adding try/catch around the code in node-cpp-skel. Then users would be forced (due to the lack of coverage of the catch to think hard about whether an exception is possible.

Navigation Menu

Repository navigation

try/catch in HandleOKCallback #114

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	Nan::HandleScope scope;

	const auto argc = 2u;
	v8::Local<v8::Value> argv[argc] = {
	Nan::Null(), Nan::New<v8::String>(result_).ToLocalChecked()};

	// Static cast done here to avoid 'cppcoreguidelines-pro-bounds-array-to-pointer-decay' warning with clang-tidy
	callback->Call(argc, static_cast<v8::Local<v8::Value>*>(argv));

try/catch in HandleOKCallback #114

Description

Activity

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions