std::exception catching once more #1555
Comments
|
In today's Node-API meeting, we discussed about loosing the type constraint on the default catch clause and avoid leaking the c++ exceptions to Node.js core because exceptions are only supported in node-addon-api. We may add a handler/configuration/macro to allow setting custom arbitrary exception handler so that if poeple still like the addon to crash if exception is unhandled can be still achievable: https://github.com/nodejs/node-addon-api/blob/main/napi-inl.h#L94-L128. We also agreed that it will not be considered as a breaking change if the default to changed to catch all exceptions and throw as JS exceptions. |
|
@kamil321 would you like to working on it? |
|
@legendecas thank you, that's a great news. But about working on it - heart says 'sure', brain screams 'nooo'. See, I'm mostly a TS dev and this is my first dip into cpp after like 7 years or so. Even in my local patch I've encountered a pretty serious wall: to construct a Napi:Error in a potential handler, we apparently need some napi_env. So I added it as a param to both WrapCallback and WrapVoidCallback and tried to pass it in every place. It worked in most cases, but sometimes I got a random nullptr instead of env, or some exception related to unwrapping the callback. If I spent another month on it, then maybe I could fix it, but I guess someone familiar with Napi internals can do it in 10 minutes. So, if there is any brave warrior up for a challenge - I don't have neither a castle nor a princess to offer, but I can promise my eternal appreciation. |
Hi there. I might be beating a dead horse here (#601, #1225), but I have to try. I've got some bindings with thousands of functions and a lot of them like to throw exceptions. I was pretty shocked to find out that node just shuts down without any message when there's anything wrong.
And that's of course because of the callback wrappers catching Napi::Error and nothing more.
I - kind of - understand that touching unknown exceptions sounds bad, but shutting down everything with neither a message nor a possibility to catch on the node end seems much, much worse. Leaving the choice to the developers is probably a good call, but not if they have to write the same try-catch 82394723 times.
I ended up modifying the header just like in this comment (e.what from std:exc, simple message from unknowns): #601 (comment) and some inner peace returned. At least until the next version upgrade.
Is this the perfect solution and should find its way into the api? No idea. But it could just lay there, hidden behind a global. Or maybe Napi could provide a way to inject a generic exception handler?
Thanks in advance for reconsidering.
The text was updated successfully, but these errors were encountered: