From 7613d669fb805607ffac6a06aa13b65b1a981af5 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 6 Feb 2023 00:52:59 +0800 Subject: [PATCH] chore(napi): enhance the error messages while converting types failed (#1473) --- .../src/bindgen_runtime/js_values/string.rs | 4 +- crates/napi/src/error.rs | 46 ++++++++++++-- crates/napi/src/js_values/function.rs | 17 ++++- crates/napi/src/js_values/global.rs | 24 ++++++- crates/napi/src/js_values/mod.rs | 12 ++-- examples/napi/__test__/error-msg.spec.ts | 59 ++++++++++++++++++ examples/napi/__test__/typegen.spec.ts.md | 1 + examples/napi/__test__/typegen.spec.ts.snap | Bin 3764 -> 3771 bytes examples/napi/__test__/values.spec.ts | 5 +- examples/napi/index.d.ts | 1 + examples/napi/src/error.rs | 5 ++ examples/napi/src/lib.rs | 2 +- 12 files changed, 161 insertions(+), 15 deletions(-) create mode 100644 examples/napi/__test__/error-msg.spec.ts diff --git a/crates/napi/src/bindgen_runtime/js_values/string.rs b/crates/napi/src/bindgen_runtime/js_values/string.rs index 0bf6cd5b..75d931b9 100644 --- a/crates/napi/src/bindgen_runtime/js_values/string.rs +++ b/crates/napi/src/bindgen_runtime/js_values/string.rs @@ -39,7 +39,7 @@ impl FromNapiValue for String { unsafe { sys::napi_get_value_string_utf8(env, napi_val, ptr::null_mut(), 0, &mut len) }, env, napi_val, - "Failed to convert napi `{}` into rust type `String`" + "Failed to convert JavaScript value `{}` into rust type `String`" )?; // end char len in C @@ -106,7 +106,7 @@ impl FromNapiValue for &str { }, env, napi_val, - "Failed to convert napi `{}` into rust type `String`" + "Failed to convert JavaScript value `{}` into rust type `String`" )?; // The `&str` should only be accepted from function arguments. diff --git a/crates/napi/src/error.rs b/crates/napi/src/error.rs index 98a4cd4b..9fb12f0c 100644 --- a/crates/napi/src/error.rs +++ b/crates/napi/src/error.rs @@ -369,11 +369,49 @@ macro_rules! check_status_and_type { match c { $crate::sys::Status::napi_ok => Ok(()), _ => { + use $crate::js_values::NapiValue; let value_type = $crate::type_of!($env, $val)?; - Err($crate::Error::new( - $crate::Status::from(c), - format!($msg, value_type), - )) + let error_msg = match value_type { + ValueType::Function => { + let function_name = unsafe { JsFunction::from_raw_unchecked($env, $val).name()? }; + format!( + $msg, + format!( + "function {}(..) ", + if function_name.len() == 0 { + "anonymous".to_owned() + } else { + function_name + } + ) + ) + } + ValueType::Object => { + let env_ = $crate::Env::from($env); + let json: $crate::JSON = env_.get_global()?.get_named_property_unchecked("JSON")?; + let object = json.stringify($crate::JsObject($crate::Value { + value: $val, + env: $env, + value_type: ValueType::Object, + }))?; + format!($msg, format!("Object {}", object)) + } + ValueType::Boolean | ValueType::Number => { + let value = + unsafe { $crate::JsUnknown::from_raw_unchecked($env, $val).coerce_to_string()? } + .into_utf8()?; + format!($msg, format!("{} {} ", value_type, value.as_str()?)) + } + #[cfg(feature = "napi6")] + ValueType::BigInt => { + let value = + unsafe { $crate::JsUnknown::from_raw_unchecked($env, $val).coerce_to_string()? } + .into_utf8()?; + format!($msg, format!("{} {} ", value_type, value.as_str()?)) + } + _ => format!($msg, value_type), + }; + Err($crate::Error::new($crate::Status::from(c), error_msg)) } } }}; diff --git a/crates/napi/src/js_values/function.rs b/crates/napi/src/js_values/function.rs index 01c777b2..ce19fe6a 100644 --- a/crates/napi/src/js_values/function.rs +++ b/crates/napi/src/js_values/function.rs @@ -1,12 +1,12 @@ use std::ptr; use super::Value; -use crate::bindgen_runtime::TypeName; #[cfg(feature = "napi4")] use crate::{ bindgen_runtime::ToNapiValue, threadsafe_function::{ThreadSafeCallContext, ThreadsafeFunction}, }; +use crate::{bindgen_runtime::TypeName, JsString}; use crate::{check_pending_exception, ValueType}; use crate::{sys, Env, Error, JsObject, JsUnknown, NapiRaw, NapiValue, Result, Status}; @@ -122,6 +122,21 @@ impl JsFunction { Ok(unsafe { JsObject::from_raw_unchecked(self.0.env, js_instance) }) } + /// function name + pub fn name(&self) -> Result { + let mut name = ptr::null_mut(); + check_pending_exception!(self.0.env, unsafe { + sys::napi_get_named_property( + self.0.env, + self.0.value, + "name\0".as_ptr().cast(), + &mut name, + ) + })?; + let name_value = unsafe { JsString::from_raw_unchecked(self.0.env, name) }; + Ok(name_value.into_utf8()?.as_str()?.to_owned()) + } + #[cfg(feature = "napi4")] pub fn create_threadsafe_function( &self, diff --git a/crates/napi/src/js_values/global.rs b/crates/napi/src/js_values/global.rs index b6ce46de..12a58b94 100644 --- a/crates/napi/src/js_values/global.rs +++ b/crates/napi/src/js_values/global.rs @@ -1,12 +1,34 @@ use std::convert::TryInto; use super::*; -use crate::Env; +use crate::{bindgen_runtime::FromNapiValue, Env}; pub struct JsGlobal(pub(crate) Value); pub struct JsTimeout(pub(crate) Value); +pub struct JSON(pub(crate) Value); + +impl FromNapiValue for JSON { + unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> Result { + Ok(JSON(Value { + env, + value: napi_val, + value_type: ValueType::Object, + })) + } +} + +impl JSON { + pub fn stringify(&self, value: V) -> Result { + let func: JsFunction = self.get_named_property_unchecked("stringify")?; + let result = func + .call(None, &[value]) + .map(|ret| unsafe { ret.cast::() })?; + result.into_utf8()?.as_str().map(|s| s.to_owned()) + } +} + impl JsGlobal { pub fn set_interval(&self, handler: JsFunction, interval: f64) -> Result { let func: JsFunction = self.get_named_property_unchecked("setInterval")?; diff --git a/crates/napi/src/js_values/mod.rs b/crates/napi/src/js_values/mod.rs index 9c016972..ec958bd3 100644 --- a/crates/napi/src/js_values/mod.rs +++ b/crates/napi/src/js_values/mod.rs @@ -3,7 +3,7 @@ use std::ffi::CString; use std::ptr; use crate::{ - bindgen_runtime::{TypeName, ValidateNapiValue}, + bindgen_runtime::{FromNapiValue, TypeName, ValidateNapiValue}, check_status, sys, type_of, Callback, Error, Result, Status, ValueType, }; @@ -337,26 +337,26 @@ macro_rules! impl_object_methods { pub fn get_named_property(&self, name: &str) -> Result where - T: NapiValue, + T: FromNapiValue, { let key = CString::new(name)?; let mut raw_value = ptr::null_mut(); check_status!(unsafe { sys::napi_get_named_property(self.0.env, self.0.value, key.as_ptr(), &mut raw_value) })?; - unsafe { T::from_raw(self.0.env, raw_value) } + unsafe { ::from_napi_value(self.0.env, raw_value) } } pub fn get_named_property_unchecked(&self, name: &str) -> Result where - T: NapiValue, + T: FromNapiValue, { let key = CString::new(name)?; let mut raw_value = ptr::null_mut(); check_status!(unsafe { sys::napi_get_named_property(self.0.env, self.0.value, key.as_ptr(), &mut raw_value) })?; - Ok(unsafe { T::from_raw_unchecked(self.0.env, raw_value) }) + unsafe { ::from_napi_value(self.0.env, raw_value) } } pub fn has_named_property(&self, name: &str) -> Result { @@ -618,6 +618,7 @@ impl_js_value_methods!(JsFunction); impl_js_value_methods!(JsExternal); impl_js_value_methods!(JsSymbol); impl_js_value_methods!(JsTimeout); +impl_js_value_methods!(JSON); impl_object_methods!(JsObject); impl_object_methods!(JsBuffer); @@ -625,6 +626,7 @@ impl_object_methods!(JsArrayBuffer); impl_object_methods!(JsTypedArray); impl_object_methods!(JsDataView); impl_object_methods!(JsGlobal); +impl_object_methods!(JSON); use ValueType::*; diff --git a/examples/napi/__test__/error-msg.spec.ts b/examples/napi/__test__/error-msg.spec.ts new file mode 100644 index 00000000..ecaf21b0 --- /dev/null +++ b/examples/napi/__test__/error-msg.spec.ts @@ -0,0 +1,59 @@ +import test from 'ava' + +import { receiveString } from '../index' + +test('Function message', (t) => { + // @ts-expect-error + t.throws(() => receiveString(function a() {}), { + message: + 'Failed to convert JavaScript value `function a(..) ` into rust type `String`', + }) + // @ts-expect-error + t.throws(() => receiveString(() => {}), { + message: + 'Failed to convert JavaScript value `function anonymous(..) ` into rust type `String`', + }) + // @ts-expect-error + t.throws(() => receiveString(1), { + message: + 'Failed to convert JavaScript value `Number 1 ` into rust type `String`', + }) + t.throws( + () => + // @ts-expect-error + receiveString({ + a: 1, + b: { + foo: 'bar', + s: false, + }, + }), + { + message: + 'Failed to convert JavaScript value `Object {"a":1,"b":{"foo":"bar","s":false}}` into rust type `String`', + }, + ) + // @ts-expect-error + t.throws(() => receiveString(Symbol('1')), { + message: + 'Failed to convert JavaScript value `Symbol` into rust type `String`', + }) + + // @ts-expect-error + t.throws(() => receiveString(), { + message: + 'Failed to convert JavaScript value `Undefined` into rust type `String`', + }) + + // @ts-expect-error + t.throws(() => receiveString(null), { + message: + 'Failed to convert JavaScript value `Null` into rust type `String`', + }) + + // @ts-expect-error + t.throws(() => receiveString(100n), { + message: + 'Failed to convert JavaScript value `BigInt 100 ` into rust type `String`', + }) +}) diff --git a/examples/napi/__test__/typegen.spec.ts.md b/examples/napi/__test__/typegen.spec.ts.md index 4177d83b..cf98e6a6 100644 --- a/examples/napi/__test__/typegen.spec.ts.md +++ b/examples/napi/__test__/typegen.spec.ts.md @@ -109,6 +109,7 @@ Generated by [AVA](https://avajs.dev). export function enumToI32(e: CustomNumEnum): number␊ export function throwError(): void␊ export function panic(): void␊ + export function receiveString(s: string): string␊ export function createExternal(size: number): ExternalObject␊ export function createExternalString(content: string): ExternalObject␊ export function getExternal(external: ExternalObject): number␊ diff --git a/examples/napi/__test__/typegen.spec.ts.snap b/examples/napi/__test__/typegen.spec.ts.snap index 84057f2f5a5c7bd33816270cd1a4ede464bdd357..269788d655202f4d9e62ed36d4abc40729503fc2 100644 GIT binary patch delta 2611 zcmV-33e5Gi9lISeK~_N^Q*L2!b7*gLAa*kf0{|&s3KEua(1hjbEy=ROD~y{ROr!%F zXjH;w#+yYMPyvxLK7af{pZiiI-_bwl&-4>|&RodhWp-DIlh{iR&z#$wIk%boG#STf zcp-lLnMp2VcJ&uAON%%nQxs^WbB} zugZBLUBOl%+CbC#8-6c^?^G~Y9LMBB715BD}11K!@)9M+cMaCjc(D;Zq z;QSt|ku~V4(ysJZD~j22mUb}#?>hnAJb!s7VbT&rt1>1Rdc^0qm{G0f?d`$hC>?k> zwt4ode92Dzii+70&8#XkPfwC^Z$MsoqCp~-RNae--7%%jyKq~rD&n?uRi!wFA-9_#(@Td16n13`p_4s_ zJ5N9J>T0(0Xb^K+CWobm_V>rYA%A3O?>(PG=<3GWNxC6>TSqddnMu0AjGj9gV~16j z-|*dInaUyc`pZBnC*%u^RQF9aJ=Hf|<#i@}3|lx1d629IhI(9rtAEKB0y_wH4O`7~ zZ$o2wAN$KV9aE(QJjO^TWU2TjOB8Hd|d0Y$m|U5!Zwg&`~z%NCorUqRf8D8 zg4TX)_874djleH-Ckx8Aq%2|aeQDR!uxYlw?b~{ntVvP(f$KWaRh`-zaM?*!(X!jE z;epcYxDcBUAFh`d)lLW#M`KvRX22Ni>j&6UcgofZ=(PrTf!|x!Y<~)DsEcjO9y}G6 z$p)>iy3IFdR>U#}TR#ao6VF@m29*9x0e#$C4cE0gG;+8`n{i&d49xj_jJZozz1QaY z^9ad=Jj1hzQ&`RjV6p%YnN{bd*Y9rsLpe%$A_4&=e#okfLed;~a^Chk(14x^}%0J$$yj0b>MR=_nPbo@K-9gK1^_5>pa`Zw>^=Has zTFAi|Mu(v1JdI=UGoEx{RUv`c)UJdHe$ zi;yi`qpFRgs?1-D?*ME&K1w6@iP3#R7Kd>vikuDgp_V#RyMH}aF;VAWstmB_VZRuY zx<7zuR-&*5gQz;L6oetmWQV~RwCe^oZ-nvSSF2=++zKPOGt*@1*%GErHA7c#dr`&2 zsdYRvuq}rz+m)!nmq7@UZ??BuJu5S`^1@2Jq3HC#dNn+rreC{Xb`Yo3A>OIAh~;cj zHP+DORBy`dsDIaCOy?H8zg!tB)x4%cpf4z?uGoTr4(Gk*X@F^Gd6;r@J%cNhX^wD<0g zdm5nbgAEcsr*V0*p^iBqZ=!lwrA*~bunYLuS0H7wr87CqvK%^knllhoGdF5UTd#qL z63Fvl!h*~hM9oNJ5@5GHRG|-1IM{HnC{6(ib9mM5={d=ru!Bvi2#rMrLK}&M9dht% z{_w_|!+)fHo#RES`!Sw(gz0?F5~;_LP0VfCJpI7Q|Ni~2f2eU3OWlVHXEX-|;*~f&7b;&tw(kl^U766cKXk zb)*(PNYZ?c&aFyC)3RBGqt|5$f75GT*wB`ZCvzP|f@_v_qiD=WoVP=utZIlglv5k3 zLCoa!l|9F!*se@TS6vd_jp+s38e==1;c1(G-_`G`ema|{7ues!JYV?_HP2lCeU)WRW_)P<2G<8&#<2WFN3pyu(W;LkM(D8fwS9 zmR;#?#hfn2?3l5rYENU=M3Hx0ePdO6bqV)?X~TKxnhljYpzHr@aioIQZpqofH(l-& zO@B|HJUh1EWlY-DR4oCIZEe&5Tc&!RVVd(7FxP-bpRb+Hsw0^|S*% zRzT;vPxF9FjFM^o4!7o^d3j=0uP>F_RDU(&s}(MyfnwIK!2-z^YHBK zw20X&F3>*@g|Kf8JFh+{oZy@Bz2pBY(SAFlsH-!%h}qeUF_p&wscN9J&gc#wwRxIJ z2Tzkt2ehfidLob`{|)YK*vYMIu0jZu0($GXRFnof#v4LdkcKRsGds6yOENE?C1I3T zWZ3V&x|(T=ubsgSULAgUd}y?PJaR0)motN7XvX0ieyT$JHd2)pTIO{|#g* zqOXxpEw_t9k9=)~Xr~H#w$82f%ret%(oi3X*libDo~Y- zTQzMWAHlMEX@`+AK4(1lrO$aOk}vEh^qjen!^`Zh5+|{j9G*G1Idg6^`FS#q)9^z4 z_zRO<#_Z~EVwM(hM5ZiZIhBlrX`ZuC61m72iP(fpc+9v+XaI)2rW3HfTVo0*`waa5W zQ&*w^)QS3O^^DOXW059kw)`9LWslX!8uV19R=S@RwP`s^dyIhho%n2?ypu2o38GaA zkbet3;`3X~s8;j#_F!?8{x}@lw0TwfW2b&a#q5Y?R+XnLO!Mf8t<=`$;20_Z)T=6B zRx9<5jfgm>43BF}w8>s8TzhAja1ms!-p3l<5R1fYf8lI=PnaxcsGgacp`O-|P3I6` z32H5t)`%NK!C)0##oksS=_jd^pmG%sCx3^D;-V4Ie+34sZiA>9th?fO%m0g0mQdJ3 zv=4Q;lN*tnQturtvlJ@$oJWvKMEoO<3K|dc=}A)V4M-DDG)Tmfsu?k{JEpXG7jCOn z6x^1sD*VPUqzD_GdVPv(Q_vO?6B(c8@_uiQ#qtwe;G*SgnWUK>NcpRr`n*ayv~G= zVGD;L50cfuP+m)L^)J~%U_-#JVXGJJ?N2OkOMe-sW2%&ZrxNLeOw~Xa=xSA{Hg{X4 zP9+n^)`98prLxVMX~R36vgb<*6n|6vwQ!+&U|yR>r+v5rIJxlP)#2bwaW$~1{4VCL zUDcG@F)RYVUl>Wi4io|`vX)3NNlvv4kwJkWpoVROFoVk5y25Z1%7z+CEqc}FP78$d z(yw7$vcv#ZRfkPmsQ4QROy7nxnoOClme9hCTVkb6)`#uNJukm6>Aa!WgnvMhv0N^Y zk854!n4O_r*amWpe}Jv(K!voiY7iq>(Atm9mLc|)5%`5}OhGl4loctyFKvPvHqF+z zZCLM;H7RO8a9t<5s_|L_E*qmNT6ViNJRf=;7h?0_!}ao_+JRrbrb{Qx`U zP8C`Kz19FP@O#UeO}z|du7B;ogQvo(*Pzu!wE5=Dide>A>n9;+;(06HfYP5SppSd2 z;ks6bMh@3#GtO(5fjOU#F?Y$T_u5>49wC{KXL!_b3dVH^El>zp6>sMn^*8vdCN)*;$5LL$wfiPs5>@XOEcHO|{jW8bk zYLzUJTVVuuW|~YrTf(%dX6Wi|FRGY0wT@>7w&kz`yAn0{G6+HP&GuHSXJv*~-an}~ z6rKK8uZG9d^lSIa4&szL#5=VXv7Ak+#u~bu>P@*F^*W5{+<&6?mn&nXn%7ha^aaIq z*3j|FYbjOg)0m9P@Ak(WpbJOJE6t z^OW#;2B0VgaevSw++WY}NeK@ACaQ;3%2ZzLx`2;; z1yUwkI+MdJ%b}yEIRim8bEB5D^%{sMfjkc;EXbTe)QmJH0d~tn75Wf`gAMnJ;uN4T zhgaR6o|D`OJJ_U(&{$L;w2?^IAqT(a4==DeOzPJ;-hYd_ALH>wn9k=ck$N22#N3w6 z(+`~d@4r9&OO2x-m$Q`4d5u9=dMzh-;+ojN>0UZjg7;kQYieJ?##9GX>Uvu^qd6!L zZ@A$xPx%V6eOEy0%7mUJXH*h%^$@I!iIBp!wU-a$MPGw`d;aeF=8JFpckh-W=#Ixp zb?AvF@_*^HY!%mSc1KeKz!}4Ben2BlIC-CZdEFpmdz?0y)IMun_lz&g|=)w zXzM5vT(hhjMPokVyd45%RYRdxnGOfT5h7~AO#56|@b zu6|eb)7d<|!2TZQ`O1H&dFD1Jua!`Hel=5#S;$Bac)dm6hYioEOU8>`Z*OSlJ2 z8_rADY^c-$UH@N;BNeoEOU@3y>2jxN`g{5y*|GgDW74jsY6*C3Yoi9(GSy?Ae4wKu zmwz$qB4R}3<)xsLiOrczi0ZleMH?Hc9D^+I05?JGL@M-u0f+XVUuym-VV9>go1sn| zKD6B*QE@R$W4-T7e$0lbjcMhsF}n7ob{Th#gGlq2PWO}dku7qU4L>m zQTR1d-@=-K*5#)5PHK79j@wzQrycOI0y@`yng?8BluYw?xHS*W%M+`5eMi)$`m0fI z{RZ4dzq*Ma*7tf&O_Ygnet+dG$fz1mBGB9siq%_S+dn zU7g8A%+6+vsXPuyRRf)MMtAtA&C^Uec$#cFpiMQ_6M-E0Z*XtJPHts$6+)mC&|Al) zqBPJk-VnlqG-ThR0sqy6KNV`T9ir`6m4 z+pZy`vaGS8;|a`T-~_>ZQ+^CiUa9xoqoOY7QoG&{{tu5kz3_9006VrA}Rm? diff --git a/examples/napi/__test__/values.spec.ts b/examples/napi/__test__/values.spec.ts index d06a870f..698e62dd 100644 --- a/examples/napi/__test__/values.spec.ts +++ b/examples/napi/__test__/values.spec.ts @@ -427,7 +427,10 @@ test('option object', (t) => { test('should throw if object type is not matched', (t) => { // @ts-expect-error const err1 = t.throws(() => receiveStrictObject({ name: 1 })) - t.is(err1!.message, 'Failed to convert napi `Number` into rust type `String`') + t.is( + err1!.message, + 'Failed to convert JavaScript value `Number 1 ` into rust type `String`', + ) // @ts-expect-error const err2 = t.throws(() => receiveStrictObject({ bar: 1 })) t.is(err2!.message, 'Missing field `name`') diff --git a/examples/napi/index.d.ts b/examples/napi/index.d.ts index 29ec410c..cea2101e 100644 --- a/examples/napi/index.d.ts +++ b/examples/napi/index.d.ts @@ -99,6 +99,7 @@ export const enum CustomNumEnum { export function enumToI32(e: CustomNumEnum): number export function throwError(): void export function panic(): void +export function receiveString(s: string): string export function createExternal(size: number): ExternalObject export function createExternalString(content: string): ExternalObject export function getExternal(external: ExternalObject): number diff --git a/examples/napi/src/error.rs b/examples/napi/src/error.rs index 42aa4ef1..dbe41276 100644 --- a/examples/napi/src/error.rs +++ b/examples/napi/src/error.rs @@ -9,3 +9,8 @@ pub fn throw_error() -> Result<()> { pub fn panic() { panic!("Don't panic"); } + +#[napi] +pub fn receive_string(s: String) -> String { + s +} diff --git a/examples/napi/src/lib.rs b/examples/napi/src/lib.rs index 69d03eec..e03619e1 100644 --- a/examples/napi/src/lib.rs +++ b/examples/napi/src/lib.rs @@ -51,5 +51,5 @@ mod typed_array; #[napi] pub fn run_script(env: Env, script: String) -> napi::Result { - env.run_script(&script) + env.run_script(script) }