more meaningful exceptions for constructors to aid debugging

especially when the original constructors throw

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 -   Support returning references to nested objects
 -   Fix [#16](https://github.com/mmomtchev/nobind/issues/16), global and `static` members can be `noexcept`
 -   Fix a minor memory leak when calling a method with incorrect number of arguments
+-   More meaningful exceptions for constructors to aid debugging
 
 ### [1.1.1] 2023-12-01
 

include/noobject.h

@@ -2,6 +2,7 @@
 #include <assert.h>
 #include <functional>
 #include <napi.h>
+#include <numeric>
 #include <tuple>
 #include <type_traits>
 
@@ -358,7 +359,7 @@ template <typename CLASS> NoObjectWrap<CLASS>::~NoObjectWrap() {
 // * From C++ with a Napi::External<> pointer -> it must construct a proxy for this object
 template <typename CLASS>
 NoObjectWrap<CLASS>::NoObjectWrap(const Napi::CallbackInfo &info) : Napi::ObjectWrap<NoObjectWrap<CLASS>>(info) {
-  // Napi::Env env = info.Env();
+  Napi::Env env{info.Env()};
   if (info.Length() == 2 && info[0].IsExternal()) {
     // From C++
     owned = info[1].ToBoolean().Value();
@@ -368,16 +369,35 @@ NoObjectWrap<CLASS>::NoObjectWrap(const Napi::CallbackInfo &info) : Napi::Object
   // From JS
   owned = true;
   if (cons.size() > info.Length() && cons[info.Length()].size() > 0) {
+    std::vector<std::string> errors;
     for (auto ctor : cons[info.Length()]) {
       try {
         (this->*ctor)(info);
         return;
-      } catch (...) {
+      } catch (const Napi::Error &e) {
+        // If there is only one constructor for the given number of arguments,
+        // throw the original construction error, instead of a generic error
+        // saying that all constructors with X arguments have been tried and none work
+        if (cons[info.Length()].size() == 1) {
+          std::rethrow_exception(std::current_exception());
+        }
+        // If not concatenate all the errors
+        errors.push_back(e.what());
+      } catch (const std::exception &e) {
+        // Same as above
+        if (cons[info.Length()].size() == 1) {
+          throw Napi::TypeError::New(env, e.what());
+        }
+        errors.push_back(e.what());
       }
     }
+    throw Napi::TypeError::New(
+        env, "All constructors with "s + std::to_string(info.Length()) + " arguments tried: ["s +
+                 std::accumulate(errors.begin() + 1, errors.end(), *errors.begin(),
+                                 [](const std::string s1, const std::string s2) { return s1 + ", " + s2; }) +
+                 "]"s);
   }
-  throw Napi::TypeError::New(info.Env(),
-                             "No constructor with the given "s + std::to_string(info.Length()) + " arguments found"s);
+  throw Napi::TypeError::New(env, "No constructor with "s + std::to_string(info.Length()) + " arguments found");
 }
 
 template <typename CLASS>

test/fixtures/two_cons.cc

@@ -1,7 +1,10 @@
 #include "two_cons.h"
+#include <stdexcept>
 
 TwoCons::TwoCons(int a) { x = a; }
 
 TwoCons::TwoCons(const std::string &s) { x = std::stoi(s); }
 
 TwoCons::TwoCons() { x = -1; }
+
+TwoCons::TwoCons(bool) { throw std::logic_error{"wrong constructor"}; }

test/fixtures/two_cons.h

@@ -6,5 +6,6 @@ class TwoCons {
 
   TwoCons(int a);
   TwoCons(const std::string &s);
+  TwoCons(bool);
   TwoCons();
 };

test/tests/basic_class.js

@@ -9,11 +9,11 @@ describe('constructor', () => {
   it('exception', () => {
     assert.throws(() => {
       new dll.Hello;
-    }, /No constructor with the given 0 arguments found/);
+    }, /No constructor with 0 arguments found/);
 
     assert.throws(() => {
       new dll.Hello(2);
-    }, /No constructor with the given 1 arguments found/);
+    }, /Expected a string/);
   });
 });
 

test/tests/class_obj.js

@@ -31,7 +31,7 @@ describe('reference', () => {
     it('null argument when Expected allowed', () => {
       assert.throws(() => {
         new dll.Hello(null);
-      }, /No constructor with the given 1 arguments found/);
+      }, /Expected a string/);
       assert.throws(() => {
         assert.isNumber(dll.hello_ref(null));
       }, /Expected an object/);
@@ -56,7 +56,7 @@ describe('reference', () => {
   it('undefined', () => {
     assert.throws(() => {
       new dll.Hello(null);
-    }, /No constructor with the given 1 arguments found/);
+    }, /Expected a string/);
     assert.throws(() => {
       assert.isNumber(dll.hello_ref(undefined));
     }, /Expected an object/);

test/tests/cons_overload.cc

@@ -3,5 +3,5 @@
 #include <nobind.h>
 
 NOBIND_MODULE(cons_overload, m) {
-  m.def<TwoCons>("TwoCons").cons<>().cons<const std::string &>().cons<int>().def<&TwoCons::x>("x");
+  m.def<TwoCons>("TwoCons").cons<>().cons<const std::string &>().cons<int>().cons<bool>().def<&TwoCons::x>("x");
 }

test/tests/cons_overload.js

@@ -15,13 +15,19 @@ describe('overloaded constructor', () => {
     assert.strictEqual(o.x, 2);
   });
 
+  it('original C++ exception', () => {
+    assert.throws(() => {
+      new dll.TwoCons(false);
+    }, /wrong constructor/);
+  });
+
   it('exception', () => {
     assert.throws(() => {
       new dll.TwoCons(1, 2, 3);
-    }, /No constructor with the given 3 arguments found/);
+    }, /No constructor with 3 arguments found/);
 
     assert.throws(() => {
       new dll.TwoCons(null);
-    }, /No constructor with the given 1 arguments found/);
+    }, /All constructors with 1 arguments tried: \[Expected a string, Expected a number, Expected a boolean\]/);
   });
 });