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

Generate node bindings using napi-rs - separate crate #665

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

mirgee
Copy link
Contributor

@mirgee mirgee commented Nov 25, 2022

  • Generate nodejs binding using napi-rs
  • Running this on Nodejs18 comes with 20x performance improvement compared to old FFI with Nodejs12, 400x performance improvement compared to old FFI with Nodejs18
  • Significantly reduced boilerplate in NodeJS classes wrapping the bindings
  • Improved error handling - if we throw error from Rust, JS exception is thrown which contains following attributes
    • napiCode - error code in NAPI terms, for errors throw from Rust code we always set GenericFailure
    • vcxErrKind - error kind as string, eg InvalidConfiguration
    • vcxErrCode - error kind in its numeric representation, eg 1016
    • vcxErrMessage- error message created within Rust code, eg "Can not updated state with message: Message deserialization failed: {:?}"

Signed-off-by: Miroslav Kovar [email protected]

@mirgee mirgee linked an issue Nov 25, 2022 that may be closed by this pull request
@mirgee mirgee force-pushed the feature/napi-rs-separate-crate branch 4 times, most recently from a900346 to 24d6a0c Compare November 25, 2022 15:20
@codecov-commenter
Copy link

codecov-commenter commented Nov 27, 2022

Codecov Report

Merging #665 (537ba3c) into revert/hide-mocking-behind-test-utils (8481495) will increase coverage by 7.01%.
The diff coverage is 0.00%.

❗ Current head 537ba3c differs from pull request most recent head fb6c20f. Consider uploading reports for the commit fb6c20f to get more accurate results

@@                            Coverage Diff                            @@
##           revert/hide-mocking-behind-test-utils     #665      +/-   ##
=========================================================================
+ Coverage                                  45.37%   52.39%   +7.01%     
=========================================================================
  Files                                        242      245       +3     
  Lines                                      17602    18391     +789     
  Branches                                    3422     3961     +539     
=========================================================================
+ Hits                                        7987     9636    +1649     
+ Misses                                      7265     6044    -1221     
- Partials                                    2350     2711     +361     
Flag Coverage Δ
unittests-aries-vcx 52.37% <0.00%> (+6.99%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
wrappers/vcx-napi-rs/src/error.rs 3.00% <0.00%> (ø)
aries_vcx/src/utils/constants.rs 0.00% <0.00%> (-100.00%) ⬇️
messages/src/protocols/proof_presentation/mod.rs 0.00% <0.00%> (-100.00%) ⬇️
...ies_vcx/src/utils/mockdata/profile/mock_profile.rs 0.00% <0.00%> (-90.00%) ⬇️
...rotocols/issuance/issuer/states/credential_sent.rs 0.00% <0.00%> (-88.89%) ⬇️
...evocation_notification/receiver/states/finished.rs 0.00% <0.00%> (-81.82%) ⬇️
...evocation_notification/receiver/states/received.rs 0.00% <0.00%> (-81.82%) ⬇️
aries_vcx/src/lib.rs 25.00% <0.00%> (-75.00%) ⬇️
.../revocation_notification/sender/states/finished.rs 0.00% <0.00%> (-75.00%) ⬇️
...s_vcx/src/utils/mockdata/profile/mock_anoncreds.rs 0.00% <0.00%> (-71.62%) ⬇️
... and 190 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Patrik-Stas Patrik-Stas mentioned this pull request Dec 17, 2022
@Patrik-Stas Patrik-Stas changed the base branch from main to crate/ddo December 22, 2022 08:18
@Patrik-Stas Patrik-Stas force-pushed the crate/ddo branch 4 times, most recently from 36509b3 to 89c218f Compare December 24, 2022 12:14
Base automatically changed from crate/ddo to main December 24, 2022 14:17
@Patrik-Stas Patrik-Stas changed the base branch from main to libvcx/refactor December 28, 2022 12:35
This was referenced Dec 28, 2022
Base automatically changed from libvcx/refactor to main December 28, 2022 15:53
@Patrik-Stas Patrik-Stas changed the base branch from main to libvcx/minimize-api-c December 29, 2022 12:28
@Patrik-Stas Patrik-Stas force-pushed the feature/napi-rs-separate-crate branch 2 times, most recently from 397f043 to 5b51a87 Compare December 29, 2022 15:27
@Patrik-Stas Patrik-Stas force-pushed the feature/napi-rs-separate-crate branch 2 times, most recently from fa9fa01 to b69ad8f Compare December 30, 2022 15:50
@mirgee mirgee force-pushed the feature/napi-rs-separate-crate branch 7 times, most recently from 5c8ed7b to 0247577 Compare January 9, 2023 15:37
@mirgee mirgee force-pushed the feature/napi-rs-separate-crate branch 11 times, most recently from b0f22cd to 559b6e2 Compare January 10, 2023 14:11
@mirgee mirgee force-pushed the feature/napi-rs-separate-crate branch 2 times, most recently from 1adb8a8 to a2b777a Compare January 11, 2023 10:51
@mirgee mirgee changed the base branch from main to revert/hide-mocking-behind-test-utils January 11, 2023 10:52
@mirgee mirgee force-pushed the feature/napi-rs-separate-crate branch from a2b777a to fb6c20f Compare January 11, 2023 12:09
Base automatically changed from revert/hide-mocking-behind-test-utils to main January 11, 2023 14:03
Signed-off-by: Miroslav Kovar <[email protected]>
@mirgee mirgee force-pushed the feature/napi-rs-separate-crate branch from fb6c20f to d68e684 Compare January 11, 2023 14:28
@Patrik-Stas Patrik-Stas merged commit f413d78 into main Jan 11, 2023
@Patrik-Stas Patrik-Stas deleted the feature/napi-rs-separate-crate branch January 11, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants