don't throw strings as exception (#207)

Throwing strings can be considered bad practice: https://stackoverflow.com/questions/11502052/throwing-strings-instead-of-errors
pmed · Jan 5, 2024 · 07cab7c · 07cab7c
1 parent dcbbfad
commit 07cab7c
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 22 deletions.
diff --git a/test/test_class.cpp b/test/test_class.cpp
@@ -228,11 +228,11 @@ void test_class_()
 		;
 
 	check_eq("C++ exception from X ctor",
-		run_script<std::string>(context, "ret = ''; try { new X(1, 2); } catch(err) { ret = err; } ret"),
+		run_script<std::string>(context, "ret = ''; try { new X(1, 2); } catch(err) { ret = err.message; } ret"),
 		"C++ exception");
 	check("Unhandled C++ exception from X ctor", context.run_script("x = new X(1, 2); x").IsEmpty());
 	check_eq("V8 exception from X ctor",
-		run_script<std::string>(context, "ret = ''; try { new X(1, 2, 3); } catch(err) { ret = err; } ret"),
+		run_script<std::string>(context, "ret = ''; try { new X(1, 2, 3); } catch(err) { ret = err.message; } ret"),
 		"JS exception");
 	check("Unhandled V8 exception from X ctor", context.run_script("x = new X(1, 2, 3); x").IsEmpty());
 

diff --git a/test/test_throw_ex.cpp b/test/test_throw_ex.cpp
@@ -4,7 +4,7 @@
 
 namespace {
 
-void test(v8pp::context& context, std::string type, v8pp::exception_ctor ctor = {}, v8::Local<v8::Value> opts = {})
+void test(v8pp::context& context, std::string type, v8pp::exception_ctor ctor, v8::Local<v8::Value> opts = {})
 {
 	v8::Isolate* isolate = context.isolate();
 
@@ -15,12 +15,7 @@ void test(v8pp::context& context, std::string type, v8pp::exception_ctor ctor =
 	check(" has caught", try_catch.HasCaught());
 	check("the same stack trace", try_catch.Message()->GetStackTrace() == v8::Exception::GetStackTrace(ex));
 	v8::String::Utf8Value const err_msg(isolate, try_catch.Message()->Get());
-
-	if (!type.empty())
-	{
-		type += ": ";
-	}
-	check_eq("message", *err_msg, "Uncaught " + type + "exception message");
+	check_eq("message", *err_msg, "Uncaught " + type + ": exception message");
 }
 
 } // unnamed namespace
@@ -30,7 +25,6 @@ void test_throw_ex()
 	v8pp::context context;
 	v8::Isolate* isolate = context.isolate();
 
-	test(context, "");
 	test(context, "Error", v8::Exception::Error);
 	test(context, "RangeError", v8::Exception::RangeError);
 	test(context, "ReferenceError", v8::Exception::ReferenceError);

diff --git a/v8pp/throw_ex.hpp b/v8pp/throw_ex.hpp
@@ -13,7 +13,7 @@ using exception_ctor = decltype(v8::Exception::Error); // assuming all Exception
 constexpr bool exception_ctor_with_options = V8_MAJOR_VERSION > 11 || (V8_MAJOR_VERSION == 11 && V8_MINOR_VERSION >= 9);
 
 v8::Local<v8::Value> throw_ex(v8::Isolate* isolate, std::string_view message,
-	exception_ctor = {}, v8::Local<v8::Value> exception_options = {});
+	exception_ctor ctor = v8::Exception::Error, v8::Local<v8::Value> exception_options = {});
 
 } // namespace v8pp
 

diff --git a/v8pp/throw_ex.ipp b/v8pp/throw_ex.ipp
@@ -7,21 +7,14 @@ V8PP_IMPL v8::Local<v8::Value> throw_ex(v8::Isolate* isolate, std::string_view m
 	v8::Local<v8::String> msg = v8::String::NewFromUtf8(isolate, message.data(),
 		v8::NewStringType::kNormal, static_cast<int>(message.size())).ToLocalChecked();
 
-	v8::Local<v8::Value> ex;
-	if (ctor)
-	{
 // if constexpr (exception_ctor_with_options) doesn't work win VC++ 2022
 #if V8_MAJOR_VERSION > 11 || (V8_MAJOR_VERSION == 11 && V8_MINOR_VERSION >= 9)
-		ex = ctor(msg, exception_options);
+	v8::Local<v8::Value> ex = ctor(msg, exception_options);
 #else
-		(void)exception_options;
-		ex = ctor(msg);
+	(void)exception_options;
+	v8::Local<v8::Value> ex = ctor(msg);
 #endif
-	}
-	else
-	{
-		ex = msg;
-	}
+
 	return isolate->ThrowException(ex);
 }