Inconsistent return value with enabled NODE_ADDON_API_ENABLE_MAYBE directive

[ikokostya]

If preprocessor directives NAPI_DISABLE_CPP_EXCEPTIONS and NODE_ADDON_API_ENABLE_MAYBE are enabled, Napi API calls should return Maybe type https://github.com/nodejs/node-addon-api/blob/HEAD/doc/error_handling.md#handling-errors-with-maybe-type-and-c-exceptions-disabled.

But many Napi methods violate this rule. For example all Napi::Number conversion methods (Int32Value(), Uint32Value(), FloatValue(), DoubleValue()) and Napi::BigInt conversion methods return unwrapped value as is:

node-addon-api/napi-inl.h

Lines 829 to 834 in 39267ba

	inline int32_t Number::Int32Value() const {
	int32_t result;
	napi_status status = napi_get_value_int32(_env, _value, &result);
	NAPI_THROW_IF_FAILED(_env, status, 0);
	return result;
	}

node-addon-api/napi.h

Lines 90 to 94 in 39267ba

	#define NAPI_THROW_IF_FAILED(env, status, ...) \
	if ((status) != napi_ok) { \
	Napi::Error::New(env).ThrowAsJavaScriptException(); \
	return __VA_ARGS__; \
	}

In the following example variable x will have 0 value if info[0] is not a number:

void Foo(const Napi::CallbackInfo& info) {
  auto x = info[0].As<Napi::Number>().Int32Value(); // expected x type is Maybe<int32_t>, 
                                                    // actual type is int32_t
}

[mhdawson]

We discussed in the team meeting today and originally these methods were intentionally left out of those that returns Maybes. @legendecas will take another look and comment.

[legendecas]

When a method returns Maybe<> type, it indicates that it may call into JavaScript. However, those number value getters don't call into JavaScript at all. Non napi_ok status simply means that the type assumption of Napi::Value::As() is failed.

I would find this issue related to unchecked typecast with Napi::Value::As(). As documented at https://github.com/nodejs/node-addon-api/blob/main/doc/value.md#as, Napi::Value::As() should be used when the type of value is known and the value is the assumed type.

I'd propose a new type cast method Napi::Value::Cast() to enforce the type assumption, or add a new definition NAPI_ENABLE_TYPE_CHECK to allow Napi::Value::As() to enforce the type assumption.

[mhdawson]

@vmoroz suggested we make the macro NAPI_ENABLE_AS_TYPE_CHECK

[mhdawson]

From follow on discussion NAPI_ENABLE_TYPE_CHECK_ON_AS was suggested