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.

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

not a constructor #368

Open
Stubbsware opened this issue Nov 20, 2024 · 6 comments
Open

not a constructor #368

Stubbsware opened this issue Nov 20, 2024 · 6 comments

Comments

@Stubbsware
Copy link

Speaks for itself, if calling new when its not a constructor, the error being reported is just
TypeError: not a constructor

For example
var Foo = () => {};
var foo = new Foo(); // TypeError: Foo is not a constructor

However, QuickJS just reports
TypeError: not a constructor

@Stubbsware
Copy link
Author

Stubbsware commented Nov 24, 2024

Needs verification, but I fixed it for my scenario. Changing line 18814 of quickjs.c from

return JS_ThrowTypeError(ctx, "not a constructor");

to

return JS_ThrowTypeError(ctx, "%s is not a constructor", JS_ToCString(ctx, JS_GetPropertyStr(ctx, func_obj, "name")));

Now running

const Foo = () => {};
const foo = new Foo();

Results in the correct error
TypeError: Foo is not a constructor

@saghul
Copy link
Contributor

saghul commented Nov 24, 2024

Calling JS code to build the error message is likely not a good idea.

In addition you are leaking the C string.

@Stubbsware
Copy link
Author

Excellent observation, but i did say I was looking for clarification on if I was on the correct path.

Change:

if (unlikely(!p->is_constructor))
    return JS_ThrowTypeError(ctx, "not a constructor");

To:

if (unlikely(!p->is_constructor)) {
    JSValue name;
    const char *func_name;
    JSValue error;
    name = JS_GetPropertyStr(ctx, func_obj, "name");
    func_name = JS_ToCString(ctx, name);
    error = JS_ThrowTypeError(ctx, "'%s' is not a constructor", func_name);
    JS_FreeCString(ctx, func_name);
    JS_FreeValue(ctx, name);
    return error;
}

@saghul
Copy link
Contributor

saghul commented Nov 29, 2024

The function name can be extracted without calling ToCString, IMHO that would be a better path.

@Stubbsware
Copy link
Author

JS_GetPropertyStr returns a JSValue which is not directly represent a string in C. So would need converted with JS_ToCString. If you know something I am missing?

@saghul
Copy link
Contributor

saghul commented Nov 29, 2024

I think using get_func_name, a helper in build_backtrace would be best. It looks for a name property without running JS code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants