Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix napi_throw_*/napi_create_*_error #14446

Merged
merged 5 commits into from
Oct 10, 2024
+223 −142
Merged

Fix napi_throw_*/napi_create_*_error #14446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7daf62a
Test napi_throw_error thoroughly
190n Oct 9, 2024
afdccc2
Fix and test napi_throw_*error functions
190n Oct 9, 2024
e4edc50
Fix napi_create_*_error functions
190n Oct 9, 2024
81ec5b9
Merge branch 'main' into ben/napi-throw-empty-msg
190n Oct 10, 2024
6cc947d
Update comments in napi tests
190n Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 51 additions & 127 deletions src/bun.js/bindings/napi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1300,66 +1300,73 @@ napi_define_properties(napi_env env, napi_value object, size_t property_count,
return napi_ok;
}

static void throwErrorWithCode(JSC::JSGlobalObject* globalObject, const char* msg_utf8, const char* code_utf8, const WTF::Function<JSObject*(JSC::JSGlobalObject*, const WTF::String&)>& createError)
static JSC::ErrorInstance* createErrorWithCode(JSC::JSGlobalObject* globalObject, const WTF::String& code, const WTF::String& message, JSC::ErrorType type)
{
auto& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
// no napi functions permit a null message, they must check before calling this function and
// return the right error code
ASSERT(!message.isNull());

auto message = msg_utf8 ? WTF::String::fromUTF8(msg_utf8) : String();
auto code = msg_utf8 ? WTF::String::fromUTF8(code_utf8) : String();
auto& vm = globalObject->vm();

auto* error = createError(globalObject, message);
if (!code.isEmpty()) {
// we don't call JSC::createError() as it asserts the message is not an empty string ""
auto* error = JSC::ErrorInstance::create(globalObject->vm(), globalObject->errorStructure(type), message, JSValue(), nullptr, RuntimeType::TypeNothing, type);
if (!code.isNull()) {
error->putDirect(vm, WebCore::builtinNames(vm).codePublicName(), JSC::jsString(vm, code), 0);
}

scope.throwException(globalObject, Exception::create(vm, error));
return error;
}

static JSValue createErrorForNapi(napi_env env, napi_value code, napi_value msg, const WTF::Function<JSObject*(JSC::JSGlobalObject*, const WTF::String&)>& constructor)
// used to implement napi_throw_*_error
static napi_status throwErrorWithCStrings(napi_env env, const char* code_utf8, const char* msg_utf8, JSC::ErrorType type)
{
auto* globalObject = toJS(env);
JSC::VM& vm = globalObject->vm();
auto catchScope = DECLARE_CATCH_SCOPE(vm);

JSValue codeValue = toJS(code);
WTF::String message;
auto& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

if (msg) {
JSValue messageValue = toJS(msg);
message = messageValue.toWTFString(globalObject);
if (catchScope.exception()) {
catchScope.clearException();
return {};
}
if (!msg_utf8) {
return napi_invalid_arg;
}

auto* error = constructor(globalObject, message);
WTF::String code = code_utf8 ? WTF::String::fromUTF8(code_utf8) : WTF::String();
WTF::String message = WTF::String::fromUTF8(msg_utf8);

if (codeValue && error) {
error->putDirect(vm, WebCore::builtinNames(vm).codePublicName(), codeValue, 0);
}
auto* error = createErrorWithCode(globalObject, code, message, type);
scope.throwException(globalObject, error);
return napi_ok;
}

if (catchScope.exception()) {
catchScope.clearException();
return {};
// code must be a string or nullptr (no code)
// msg must be a string
// never calls toString, never throws
static napi_status createErrorWithNapiValues(napi_env env, napi_value code, napi_value message, JSC::ErrorType type, napi_value* result)
{
if (!result || !message) {
return napi_invalid_arg;
}
JSValue js_code = toJS(code);
JSValue js_message = toJS(message);
if (!js_message.isString() || !(js_code.isEmpty() || js_code.isString())) {
return napi_string_expected;
}

return error;
auto* globalObject = toJS(env);

auto wtf_code = js_code.isEmpty() ? WTF::String() : js_code.getString(globalObject);
auto wtf_message = js_message.getString(globalObject);

*result = toNapi(
createErrorWithCode(globalObject, wtf_code, wtf_message, type),
globalObject);
return napi_ok;
}

extern "C" napi_status napi_throw_error(napi_env env,
const char* code,
const char* msg)
{
NAPI_PREMABLE
Zig::GlobalObject* globalObject = toJS(env);

throwErrorWithCode(globalObject, msg, code, [](JSC::JSGlobalObject* globalObject, const WTF::String& message) {
return JSC::createError(globalObject, message);
});

return napi_ok;
return throwErrorWithCStrings(env, code, msg, JSC::ErrorType::Error);
}

extern "C" napi_status napi_create_reference(napi_env env, napi_value value,
Expand Down Expand Up @@ -1650,110 +1657,44 @@ extern "C" napi_status node_api_create_syntax_error(napi_env env,
napi_value* result)
{
NAPI_PREMABLE
if (UNLIKELY(!result)) {
return napi_invalid_arg;
}

auto err = createErrorForNapi(env, code, msg, [](JSC::JSGlobalObject* globalObject, const WTF::String& message) {
return JSC::createSyntaxError(globalObject, message);
});

if (UNLIKELY(!err)) {
return napi_generic_failure;
}

*result = toNapi(err, toJS(env));
return napi_ok;
return createErrorWithNapiValues(env, code, msg, JSC::ErrorType::SyntaxError, result);
}

extern "C" napi_status node_api_throw_syntax_error(napi_env env,
const char* code,
const char* msg)
{
NAPI_PREMABLE

auto globalObject = toJS(env);

throwErrorWithCode(globalObject, msg, code, [](JSC::JSGlobalObject* globalObject, const WTF::String& message) {
return JSC::createSyntaxError(globalObject, message);
});

return napi_ok;
return throwErrorWithCStrings(env, code, msg, JSC::ErrorType::SyntaxError);
}

extern "C" napi_status napi_throw_type_error(napi_env env, const char* code,
const char* msg)
{
NAPI_PREMABLE
Zig::GlobalObject* globalObject = toJS(env);

throwErrorWithCode(globalObject, msg, code, [](JSC::JSGlobalObject* globalObject, const WTF::String& message) {
return JSC::createTypeError(globalObject, message);
});

return napi_ok;
return throwErrorWithCStrings(env, code, msg, JSC::ErrorType::TypeError);
}

extern "C" napi_status napi_create_type_error(napi_env env, napi_value code,
napi_value msg,
napi_value* result)
{
if (UNLIKELY(!result || !env)) {
return napi_invalid_arg;
}

auto err = createErrorForNapi(env, code, msg, [](JSC::JSGlobalObject* globalObject, const WTF::String& message) {
if (message.isEmpty()) {
return JSC::createTypeError(globalObject);
}

return JSC::createTypeError(globalObject, message);
});

if (UNLIKELY(!err)) {
return napi_generic_failure;
}

*result = toNapi(err, toJS(env));
return napi_ok;
NAPI_PREMABLE
return createErrorWithNapiValues(env, code, msg, JSC::ErrorType::TypeError, result);
}

extern "C" napi_status napi_create_error(napi_env env, napi_value code,
napi_value msg,
napi_value* result)
{
NAPI_PREMABLE

if (UNLIKELY(!result)) {
return napi_invalid_arg;
}

auto err = createErrorForNapi(env, code, msg, [](JSC::JSGlobalObject* globalObject, const WTF::String& message) {
if (message.isEmpty()) {
return JSC::createError(globalObject, String("Error"_s));
}

return JSC::createError(globalObject, message);
});

if (UNLIKELY(!err)) {
return napi_generic_failure;
}

*result = toNapi(err, toJS(env));
return napi_ok;
return createErrorWithNapiValues(env, code, msg, JSC::ErrorType::Error, result);
}
extern "C" napi_status napi_throw_range_error(napi_env env, const char* code,
const char* msg)
{
NAPI_PREMABLE
Zig::GlobalObject* globalObject = toJS(env);

throwErrorWithCode(globalObject, msg, code, [](JSC::JSGlobalObject* globalObject, const WTF::String& message) {
return JSC::createRangeError(globalObject, message);
});

return napi_ok;
return throwErrorWithCStrings(env, code, msg, JSC::ErrorType::RangeError);
}

extern "C" napi_status napi_object_freeze(napi_env env, napi_value object_value)
Expand Down Expand Up @@ -1818,24 +1759,7 @@ extern "C" napi_status napi_create_range_error(napi_env env, napi_value code,
napi_value* result)
{
NAPI_PREMABLE

if (UNLIKELY(!result)) {
return napi_invalid_arg;
}

auto err = createErrorForNapi(env, code, msg, [](JSC::JSGlobalObject* globalObject, const WTF::String& message) {
if (message.isEmpty()) {
return JSC::createRangeError(globalObject, String("Range error"_s));
}

return JSC::createRangeError(globalObject, message);
});

if (UNLIKELY(!err)) {
return napi_generic_failure;
}
*result = toNapi(err, toJS(env));
return napi_ok;
return createErrorWithNapiValues(env, code, msg, JSC::ErrorType::RangeError, result);
}

extern "C" napi_status napi_get_new_target(napi_env env,
Expand Down
Loading