From 101ccb760221fd9d6cbae4fdaa767d92e59f577b Mon Sep 17 00:00:00 2001
From: Gabriel Schulhof <gabrielschulhof@gmail.com>
Date: Fri, 9 Jul 2021 10:26:39 -0700
Subject: [PATCH] src,test: Fix up null char * exception thrown

Throw an exception when receiving a null pointer for the `char *` and
`char16_t *` overloads of `String::New` that looks identical to an
error that core would have thrown under the circumstances
(`napi_invalid_arg`).

Also, rename the test methods to conform with our naming convention.
---
 napi-inl.h   | 13 ++++++++++---
 test/name.cc | 12 +++++++++---
 test/name.js |  4 ++--
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/napi-inl.h b/napi-inl.h
index 93cef5958..cec9b9224 100644
--- a/napi-inl.h
+++ b/napi-inl.h
@@ -900,10 +900,11 @@ inline String String::New(napi_env env, const std::u16string& val) {
 }
 
 inline String String::New(napi_env env, const char* val) {
+  // TODO(@gabrielschulhof) Remove if-statement when core's error handling is
+  // available in all supported versions.
   if (val == nullptr) {
-    NAPI_THROW(
-        TypeError::New(env, "String::New received a nullpointer as a value"),
-        Napi::String());
+    // Throw an error that looks like it came from core.
+    NAPI_THROW_IF_FAILED(env, napi_invalid_arg, String());
   }
   napi_value value;
   napi_status status = napi_create_string_utf8(env, val, std::strlen(val), &value);
@@ -913,6 +914,12 @@ inline String String::New(napi_env env, const char* val) {
 
 inline String String::New(napi_env env, const char16_t* val) {
   napi_value value;
+  // TODO(@gabrielschulhof) Remove if-statement when core's error handling is
+  // available in all supported versions.
+  if (val == nullptr) {
+    // Throw an error that looks like it came from core.
+    NAPI_THROW_IF_FAILED(env, napi_invalid_arg, String());
+  }
   napi_status status = napi_create_string_utf16(env, val, std::u16string(val).size(), &value);
   NAPI_THROW_IF_FAILED(env, status, String());
   return String(env, value);
diff --git a/test/name.cc b/test/name.cc
index 4e56141bc..d8556e154 100644
--- a/test/name.cc
+++ b/test/name.cc
@@ -82,18 +82,24 @@ Value CheckSymbol(const CallbackInfo& info) {
   return Boolean::New(info.Env(), info[0].Type() == napi_symbol);
 }
 
-void AssertErrorThrownWhenPassedNullptr(const CallbackInfo& info) {
+void NullStringShouldThrow(const CallbackInfo& info) {
   const char* nullStr = nullptr;
   String::New(info.Env(), nullStr);
 }
 
+void NullString16ShouldThrow(const CallbackInfo& info) {
+  const char16_t* nullStr = nullptr;
+  String::New(info.Env(), nullStr);
+}
+
 Object InitName(Env env) {
   Object exports = Object::New(env);
 
   exports["echoString"] = Function::New(env, EchoString);
   exports["createString"] = Function::New(env, CreateString);
-  exports["nullStringShouldThrow"] =
-      Function::New(env, AssertErrorThrownWhenPassedNullptr);
+  exports["nullStringShouldThrow"] = Function::New(env, NullStringShouldThrow);
+  exports["nullString16ShouldThrow"] =
+      Function::New(env, NullString16ShouldThrow);
   exports["checkString"] = Function::New(env, CheckString);
   exports["createSymbol"] = Function::New(env, CreateSymbol);
   exports["checkSymbol"] = Function::New(env, CheckSymbol);
diff --git a/test/name.js b/test/name.js
index 4c74542f0..717f1862f 100644
--- a/test/name.js
+++ b/test/name.js
@@ -9,8 +9,8 @@ function test(binding) {
 
 
   assert.throws(binding.name.nullStringShouldThrow, {
-    name: 'TypeError',
-    message: 'String::New received a nullpointer as a value',
+    name: 'Error',
+    message: 'Error in native callback',
   });
   assert.ok(binding.name.checkString(expected, 'utf8'));
   assert.ok(binding.name.checkString(expected, 'utf16'));