From bf49519a4ce08ee5320327c9a0199cd89d5b87b3 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Fri, 14 Jun 2024 16:17:52 +0100 Subject: [PATCH] src: improve messages on CheckCast (#1507) * src: improve messages on CheckCast Print actual value type if the check failed. * fixup! --- napi-inl.h | 55 ++++++++++++++++++++++++++++++------------------------ napi.h | 20 ++++++++++++++++++++ 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index 9211ef926..eb520a022 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -9,8 +9,11 @@ //////////////////////////////////////////////////////////////////////////////// // Note: Do not include this file directly! Include "napi.h" instead. +// This should be a no-op and is intended for better IDE integration. +#include "napi.h" #include +#include #include #if NAPI_HAS_THREADS #include @@ -358,6 +361,18 @@ struct AccessorCallbackData { void* data; }; +// Debugging-purpose C++-style variant of sprintf(). +inline std::string StringFormat(const char* format, ...) { + std::string result; + va_list args; + va_start(args, format); + int len = vsnprintf(nullptr, 0, format, args); + result.resize(len); + vsnprintf(&result[0], len + 1, format, args); + va_end(args); + return result; +} + } // namespace details #ifndef NODE_ADDON_API_DISABLE_DEPRECATED @@ -826,8 +841,7 @@ inline void Boolean::CheckCast(napi_env env, napi_value value) { napi_valuetype type; napi_status status = napi_typeof(env, value, &type); NAPI_CHECK(status == napi_ok, "Boolean::CheckCast", "napi_typeof failed"); - NAPI_CHECK( - type == napi_boolean, "Boolean::CheckCast", "value is not napi_boolean"); + NAPI_INTERNAL_CHECK_EQ(type, napi_boolean, "%d", "Boolean::CheckCast"); } inline Boolean::Boolean() : Napi::Value() {} @@ -863,8 +877,7 @@ inline void Number::CheckCast(napi_env env, napi_value value) { napi_valuetype type; napi_status status = napi_typeof(env, value, &type); NAPI_CHECK(status == napi_ok, "Number::CheckCast", "napi_typeof failed"); - NAPI_CHECK( - type == napi_number, "Number::CheckCast", "value is not napi_number"); + NAPI_INTERNAL_CHECK_EQ(type, napi_number, "%d", "Number::CheckCast"); } inline Number::Number() : Value() {} @@ -959,8 +972,7 @@ inline void BigInt::CheckCast(napi_env env, napi_value value) { napi_valuetype type; napi_status status = napi_typeof(env, value, &type); NAPI_CHECK(status == napi_ok, "BigInt::CheckCast", "napi_typeof failed"); - NAPI_CHECK( - type == napi_bigint, "BigInt::CheckCast", "value is not napi_bigint"); + NAPI_INTERNAL_CHECK_EQ(type, napi_bigint, "%d", "BigInt::CheckCast"); } inline BigInt::BigInt() : Value() {} @@ -1046,9 +1058,10 @@ inline void Name::CheckCast(napi_env env, napi_value value) { napi_valuetype type; napi_status status = napi_typeof(env, value, &type); NAPI_CHECK(status == napi_ok, "Name::CheckCast", "napi_typeof failed"); - NAPI_CHECK(type == napi_string || type == napi_symbol, - "Name::CheckCast", - "value is not napi_string or napi_symbol"); + NAPI_INTERNAL_CHECK(type == napi_string || type == napi_symbol, + "Name::CheckCast", + "value is not napi_string or napi_symbol, got %d.", + type); } inline Name::Name() : Value() {} @@ -1115,8 +1128,7 @@ inline void String::CheckCast(napi_env env, napi_value value) { napi_valuetype type; napi_status status = napi_typeof(env, value, &type); NAPI_CHECK(status == napi_ok, "String::CheckCast", "napi_typeof failed"); - NAPI_CHECK( - type == napi_string, "String::CheckCast", "value is not napi_string"); + NAPI_INTERNAL_CHECK_EQ(type, napi_string, "%d", "String::CheckCast"); } inline String::String() : Name() {} @@ -1252,8 +1264,7 @@ inline void Symbol::CheckCast(napi_env env, napi_value value) { napi_valuetype type; napi_status status = napi_typeof(env, value, &type); NAPI_CHECK(status == napi_ok, "Symbol::CheckCast", "napi_typeof failed"); - NAPI_CHECK( - type == napi_symbol, "Symbol::CheckCast", "value is not napi_symbol"); + NAPI_INTERNAL_CHECK_EQ(type, napi_symbol, "%d", "Symbol::CheckCast"); } inline Symbol::Symbol() : Name() {} @@ -1424,8 +1435,7 @@ inline void Object::CheckCast(napi_env env, napi_value value) { napi_valuetype type; napi_status status = napi_typeof(env, value, &type); NAPI_CHECK(status == napi_ok, "Object::CheckCast", "napi_typeof failed"); - NAPI_CHECK( - type == napi_object, "Object::CheckCast", "value is not napi_object"); + NAPI_INTERNAL_CHECK_EQ(type, napi_object, "%d", "Object::CheckCast"); } inline Object::Object() : TypeTaggable() {} @@ -1837,9 +1847,7 @@ inline void External::CheckCast(napi_env env, napi_value value) { napi_valuetype type; napi_status status = napi_typeof(env, value, &type); NAPI_CHECK(status == napi_ok, "External::CheckCast", "napi_typeof failed"); - NAPI_CHECK(type == napi_external, - "External::CheckCast", - "value is not napi_external"); + NAPI_INTERNAL_CHECK_EQ(type, napi_external, "%d", "External::CheckCast"); } template @@ -2295,12 +2303,13 @@ inline void TypedArrayOf::CheckCast(napi_env env, napi_value value) { "TypedArrayOf::CheckCast", "napi_is_typedarray failed"); - NAPI_CHECK( + NAPI_INTERNAL_CHECK( (type == TypedArrayTypeForPrimitiveType() || (type == napi_uint8_clamped_array && std::is_same::value)), "TypedArrayOf::CheckCast", - "Array type must match the template parameter. (Uint8 arrays may " - "optionally have the \"clamped\" array type.)"); + "Array type must match the template parameter, (Uint8 arrays may " + "optionally have the \"clamped\" array type.), got %d.", + type); } template @@ -2481,9 +2490,7 @@ inline void Function::CheckCast(napi_env env, napi_value value) { napi_valuetype type; napi_status status = napi_typeof(env, value, &type); NAPI_CHECK(status == napi_ok, "Function::CheckCast", "napi_typeof failed"); - NAPI_CHECK(type == napi_function, - "Function::CheckCast", - "value is not napi_function"); + NAPI_INTERNAL_CHECK_EQ(type, napi_function, "%d", "Function::CheckCast"); } inline Function::Function() : Object() {} diff --git a/napi.h b/napi.h index 0b1fe170a..24298ad9a 100644 --- a/napi.h +++ b/napi.h @@ -142,6 +142,26 @@ static_assert(sizeof(char16_t) == sizeof(wchar_t), } \ } while (0) +// Internal check helper. Be careful that the formatted message length should be +// max 255 size and null terminated. +#define NAPI_INTERNAL_CHECK(expr, location, ...) \ + do { \ + if (!(expr)) { \ + std::string msg = Napi::details::StringFormat(__VA_ARGS__); \ + Napi::Error::Fatal(location, msg.c_str()); \ + } \ + } while (0) + +#define NAPI_INTERNAL_CHECK_EQ(actual, expected, value_format, location) \ + do { \ + auto actual_value = (actual); \ + NAPI_INTERNAL_CHECK(actual_value == (expected), \ + location, \ + "Expected " #actual " to be equal to " #expected \ + ", but got " value_format ".", \ + actual_value); \ + } while (0) + #define NAPI_FATAL_IF_FAILED(status, location, message) \ NAPI_CHECK((status) == napi_ok, location, message)