We should add try/catch around code in the HandleOKCallback like
|
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.
Reactions are currently unavailable
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
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.