From e3adf5dac4d3dc4bc5ba96d65dc9fce966fceb2e Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 24 Jan 2023 19:07:33 +0800 Subject: [PATCH] fix(napi): unhandled promise rejection while using EitherN> (#1452) --- .../src/bindgen_runtime/js_values/either.rs | 135 ++++++++---------- examples/napi/__test__/typegen.spec.ts.md | 1 + examples/napi/__test__/typegen.spec.ts.snap | Bin 3730 -> 3746 bytes examples/napi/__test__/values.spec.ts | 10 ++ examples/napi/index.d.ts | 1 + examples/napi/src/either.rs | 11 ++ 6 files changed, 81 insertions(+), 77 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/js_values/either.rs b/crates/napi/src/bindgen_runtime/js_values/either.rs index 840f1526..37f298f9 100644 --- a/crates/napi/src/bindgen_runtime/js_values/either.rs +++ b/crates/napi/src/bindgen_runtime/js_values/either.rs @@ -1,30 +1,9 @@ use super::{FromNapiValue, ToNapiValue, TypeName, ValidateNapiValue}; use crate::{ bindgen_runtime::{Null, Undefined, Unknown}, - sys, Env, Error, JsUndefined, NapiRaw, NapiValue, Status, ValueType, + check_status, sys, Env, Error, JsUndefined, NapiRaw, NapiValue, Status, ValueType, }; -#[derive(Debug, Clone, Copy)] -pub enum Either { - A(A), - B(B), -} - -unsafe impl Send for Either {} -unsafe impl Sync for Either {} - -impl, B: AsRef, T> AsRef for Either -where - T: ?Sized, -{ - fn as_ref(&self) -> &T { - match &self { - Self::A(a) => a.as_ref(), - Self::B(b) => b.as_ref(), - } - } -} - impl Either { /// # Safety /// Backward compatible with `Either` in **v1** @@ -36,25 +15,6 @@ impl Either { } } -impl ValidateNapiValue for Either { - unsafe fn validate( - env: sys::napi_env, - napi_val: sys::napi_value, - ) -> crate::Result { - unsafe { A::validate(env, napi_val).or_else(|_| B::validate(env, napi_val)) } - } -} - -impl TypeName for Either { - fn type_name() -> &'static str { - "Either" - } - - fn value_type() -> ValueType { - ValueType::Unknown - } -} - // Backwards compatibility with v1 impl From> for Option { fn from(value: Either) -> Option { @@ -83,41 +43,6 @@ impl From> for Option { } } -impl< - A: TypeName + FromNapiValue + ValidateNapiValue, - B: TypeName + FromNapiValue + ValidateNapiValue, - > FromNapiValue for Either -{ - unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> crate::Result { - if unsafe { A::validate(env, napi_val) }.is_ok() { - unsafe { A::from_napi_value(env, napi_val) }.map(Either::A) - } else if unsafe { B::validate(env, napi_val) }.is_ok() { - unsafe { B::from_napi_value(env, napi_val).map(Either::B) } - } else { - Err(Error::new( - Status::InvalidArg, - format!( - "Value is not either {} or {}", - A::type_name(), - B::type_name() - ), - )) - } - } -} - -impl ToNapiValue for Either { - unsafe fn to_napi_value( - env: sys::napi_env, - value: Self, - ) -> crate::Result { - match value { - Self::A(a) => unsafe { A::to_napi_value(env, a) }, - Self::B(b) => unsafe { B::to_napi_value(env, b) }, - } - } -} - macro_rules! either_n { ( $either_name:ident, $( $parameter:ident ),+ $( , )* ) => { #[derive(Debug, Clone, Copy)] @@ -143,7 +68,19 @@ macro_rules! either_n { unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> crate::Result { let mut ret = Err(Error::new(Status::InvalidArg, "Invalid value".to_owned())); $( - if unsafe { $parameter::validate(env, napi_val).is_ok() && { ret = $parameter ::from_napi_value(env, napi_val).map(Self:: $parameter ); ret.is_ok() } } { + if unsafe { + match $parameter::validate(env, napi_val) { + Ok(maybe_rejected_promise) => { + if maybe_rejected_promise.is_null() { + true + } else { + silence_rejected_promise(env, maybe_rejected_promise)?; + false + } + }, + Err(_) => false + } + } && unsafe { { ret = $parameter ::from_napi_value(env, napi_val).map(Self:: $parameter ); ret.is_ok() } } { ret } else )+ @@ -194,6 +131,16 @@ macro_rules! either_n { } } + impl ),+ > AsRef for $either_name < $( $parameter ),+ > + where Data: ?Sized, + { + fn as_ref(&self) -> &Data { + match &self { + $( Self:: $parameter (v) => v.as_ref() ),+ + } + } + } + impl< $( $parameter ),+ > $either_name < $( $parameter ),+ > where $( $parameter: NapiRaw ),+ { @@ -206,6 +153,7 @@ macro_rules! either_n { }; } +either_n!(Either, A, B); either_n!(Either3, A, B, C); either_n!(Either4, A, B, C, D); either_n!(Either5, A, B, C, D, E); @@ -230,3 +178,36 @@ either_n!(Either23, A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, either_n!(Either24, A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X); either_n!(Either25, A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y); either_n!(Either26, A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z); + +fn silence_rejected_promise(env: sys::napi_env, promise: sys::napi_value) -> crate::Result<()> { + let mut catch_method = std::ptr::null_mut(); + check_status!(unsafe { + sys::napi_get_named_property(env, promise, "catch\0".as_ptr().cast(), &mut catch_method) + })?; + let mut catch_noop_callback = std::ptr::null_mut(); + check_status!(unsafe { + sys::napi_create_function( + env, + "catch\0".as_ptr().cast(), + 5, + Some(noop), + std::ptr::null_mut(), + &mut catch_noop_callback, + ) + })?; + check_status!(unsafe { + sys::napi_call_function( + env, + promise, + catch_method, + 1, + vec![catch_noop_callback].as_ptr().cast(), + std::ptr::null_mut(), + ) + })?; + Ok(()) +} + +unsafe extern "C" fn noop(_env: sys::napi_env, _info: sys::napi_callback_info) -> sys::napi_value { + std::ptr::null_mut() +} diff --git a/examples/napi/__test__/typegen.spec.ts.md b/examples/napi/__test__/typegen.spec.ts.md index e16b013e..e94183a7 100644 --- a/examples/napi/__test__/typegen.spec.ts.md +++ b/examples/napi/__test__/typegen.spec.ts.md @@ -82,6 +82,7 @@ Generated by [AVA](https://avajs.dev). }␊ export function eitherFromObjects(input: A | B | C): string␊ export function eitherBoolOrFunction(input: boolean | ((...args: any[]) => any)): void␊ + export function promiseInEither(input: number | Promise): Promise␊ /** default enum values are continuos i32s start from 0 */␊ export const enum Kind {␊ /** Barks */␊ diff --git a/examples/napi/__test__/typegen.spec.ts.snap b/examples/napi/__test__/typegen.spec.ts.snap index 1cd4acce2df8225927a4ad2cf59eda1482f2b922..4689623009c11a76cf1646ee5f5a4b0a10122246 100644 GIT binary patch literal 3746 zcmV;T4qfpzV&$D?49;uS zpcv1ATOW%E00000000B6TkCEdw-ruX6b1aRZ@{`JGHb|=B^!df@CB=tl*qOuLt57k z6rtwMaCd3V4kts7rHvw>d4vA$pCWm}K0?nShclcDcUOsh(+ubEPGLA)f zA%FUXDXwC6^*1>ac^r``OISt~BcaGL7Am3^DI*b^kO_|&mkCXQQpulwjv}h)x7*V{ zeDS9*@YnDD_{E=p|9<!a$p(I@4I{ZgSW3o{~R8lj`}3Y z=VO*xibIuGJURx_?jt6q?og!yL1L|A&&C7gJ5W% zw+LlE*M#t0HfVcZ29(J zHG^f)qJf@oGBM|pJ>AJCldhRk>~xsNijOWuK(nbG%<1cs(wI*{@di;8(0=irsRInb|!S>@Nl%^R};O+L~fjrf7560Z##K^j~-=o1v_!*V^yv*+ZK;1K}ek5+e);*Y(=i?Bi9ZS#eFB|pM81QWs& z*y2CJ%Ej@TW1K3QDc>j^ux8*}PDBr~WHu|u(QjLyVY9}0$Q49Ov2mQUCFYD-Tx>>a`&9vblD)gTiY_(%QBIUz$-tkDy{LTwP4mJ`;`eVS&!tO$wzehOBL?U)|ze66!T;8@Hc#h3Op5%%( zdg71vB4Ml4DZY zu0A*9F2~r0B?hX+wv1?45dF|5>f%@1opSC2;Bp844*g-3>AM4ZcAS--r|4d3U*K^>J)bj6 zjc+*U+=-6ws3LXSQVeQR1rRU==L2>lpOa5C&cUkC4D+%?fi)74syy0~#>kKfAdrWx zj79f-#gj-&5>a<(cA=9FLUzRzf`Nw{#g|uL9HsUm#rGjq@O{&MA5p0mMbA9Ez^I~H z4`;Z&c?xtmUXfOPD{}abx#bwV&;uo5Em9Vl0gEmF7HruQ-Ll4bI#Vmn&x+i%>}4!Q!26DWE}p!dFa`;t zRRNGoJ>v6Q^r%+z_WEEkN_!lRZP>gj{IOlXp>lReQ!C077K$u-YBRO9IXHq00OhI^ znAJjkYds?NDc$259c{4Ja@XD-g}VsSR_$YrYKR3Xw!d^XzQ;|LJ#@`X_s~yk%+16Q zpb2^{mga~XMAKjvUHRTtBIzH}1VQC0983-p%|*kZ{|W>v?gFUkth@Yo%l^v~mQdJ3 zv=4c?6C076QtllkGXWWV#v=$NBL0a-IgJO|^f)Q^284;nh9qK1Rg9R}9aHMOOSjcZ z3T`{DPW;BugcS9?=Q9Xh z#Z}wM)K_n7Nai#xVrWpK7ft}!q1EL#eD`RkHb}kxGLp&&`5YrPWl%GoE`zSJIukyI zEgXhCNLCX=dac0JzhVoC1p&K;ty;LZKC!$k{dFwHR0{!5CB%eGbwd~FYEkGicUz=R zCKJcjfEo9tqs^LX!#ka_7fTY9BKd1!p?IKQdyG!~a3|oz!h_cbgHz4bz=rZW7`Jw1 zQ+mg+2<(1d%mnm617MQ1WCoMu#LOWoAkYQW&^8DwpuD9kbT^@Dh_RHSS8Q&#Ksqn| z8sd^A2C^zUZ0bVA-$-KkHk{F9%1pL|8djJkR_bKE*{v1s*IFX1xWhNurByPOpfi47Pp}awcE2{0#{GxhDFwyPB?R zb!g;pjXLAJb}5+i`3Pf|w0f`g^~Vv4Np*@x4JWXi5zwRoo*C;xO0VBt&4+Z9@p*P~$)`G{*VL!_ zWet2Z5Lm;Zk)#tRAFS+#r!MC)5vqkXs;V+N%lxf)`=HzLp@`ULhWm^x4r3wnj1A2h zl|CZ7J*u4OV=bKq*yF8Vjij#wAk0b#t1t-FaYG;sS*kh|rhr}7v3Wg=2Vbv}9pqMs zV9tsnQ_rR_?NL)S^|lt3Pn=xG(*w(LSb<#$4Ymwikb3)Qt5vfqnpWOFsTUNT`d6=p z<0<;J>t*|~pw8f(Qj1u|CRJe#O-}Wu&5nK@#&mAs{preBi{=fL5^X^~oi${<@>)t2 zy0t)R#Xk#FHG1y2oUkYlS?7XxL@;;AJ@P^40~m&j>_c&nKnF1&OzQ`5{om&tkN#no zK|06cND?K;7y^=f9P@AkVf2a9CE$anof1AzffRWn4m{HR^$hP91Ws%3{El0x*LSrB z37^xrJay1V5D)}W)2!p4@|M;GeC%tIGIBCu8HP;GIHsG^5L7)kY7tnk0YnJ|ZO~ys z<_uh5WRN5%ArG}5LkI^A6<>1-R2Y!!(oN5a)TC`}j*8TnAs}@zBWar)e8V5!Mst`n zuXDWQbU((kh)~SuEK#N%*~CDV&BaGf{`cSi{7bjvL@sB7&UuYNclKtUU{-6>22OXy zR8PF;O5cq34eT;~+NAHQg)^Fg0P(sSp4pVIFt_h>h(nn)z0`~vb8A-}IW7DU4?0saZ$F@2X{8DH{C|^L7AQtA>~mIfI*!%HVaNOVmaV#gqrU76CuoG!-f zh_R@uHDd`x5AWLT#;Ww{Gu#EH3(m{1z8IxW=o<5yQ>dtoS#oypO_w=E)!#Lz!jA5D zDU&fZRetZ$t&JLCOI43}@{x}6OvS8gkYb9wyy0^^u>nw?TqNSsEw&qzG}HkDS>S1G zg6KqP_`iTb`}Z$3`;@TD6PnIYCJrA)?+>ZG7>d~J`;wnxL)6-g^*~El+%Lj^d*(+& z#a_yRlz2V&88HuyB`_baJCp(j4tV6GP6d=L=XQ=`hv^u_H;BH0{f4TUV+R^bW-C(e zcC2n|7J)$ISih=*<*(JZ*WA6FX(5)L?KDSTj)}Zl*v(tvRx?_D9n&|wzJPf1uM$k(K1fn>}zOo-hv-Xk;NJj+QpUab~w~Y44)=~mj>dN!JwN*A_G90NV_?nb}RTj;!}wG1DAyxkL`GY?9rAWT8pU`vWLJlvpyGrbv$l;lDn=|J&LW2i{@_XQ(j zF(0R~`uy%?-@Kmp$q0sS?%V(?_sD3*1%W>lWKV=d4lD!eeQ*C@@aEO%pTm>0QJ*Bm ze9UqSF)5Nz@-zYc$+u}9iJ;dfgFL5;Cqm{tnLbk_zG6}kkD7 zT7aUMD?s=z8n8VtgED;~K!u1sRhsnB2XsM+<=uHF6cey$ZCL!>g2z=k023OBMG^)X zl`~idr5c#|I#1_ZuqV64WYQ%m#Ysm+Ecxg%4QM{K#GJW4EsgmU3~vxc0qvWY4KlWm zJ^Rp_7XrtU?eghiB7^n74tE|cfz-ob0&jL64aAv}Js5L`6Ri)=!~>9X9Rdn&bL;UE zKv=usWfX)oj>j~-=o3`w!*V^y(`V#k$|FF4KU&k%41eq=UJM%q-Zr1xSMnolLvSHH zfi37YVpw)zLDTpGnM4FX!)Z5l45cdSUR+Eccp+z499-5P<GDB#x3+b%pXX`r^7085J7>UK@4O(#=SAmqdwV%e<7T8J1IT?EtfMjT_HkM`|x(GQ^@$oZag}e_>uJtb4{FC zT!@o|1rnlfpSYS=RxGyJmN_q zSz{*tXfG0u3bi5v{fYr2geB2)^U_`)2D)Uq4hbiDUA}T#l@ZUwvff4e3YDE)&S_xr zaIozm3J>EnjTuc^e_17!n~ zH0`$*zXICTE{o>NfQAO>hc-|bzsm2FvJZgEUHCinr&W6HF4);gUPc}>y|TW*6Nq|l z%mr;wh!n8Ni#hp7;{sd&&9R6|B)EWb|xp*;eb0Jr2}tGrNiU-BeUVk6dFnqR02 zfsnm)3X%KahWYXmJeg1~-+UiZ3EwyE_YoCxVP+QL1*QO%@*Hye^9z3IucVXpw_{GIE7OKo2@lF68@4O zW*&U3_%$;xq$}7eL>p)+|G@91@SXYs1L7FiRRIebQNDI9YiH`RF@T~@KdqiIT4XHJ z1kH|r1CHmh8d-y$DyvFgv7(SHXK9}j@V*mF&69T$CJaHeDqC@(M|^&Z8P#gu-X1KD z(s_nso8zv^RqWKSshAzn%&PK~g=ro=v1QfT92`Tsj(Szq#%iU$vEd8nl(B4$i8eE9 z#a8bOQz-;lt5LB=>%t;2+g~^v-xJfy8LGjhW~iq%WYYm3EJ3Zs(t>V-C>Shxt3cVR z`urqy#!{}r;p8w;Tr{HZufSl{Z4g-n8#J!Y?NHnQi&K_R*h91rMYNO1kegEP9sR8o z%I%y->@bP=M;;Y49^})Lq}&^j)17FLh$U6$U}ASnY4a}JRx5+QEnQWmjbX^Gq3F?U#X`3>JamZ=<4ufGhWazehqNOjLr(^EamRbFSp$FPONkO#?XV5o&9xcZlD zA+UP4YuIXBdz%T%d&pnK>6j`d;1NPPAyYNb1-e=lswdr6sZ(~uv2|cNe5q`+X4>#h zr|kKXYQj`%EnKJ`nAfJ!X&U) zAZ@G~#0VC&_G7cZhfQDvexbWaP>m&JEsyU@JC=q`v-NGy)VpL&irNoc*NLv`lGcFB zE~bi>-EIvJb6&@V*nIeKy}YP)I+r*a!-_Nm#$aDRz;?D%g;qeXHNXq}-m+#>FGHDY z+vVV?umABb$`cU?sMteRWfYR;z>|}$`qDVS z5K`Rs>*Tf?_>G)w?yL{~ib|ert^=Q2xz}VzfWK0?^50un-*6p#1i8_N)Wq>`4 z`qh}!**&6JiNYETqUt!#4?~v84udgh*9~mm2;;%8R>=~%6-IDprpeT^B}|)YhOXZB zqKb)A>v(2hTMj$0D^Y_lgAgR&Y;U!CR%U4BWs!PA(dmEnYIrufv$mEqZ^sGFGa2O@%;TP)uhH9k0AYQl)M!B(>(BMN|!XE~K2Y zs0dl-f_G#vcgQ{RUeyDbhNt;^bLg*<=@G?N)jP}m&xSe`+v1*X;IgQIx2XzDiNf6b; zD*q|3R9(Qwz5*#TC!LmI%H+_|)0}~znz>QSz;+p^9UO z!oh}yuQ&xLOvqL1rsrg8!VWg6A~aSA2yG-1cF4i6`NOMY4wL$IjyIC-$9NVIrt>*V zq#j2$F;Qjn^aCgV`|p4LrN&W^%UMe2yvCp_y_OT)YE5k5bT6GM!F#UuHMOr{m#Nbx zb+Ig*(Hs^as|(L`Y!|+G~IDqOZZedv|_)^W}H_ zyLU?wbo=6@I&Q?X@O0YoMMWKT>N0dJ!QKQQzTVZ5TKFJI^ErB|Dz8k-&J&Jam-+Zj zuX*!9TQ;7Wb!7amS=NoBF&}Z>4uP_&Ar?eVp{E8hE!TJ79QR;bEG1oa+j2Li7i?>c zy>W&oVfuYnzpG;CY@S|VvksGXL4SDi^AScD0dWF1Q!f6Y6!!7uiv0b zNRdVAEJ76prK3}Rzmt8yQt=KioeUw+HEF0FlT>!4TMu)(7_(!>qN>-7Ef7WC_1leA z>D49N1EvkYW1`OK2|{QbkpSlml!3}{2aIDp?P^?Rj+Sn z+Pr-=>aE{^+vr!9W`SVLbxer6gWk3lxX5!>>kB6kojiun0N$v<_I@Szt<^kbS=w5R zi0AHx$sr?_OkXNHRnJ{`aJ)3fu0X4N^YHBKw20X&F3>*@g|Kf8JFh+{oZy@Bz2iS< zXuq9N)YX|>#O!Rwn9AdTR5j39XLN^;+C0sqgQv-+1KLz$JrT%}{|5Iq?BrH9S0MyS z0ljrxDoO(#;|(D!NJEy+nVs9UC7GAck}%3EGHl*oUCp$`*ACPMuMWRDKH5JXITqh> z^t}DgokzZNXPx8#(B{_TYLd!oIxyk?r&x;U8{|{V?c&fQ-&i5qse+!Zb89`b%(R>I wR9M095uZZdAGj>scyzbr>dqBt3lCscoGy3k^qZAO`Zss}56J*kTwgW-05v8z4FCWD diff --git a/examples/napi/__test__/values.spec.ts b/examples/napi/__test__/values.spec.ts index 818a06c2..b6b98029 100644 --- a/examples/napi/__test__/values.spec.ts +++ b/examples/napi/__test__/values.spec.ts @@ -119,6 +119,7 @@ import { bigintFromI64, acceptThreadsafeFunction, acceptThreadsafeFunctionFatal, + promiseInEither, } from '../' test('export const', (t) => { @@ -833,6 +834,15 @@ Napi4Test('object only from js', (t) => { }) }) +Napi4Test('promise in either', async (t) => { + t.is(await promiseInEither(1), false) + t.is(await promiseInEither(20), true) + t.is(await promiseInEither(Promise.resolve(1)), false) + t.is(await promiseInEither(Promise.resolve(20)), true) + // @ts-expect-error + t.throws(() => promiseInEither('1')) +}) + const Napi5Test = Number(process.versions.napi) >= 5 ? test : test.skip Napi5Test('Date test', (t) => { diff --git a/examples/napi/index.d.ts b/examples/napi/index.d.ts index 5b2f39c9..88078283 100644 --- a/examples/napi/index.d.ts +++ b/examples/napi/index.d.ts @@ -72,6 +72,7 @@ export interface C { } export function eitherFromObjects(input: A | B | C): string export function eitherBoolOrFunction(input: boolean | ((...args: any[]) => any)): void +export function promiseInEither(input: number | Promise): Promise /** default enum values are continuos i32s start from 0 */ export const enum Kind { /** Barks */ diff --git a/examples/napi/src/either.rs b/examples/napi/src/either.rs index cdebbed3..ac9b1fdd 100644 --- a/examples/napi/src/either.rs +++ b/examples/napi/src/either.rs @@ -130,3 +130,14 @@ pub fn either_from_objects(input: Either3) -> String { #[napi] pub fn either_bool_or_function(_input: Either) {} + +#[napi] +pub async fn promise_in_either(input: Either>) -> Result { + match input { + Either::A(a) => Ok(a > 10), + Either::B(b) => { + let r = b.await?; + Ok(r > 10) + } + } +}