heisenware / vrpc-js

Asynchronous RPC via MQTT for Javascript
https://vrpc.io
MIT License
20 stars 7 forks source link

Exceptions on a VrpcLocal callback cause process crash #52

Closed cstim closed 2 years ago

cstim commented 3 years ago

Describe the Bug In the Cybus software we were dealing with a vrpc-wrapped C++ extension where we called some method from javascript into C++, but were passing a callback to that method, which then is called from the C++ side once it is finished. As things go, our supplied callback method was unexpectedly throwing an exception.

However, the VrpcLocal location that calls our callback doesn't guard this function call by try-catch, causing the whole process to segfault!

{"level":30,"time":1635256410381,"pid":11,"hostname":"protocol-mapper","service":"protocol-mapper","id":"focasmachineservice-focasConnection","className":"FocasConnection","msg":"Connected FocasConnection"} /snapshot/app/src/protocols/focas/stack/node_modules/vrpc/vrpc/VrpcLocal.js:233 value.apply(null, args) // This is the actual function call

TypeError: Cannot read property 'id' of undefined at callback (/snapshot/app/src/protocols/focas/FocasConnection.js) at EventEmitter.<anonymous> (/snapshot/app/src/protocols/focas/stack/node_modules/vrpc/vrpc/VrpcLocal.js:233:17) at Object.onceWrapper (events.js:422:26) at EventEmitter.emit (events.js:315:20) at /snapshot/app/src/protocols/focas/stack/node_modules/vrpc/vrpc/VrpcLocal.js:63:26

(In that case, we had some stupid typo which caused the TypeError exception, but the main point is that any exception could appear in such a callback.)

Our request is whether VrpcLocal.js line 233 could guard the apply function call with try-catch, so that an exception in the called function is caught, gets an error logged, and life goes on. (vrpc version 2.6.0.)

@bheisen Are you open for such a try-catch around the apply in VrpcLocal?

bheisen commented 2 years ago

Yes, I see it - and I am open for the suggested improvement. Thanks for your PR, I will additionally:

create-issue-branch[bot] commented 2 years ago

Branch bugfix/52-exceptions-on-a-vrpclocal-callback-cause-process-crash created!

cstim commented 2 years ago

... and I think there was one more location when a static function with callback was called, where the try-catch could be applied.