Error was ignored if ThreadSafeFunction::GetContext() failed

[legendecas]

Currently napi_status was ignored in ThreadSafeFunction::GetContext(). No napi_status shall be ignored as it would unstablize the behavior we'd expect.

Since the function signature doesn't support error returning, an alternative might be aborting on non-napi_ok statuses.

Originally posted by @legendecas in #546

[KevinEady]

Hi team,

I can start work on this if nobody has? (direct ping to @legendecas since he reported this)

[legendecas]

@KevinEady You have it. :P

[KevinEady]

Started working on this and I have some questions here. Specifically, since GetContext() can be called on any thread, I don't really how to handle this specific error. Since it is not running on the Node thread, any Error-creating methods wont work (there is no napi_env) ...

inline ThreadSafeFunction::ConvertibleContext
ThreadSafeFunction::GetContext() const {
  void* context;
  
  napi_status status = napi_get_threadsafe_function_context(*_tsfn, &context);
  NAPI_FATAL_IF_FAILED(status, "ThreadSafeFunction::GetContext", "napi_get_threadsafe_function_context");
  
  return ConvertibleContext({ context });
}

This works, but as a fatal error, will end the process, and does not allow a way for the caller to catch this error -- with NAPI_DISABLE_CPP_EXCEPTIONS I cannot surround my GetContext() call with try/catch ... so I don't know how to continue.

Any suggestions?

[legendecas]

I think aborting would be just fine. Since we don't expect any error on this method. Neither do we have any recover solution on the error situation.

[gabrielschulhof]

Looking at the implementation, napi_get_threadsafe_function_context() will only fail if one of the parameters passed in is NULL. So, since both parameters are under our control, it should never fail – unless, in the future, we introduce some new failure mode, in which case the abort should be perfect.