src: improve messages on CheckCast (#1507)

* src: improve messages on CheckCast Print actual value type if the check failed. * fixup!
nodejs · Jun 14, 2024 · bf49519 · bf49519
1 parent 40bcb09
commit bf49519
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 24 deletions.
diff --git 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 <algorithm>
+#include <cstdarg>
 #include <cstring>
 #if NAPI_HAS_THREADS
 #include <mutex>
@@ -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<T>::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 <typename T>
@@ -2295,12 +2303,13 @@ inline void TypedArrayOf<T>::CheckCast(napi_env env, napi_value value) {
              "TypedArrayOf::CheckCast",
              "napi_is_typedarray failed");
 
-  NAPI_CHECK(
+  NAPI_INTERNAL_CHECK(
       (type == TypedArrayTypeForPrimitiveType<T>() ||
        (type == napi_uint8_clamped_array && std::is_same<T, uint8_t>::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 <typename T>
@@ -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
@@ -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)