vsrs / rsciter

Unofficial Rust bindings for Sciter
6 stars 2 forks source link

functor_invoke_thunk should return errors using Value::error_string #4

Open 4silvertooth opened 2 months ago

4silvertooth commented 2 months ago

Although I know it's very much WIP, do notice that method_thunk shouldn't return 0 https://github.com/vsrs/rsciter/blob/b0fe1f4f67d7f19cdf48473029755ea77e21a8ca/crates/rsciter/examples/global_asset.rs#L101

The body should be similar to functor_invoke_thunk https://github.com/vsrs/rsciter/blob/b0fe1f4f67d7f19cdf48473029755ea77e21a8ca/crates/rsciter/src/value.rs#L567

*p_value must be either result or Value::error_string

member_function thunk here always returns TRUE https://gitlab.com/sciter-engine/sciter-js-sdk/-/blob/main/include/sciter-om-def.h?ref_type=heads#L227

vsrs commented 2 months ago

@4silvertooth,

*p_value must be either result or Value::error_string

Not necessary. You can place your own error in p_value (Value::error_string as you correctly pointed out), but if you just return 0, the engine will generate the TypeError error on its own.

Here is the relevant sciter code (if you have a license, you can check engine\xdomjs\xasset.cpp,25 asset_method_magic_bound function):

      if (!md.func(pass, nargs, args.cbegin(), &rv))
        //throw qjs::om::type_error(string::format("method %s::%s",symbol_name_a(psp->name).c_str(), symbol_name_a(md.name).c_str()));
        return JS_ThrowTypeError(ctx, string::format("method %s::%s", hruntime::current().symbol_name_a(psp->name).c_str(), hruntime::current().symbol_name_a(md.name).c_str()));
      return conv<tool::value>::wrap(ctx, rv);

So returning 0 is a convinient way to report a method call error without details.

Anyway, thank you for your attention :) In the final code, I'll definitely construct the Value::error string first to provide a better error message.

vsrs commented 2 months ago

btw, just realized that functor_invoke_thunk https://github.com/vsrs/rsciter/blob/b0fe1f4f67d7f19cdf48473029755ea77e21a8ca/crates/rsciter/src/value.rs#L567-L580

may report error via Value::error_string if state.functor.invoke(args) call fails. Thanks.