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.
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
abortAn 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
catchto think hard about whether an exception is possible.