Repository navigation
Can't use node::FatalError() #232
Description
... because node_internals.h is not available outside of node. So, if we want node-api to contain unaltered versions of the implementation, we must not rely on any node internals.
I guess we could simply throw a JS exception. Since Isolate::GetCurrent() is going away, and since we don't have an env from which to extract an isolate, is there some other way we can signal that we wish to throw an exception?
Activity
bnoordhuis commentedon Apr 17, 2017
FatalError() is a noreturn function. Its n-api equivalent shouldn't throw an exception, it should exit.
addaleax commentedon Apr 17, 2017
Just copying Node’s
FatalError, i.e. fprintf() + fflush() + abort(), sounds good to me too.Just curious, is that publicly announced anywhere?
bnoordhuis commentedon Apr 17, 2017
Not really, but V8 people have repeatedly indicated over the years they want to get rid of it.
gabrielschulhof commentedon Apr 17, 2017
@addaleax well, it's actually more than that, because in Windows it's
raise(SIGABRT), and not fprintf() butWriteConsoleW(), etc. So, there's quite a bit of code innode.ccdealing with this.gabrielschulhof commentedon Apr 17, 2017
@bnoordhuis we were originally debating whether we should consider the absence of an
enva fatal error or not. There was a good argument for "not" in that this is just a module, so perhaps the whole app shouldn't be shot down because of a buggy module. The reason we went with the "aye" in the end was that it's very much a corner case anyway.gabrielschulhof commentedon Apr 17, 2017
But without an
env, and hence without an isolate, we can't even throw an exception.gabrielschulhof commentedon Apr 17, 2017
We can wrap the call to
node::FatalError()in#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALSto avoid failure to compile outside of node, but we should really have something in the#elsecase.jasongin commentedon Apr 17, 2017
I'm still not convinced we need
FatalError()functionality. The one place where it is used now could return anapi_statuserror instead.jasongin commentedon Apr 17, 2017
Related: nodejs/node-addon-api#25
n-api: provide node::FatalError() when internals are absent
jasongin commentedon May 4, 2017
We might want to actually expose
node::FatalError()(or similar) as anapi_fatal_error()API. See nodejs/node-addon-api#32 (comment) for a scenario where it is needed.mhdawson commentedon Jun 26, 2017
nodejs/node#13927
digitalinfinity commentedon Aug 31, 2017
Addressed by nodejs/node#13971