From 0a5713d62ce87ba85d07476f77c8e2822b37c897 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Thu, 23 May 2024 11:23:54 +0100 Subject: [PATCH] src: improve messages on CheckCast Print actual value type if the check failed. --- napi-inl.h | 51 +++++++++++++++++++++++++++------------------------ napi.h | 17 +++++++++++++++++ 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index 40e7d4208..6ab664e7e 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -9,6 +9,8 @@ //////////////////////////////////////////////////////////////////////////////// // 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 @@ -337,6 +339,15 @@ struct AccessorCallbackData { void* data; }; +// Debugging-purpose C++-style variant of sprintf(). +template +inline std::string SPrintF(const char* format, Args&&... args) { + char buf[256]; + int ret = snprintf(buf, 256, format, args...); + NAPI_CHECK(ret >= 0, "SPrintF", "Malformed format"); + return buf; +} + } // namespace details #ifndef NODE_ADDON_API_DISABLE_DEPRECATED @@ -805,8 +816,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() {} @@ -842,8 +852,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() {} @@ -938,8 +947,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() {} @@ -1025,9 +1033,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() {} @@ -1094,8 +1103,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() {} @@ -1231,8 +1239,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() {} @@ -1403,8 +1410,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() {} @@ -1814,9 +1820,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 @@ -2270,12 +2274,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 @@ -2456,9 +2461,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..d312f630e 100644 --- a/napi.h +++ b/napi.h @@ -142,6 +142,23 @@ 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::SPrintF(__VA_ARGS__); \ + Napi::Error::Fatal(location " " #expr, msg.c_str()); \ + } \ + } while (0) + +#define NAPI_INTERNAL_CHECK_EQ(actual, expected, value_format, location) \ + NAPI_INTERNAL_CHECK((actual) == (expected), \ + location, \ + "Expected " #actual " to be equal to " #expected \ + ", but got " value_format ".", \ + actual) + #define NAPI_FATAL_IF_FAILED(status, location, message) \ NAPI_CHECK((status) == napi_ok, location, message)