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

napi_async_init and napi_async_destroy are not supported #18610

Closed
mysterycommand opened this issue Apr 6, 2023 · 5 comments
Closed

napi_async_init and napi_async_destroy are not supported #18610

mysterycommand opened this issue Apr 6, 2023 · 5 comments
Labels
node compat node native extension related to the node-api (.node)

Comments

@mysterycommand
Copy link

Hello, I was trying to see if I could get wrtc running in Deno. This code:

import { RTCPeerConnection } from "npm:wrtc";
new RTCPeerConnection({
  iceServers: [
    {
      urls: [
        "stun:stun.l.google.com:19302",
        "stun:global.stun.twilio.com:3478",
      ],
    },
  ],
});

Causes this error in the CLI:

$ RUST_BACKTRACE=full deno task start
Task start deno run -A --watch=static/,routes/ dev.ts
Watcher Process started.
The manifest has been generated for 2 routes and 1 islands.

============================================================
Deno has panicked. This is a bug in Deno. Please report this
at https://github.com/denoland/deno/issues/new.
If you can reliably reproduce this panic, include the
reproduction steps and re-run with the RUST_BACKTRACE=1 env
var set and include the backtrace in your report.

Platform: macos x86_64
Version: 1.32.3
Args: ["/Users/matt/.deno/bin/deno", "run", "-A", "--watch=static/,routes/", "dev.ts"]

thread 'main' panicked at 'not yet implemented', cli/napi/async.rs:72:3
stack backtrace:
   0:        0x103cc458a - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h47bed05ceb7970ee
   1:        0x1031d66da - core::fmt::write::hefd823d99333384a
   2:        0x103c938ac - std::io::Write::write_fmt::hb978956bb1b85e2b
   3:        0x103cc9afa - std::sys_common::backtrace::print::hc9e298c4664da113
   4:        0x103cc9723 - std::panicking::default_hook::{{closure}}::hb97411d6e916e1d2
   5:        0x103cc941d - std::panicking::default_hook::hc3ac81176817192f
   6:        0x1030cde27 - deno::setup_panic_hook::{{closure}}::hcd7f3bb617ae73eb
   7:        0x103cca4a2 - std::panicking::rust_panic_with_hook::hc11209fa2686218a
   8:        0x103cca228 - std::panicking::begin_panic_handler::{{closure}}::h5bad1bd665ab68a6
   9:        0x103cca1b8 - std::sys_common::backtrace::__rust_end_short_backtrace::h6f4efa73918edf57
  10:        0x103cca182 - _rust_begin_unwind
  11:        0x105152953 - core::panicking::panic_fmt::hb5474ac70e328787
  12:        0x105152a37 - core::panicking::panic::hff10b945c4ba062c
  13:        0x1031176f0 - _napi_async_init
  14:        0x11bbfda49 - __ZN4Napi12AsyncContextC2EP10napi_env__PKcRKNS_6ObjectE
  15:        0x11bc36075 - __ZN11node_webrtc15AsyncObjectWrapINS_17RTCPeerConnectionEEC2EPKcRKN4Napi12CallbackInfoE
  16:        0x11bc2452f - __ZN11node_webrtc23AsyncObjectWrapWithLoopINS_17RTCPeerConnectionEEC2EPKcRS1_RKN4Napi12CallbackInfoE
  17:        0x11bc23c3f - __ZN11node_webrtc17RTCPeerConnectionC2ERKN4Napi12CallbackInfoE
  18:        0x11bc48e45 - __ZZN4Napi10ObjectWrapIN11node_webrtc17RTCPeerConnectionEE26ConstructorCallbackWrapperEP10napi_env__P20napi_callback_info__ENKUlvE_clEv
  19:        0x11bc48b00 - __ZN4Napi10ObjectWrapIN11node_webrtc17RTCPeerConnectionEE26ConstructorCallbackWrapperEP10napi_env__P20napi_callback_info__
  20:        0x103654589 - deno_napi::function::call_fn::h4b808f1fd515756c
  21:        0x104095e40 - __ZN2v88internal12_GLOBAL__N_119HandleApiCallHelperILb1EEENS0_11MaybeHandleINS0_6ObjectEEEPNS0_7IsolateENS0_6HandleINS0_10HeapObjectEEENS8_INS0_20FunctionTemplateInfoEEENS8_IS4_EEPmi
  22:        0x1040958a8 - __ZN2v88internal21Builtin_HandleApiCallEiPmPNS0_7IsolateE

I thought maybe it was because the implementation is doing this slightly hinky old-style Node thing:

'use strict';

try {
  module.exports = require('../build/Debug/wrtc.node');
} catch (error) {
  module.exports = require('../build/Release/wrtc.node');
}

… but trying to access the published wrtc.node add-on directly causes the same error:

import { createRequire } from "node:module";

const require = createRequire(import.meta.url);
const {
  RTCPeerConnection,
} = require("../../node_modules/wrtc/build/Release/wrtc.node");
// etc ...

Are there plans to finish the implementation in cli/napi/async.rs?

@ry ry added node compat node native extension related to the node-api (.node) labels Apr 6, 2023
@mysterycommand
Copy link
Author

mysterycommand commented Apr 30, 2023

Related(?) I tried to get node-datachannel working which seems similar in spirit, but more recently updated and better maintained, and ran into napi_add_finalizer is not yet supported. which comes from cli/napi/js_native_api.rs … please lmk if you'd like me to file a separate issue, or if there's anything I can do to help!

@bartlomieju bartlomieju changed the title npm:wrtc causes panic napi_async_init and napi_async_destroy are not supported May 10, 2023
@bartlomieju
Copy link
Member

@mysterycommand the finalizer problem is tracked in #17325

bartlomieju added a commit that referenced this issue May 24, 2023
We don't have support for "AsyncContext" in "node:async_hooks" 
module, so these two APIs are just noops.

Towards #18610.
@bartlomieju
Copy link
Member

@mysterycommand FYI I changed these APIs to be no-ops in #19234 and this will be released in Deno v1.34 tomorrow. Let me know if that solves your issue.

@mysterycommand
Copy link
Author

@mysterycommand FYI I changed these APIs to be no-ops in #19234 and this will be released in Deno v1.34 tomorrow. Let me know if that solves your issue.

Thank you! I'll give it a look as soon as I'm able

@bartlomieju
Copy link
Member

I'm gonna close this issue for now, since this has been shipped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node compat node native extension related to the node-api (.node)
Projects
None yet
Development

No branches or pull requests

3 participants