feat: Improved type conversion error messages

Benjamin-Dobell · Benjamin-Dobell · commit 0b3544b447fa · 2025-03-28T05:03:06.000+11:00
diff --git a/bridge/jsb_object_bindings.cpp b/bridge/jsb_object_bindings.cpp
@@ -228,7 +228,9 @@ namespace jsb
                 : !TypeConvert::js_to_gd_var(isolate, context, info[index], args[index]))
             {
                 // revert all constructors
-                const String error_message = jsb_errorf("bad argument: %d", index);
+                const String error_message = index < known_argc
+                    ? jsb_errorf("Bad argument: %d. Unable to convert JS %s to Godot %s", index, TypeConvert::js_debug_typeof(isolate, info[index]), Variant::get_type_name(method_info.argument_types[index]))
+                    : jsb_errorf("Bad argument: %d. Unable to convert JS %s", index, TypeConvert::js_debug_typeof(isolate, info[index]));
                 while (index >= 0) { args[index--].~Variant(); }
                 impl::Helper::throw_error(isolate, error_message);
                 return;
@@ -251,7 +253,8 @@ namespace jsb
             info.GetReturnValue().Set(jrval);
             return;
         }
-        jsb_throw(isolate, "failed to translate godot variant to v8 value");
+        const String error_message = jsb_errorf("Failed to translate returned Godot %s to a JS value", Variant::get_type_name(crval.get_type()));
+        impl::Helper::throw_error(isolate, error_message);
     }
 
     void ObjectReflectBindingUtil::_godot_object_method(const v8::FunctionCallbackInfo<v8::Value>& info)
@@ -294,7 +297,7 @@ namespace jsb
             if (!TypeConvert::js_to_gd_var(isolate, context, info[index], type, args[index]))
             {
                 // revert all constructors
-                const String error_message = jsb_errorf("bad argument: %d", index);
+                const String error_message = jsb_errorf("Failed to call: %s. Bad argument: %d. Unable to convert JS %s to Godot %s", method_bind->get_name(), index, TypeConvert::js_debug_typeof(isolate, info[index]), Variant::get_type_name(type));
                 while (index >= 0) { args[index--].~Variant(); }
                 impl::Helper::throw_error(isolate, error_message);
                 return;
@@ -324,7 +327,8 @@ namespace jsb
             info.GetReturnValue().Set(jrval);
             return;
         }
-        jsb_throw(isolate, "failed to translate godot variant to v8 value");
+        const String error_message = jsb_errorf("Failed to return from call: %s. Failed to translate returned Godot %s to a JS value", method_bind->get_name(), index, Variant::get_type_name(crval.get_type()));
+        impl::Helper::throw_error(isolate, error_message);
     }
 
     void ObjectReflectBindingUtil::_godot_object_get2(const v8::FunctionCallbackInfo<v8::Value>& info)
@@ -369,7 +373,9 @@ namespace jsb
             info.GetReturnValue().Set(jrval);
             return;
         }
-        jsb_throw(isolate, "failed to translate godot variant to v8 value");
+        const String error_message = jsb_errorf("Failed to get property: %s. Failed to translate returned Godot %s to a JS value",
+            property_info.getter_func->get_name(), index, Variant::get_type_name(crval.get_type()));
+        impl::Helper::throw_error(isolate, error_message);
     }
 
     void ObjectReflectBindingUtil::_godot_object_set2(const v8::FunctionCallbackInfo<v8::Value>& info)
diff --git a/bridge/jsb_type_convert.cpp b/bridge/jsb_type_convert.cpp
@@ -81,6 +81,63 @@ namespace jsb
 #endif
     }
 
+    String TypeConvert::js_debug_typeof(v8::Isolate* isolate, const v8::Local<v8::Value>& p_jval)
+    {
+        if (p_jval.IsEmpty())
+        {
+            return "<empty>";
+        }
+        if (p_jval->IsUndefined())
+        {
+            return "undefined";
+        }
+        if (p_jval->IsNull())
+        {
+            return "null";
+        }
+        if (p_jval->IsBoolean())
+        {
+            return "boolean";
+        }
+        if (p_jval->IsNumber())
+        {
+            return "number";
+        }
+        if (p_jval->IsString())
+        {
+            return "string";
+        }
+        if (p_jval->IsObject())
+        {
+            v8::Local<v8::Object> object = p_jval.As<v8::Object>();
+
+            if (is_variant(object))
+            {
+                const Variant* variant = (Variant*) object->GetAlignedPointerFromInternalField(IF_Pointer);
+                return jsb_format("variant (%s)", Variant::get_type_name(variant->get_type()));
+            }
+
+            v8::Local<v8::String> constructor_name = object->GetConstructorName();
+
+            if (constructor_name.IsEmpty())
+            {
+                return "object";
+            }
+
+            return jsb_format("object (%s)", impl::Helper::to_string(isolate, constructor_name));
+        }
+        if (p_jval->IsFunction())
+        {
+            return "function";
+        }
+        if (p_jval->IsArray())
+        {
+            return "array";
+        }
+
+        return "<unknown>";
+    }
+
     // translate js val into gd variant with an expected type
     bool TypeConvert::js_to_gd_var(v8::Isolate* isolate, const v8::Local<v8::Context>& context, const v8::Local<v8::Value>& p_jval, Variant::Type p_type, Variant& r_cvar)
     {
@@ -92,15 +149,15 @@ namespace jsb
                 r_cvar = p_jval.As<v8::Number>()->Value();
                 return true;
             }
-            return false;
+            break;
         case Variant::INT:
             // strict?
             if (int64_t val; impl::Helper::to_int64(p_jval, val))
             {
                 r_cvar = val;
                 return true;
             }
-            return false;
+            break;
         case Variant::OBJECT:
             {
                 if (!p_jval->IsObject())
@@ -111,12 +168,13 @@ namespace jsb
                         r_cvar = (Object*) nullptr;
                         return true;
                     }
-                    return false;
+                    break;
                 }
                 const v8::Local<v8::Object> self = p_jval.As<v8::Object>();
+
                 if (!TypeConvert::is_object(self))
                 {
-                    return false;
+                    break;
                 }
 
                 void* pointer = self->GetAlignedPointerFromInternalField(IF_Pointer);
@@ -125,8 +183,12 @@ namespace jsb
             }
         case Variant::BOOL:
             // strict?
-            if (p_jval->IsBoolean()) { r_cvar = p_jval->BooleanValue(isolate); return true; }
-            return false;
+            if (p_jval->IsBoolean())
+            {
+                r_cvar = p_jval->BooleanValue(isolate);
+                return true;
+            }
+            break;
         case Variant::STRING:
             if (p_jval->IsString())
             {
@@ -139,7 +201,7 @@ namespace jsb
                 r_cvar = impl::Helper::to_string(isolate, p_jval);
                 return true;
             }
-            return false;
+            break;
         case Variant::STRING_NAME:
             // cache the JSValue and StringName pair because the expected type is StringName
             if (p_jval->IsString())
@@ -199,12 +261,12 @@ namespace jsb
                 if (!p_jval->IsObject())
                 {
                     //TODO should auto convert a null/undefined value to a default (variant) counterpart?
-                    return false;
+                    break;
                 }
                 const v8::Local<v8::Object> self = p_jval.As<v8::Object>();
                 if (!is_variant(self))
                 {
-                    return false;
+                    break;
                 }
 
                 void* pointer = self->GetAlignedPointerFromInternalField(IF_Pointer);
@@ -216,6 +278,9 @@ namespace jsb
             return js_to_gd_var(isolate, context, p_jval, r_cvar);
         default: return false;
         }
+
+        JSB_LOG(Warning, "Failed to convert JS variable to Variant. Expected: %s. Found: %s", Variant::get_type_name(p_type), js_debug_typeof(isolate, p_jval));
+        return false;
     }
 
     bool TypeConvert::gd_var_to_js(v8::Isolate* isolate, const v8::Local<v8::Context>& context, const Variant& p_cvar, Variant::Type p_type, v8::Local<v8::Value>& r_jval)
diff --git a/bridge/jsb_type_convert.h b/bridge/jsb_type_convert.h
@@ -9,6 +9,13 @@ namespace jsb
 {
     struct TypeConvert
     {
+        /**
+         * Returns a string representation of a JavaScript type. For primitives, equivalent to the typeof operator in
+         * JS. For JS objects, we perform additional inspections and display if the object is an array, the Godot
+         * variant type, or the constructor name, when available.
+         */
+        static String js_debug_typeof(v8::Isolate* isolate, const v8::Local<v8::Value>& p_jval);
+
         /**
          * Translate a Godot object into a javascript object. The type of `p_object_obj` will be automatically exposed to the context if not existed.
          * @param p_godot_obj non-null godot object pointer

Original file line number	Diff line number	Diff line change
`@@ -228,7 +228,9 @@ namespace jsb`
`228`	`228`	`: !TypeConvert::js_to_gd_var(isolate, context, info[index], args[index]))`
`229`	`229`	`{`
`230`	`230`	`// revert all constructors`
`231`		`- const String error_message = jsb_errorf("bad argument: %d", index);`
	`231`	`+ const String error_message = index < known_argc`
	`232`	`+ ? jsb_errorf("Bad argument: %d. Unable to convert JS %s to Godot %s", index, TypeConvert::js_debug_typeof(isolate, info[index]), Variant::get_type_name(method_info.argument_types[index]))`
	`233`	`+ : jsb_errorf("Bad argument: %d. Unable to convert JS %s", index, TypeConvert::js_debug_typeof(isolate, info[index]));`
`232`	`234`	`while (index >= 0) { args[index--].~Variant(); }`
`233`	`235`	`impl::Helper::throw_error(isolate, error_message);`
`234`	`236`	`return;`
`@@ -251,7 +253,8 @@ namespace jsb`
`251`	`253`	`info.GetReturnValue().Set(jrval);`
`252`	`254`	`return;`
`253`	`255`	`}`
`254`		`- jsb_throw(isolate, "failed to translate godot variant to v8 value");`
	`256`	`+ const String error_message = jsb_errorf("Failed to translate returned Godot %s to a JS value", Variant::get_type_name(crval.get_type()));`
	`257`	`+ impl::Helper::throw_error(isolate, error_message);`
`255`	`258`	`}`
`256`	`259`
`257`	`260`	`void ObjectReflectBindingUtil::_godot_object_method(const v8::FunctionCallbackInfo<v8::Value>& info)`
`@@ -294,7 +297,7 @@ namespace jsb`
`294`	`297`	`if (!TypeConvert::js_to_gd_var(isolate, context, info[index], type, args[index]))`
`295`	`298`	`{`
`296`	`299`	`// revert all constructors`
`297`		`- const String error_message = jsb_errorf("bad argument: %d", index);`
	`300`	`+ const String error_message = jsb_errorf("Failed to call: %s. Bad argument: %d. Unable to convert JS %s to Godot %s", method_bind->get_name(), index, TypeConvert::js_debug_typeof(isolate, info[index]), Variant::get_type_name(type));`
`298`	`301`	`while (index >= 0) { args[index--].~Variant(); }`
`299`	`302`	`impl::Helper::throw_error(isolate, error_message);`
`300`	`303`	`return;`
`@@ -324,7 +327,8 @@ namespace jsb`
`324`	`327`	`info.GetReturnValue().Set(jrval);`
`325`	`328`	`return;`
`326`	`329`	`}`
`327`		`- jsb_throw(isolate, "failed to translate godot variant to v8 value");`
	`330`	`+ const String error_message = jsb_errorf("Failed to return from call: %s. Failed to translate returned Godot %s to a JS value", method_bind->get_name(), index, Variant::get_type_name(crval.get_type()));`
	`331`	`+ impl::Helper::throw_error(isolate, error_message);`
`328`	`332`	`}`
`329`	`333`
`330`	`334`	`void ObjectReflectBindingUtil::_godot_object_get2(const v8::FunctionCallbackInfo<v8::Value>& info)`
`@@ -369,7 +373,9 @@ namespace jsb`
`369`	`373`	`info.GetReturnValue().Set(jrval);`
`370`	`374`	`return;`
`371`	`375`	`}`
`372`		`- jsb_throw(isolate, "failed to translate godot variant to v8 value");`
	`376`	`+ const String error_message = jsb_errorf("Failed to get property: %s. Failed to translate returned Godot %s to a JS value",`
	`377`	`+ property_info.getter_func->get_name(), index, Variant::get_type_name(crval.get_type()));`
	`378`	`+ impl::Helper::throw_error(isolate, error_message);`
`373`	`379`	`}`
`374`	`380`
`375`	`381`	`void ObjectReflectBindingUtil::_godot_object_set2(const v8::FunctionCallbackInfo<v8::Value>& info)`
Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,13 @@ namespace jsb`
`9`	`9`	`{`
`10`	`10`	`struct TypeConvert`
`11`	`11`	`{`
	`12`	`+ /**`
	`13`	`+ * Returns a string representation of a JavaScript type. For primitives, equivalent to the typeof operator in`
	`14`	`+ * JS. For JS objects, we perform additional inspections and display if the object is an array, the Godot`
	`15`	`+ * variant type, or the constructor name, when available.`
	`16`	`+ */`
	`17`	`+ static String js_debug_typeof(v8::Isolate* isolate, const v8::Local<v8::Value>& p_jval);`
	`18`	`+`
`12`	`19`	`/**`
`13`	`20`	* Translate a Godot object into a javascript object. The type of `p_object_obj` will be automatically exposed to the context if not existed.
`14`	`21`	`* @param p_godot_obj non-null godot object pointer`