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

node-addon-api cannot receive more than 6 args #7685

Closed
kkocdko opened this issue Dec 15, 2023 · 0 comments · Fixed by #7765
Closed

node-addon-api cannot receive more than 6 args #7685

kkocdko opened this issue Dec 15, 2023 · 0 comments · Fixed by #7765
Labels
bug Something isn't working napi Compatibility with the native layer of Node.js

Comments

@kkocdko
Copy link

kkocdko commented Dec 15, 2023

What version of Bun is running?

1.0.18+36c316a24

What platform is your computer?

Fedora 39 Linux 6.6.4-200.fc39.x86_64

What steps can reproduce the bug?

The minimal reproducable example here:

#define NODE_ADDON_API_DISABLE_DEPRECATED
#include <napi.h>

#include <cassert>

Napi::Value Foo(const Napi::CallbackInfo& info) {
  Napi::Env napiEnv(info.Env());
  Napi::HandleScope scope(napiEnv);
  #define napi_assert(expr) {if(!expr){Napi::Error::New(napiEnv, #expr).ThrowAsJavaScriptException();}}
  napi_assert(info[0].IsNumber());
  napi_assert(info[1].IsNumber());
  napi_assert(info[2].IsNumber());
  napi_assert(info[3].IsNumber());
  napi_assert(info[4].IsNumber());
  napi_assert(info[5].IsNumber());
  napi_assert(info[6].IsNumber());
  napi_assert(info[7].IsNumber());
  return Napi::String::New(napiEnv, "success");
}

Napi::Object Init(Napi::Env env, Napi::Object exports) {
  exports.Set(Napi::String::New(env, "foo"),
              Napi::Function::New(env, Foo));
  return exports;
}

NODE_API_MODULE(addon, Init)
const native = require("./dist/native.node");
const args = [...Array(20).keys()];
console.log(JSON.stringify(args));
console.log(native.foo(...args));

Test in GitHub Actions here.

What is the expected behavior?

Like Node.js.

What do you see instead?

The 7th argument info[6].IsNumber() assert failed.

> bun
[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19]
1 | const native = require("./dist/native.node");
2 | const args = [...Array(20).keys()];
3 | console.log(JSON.stringify(args));
4 | console.log(native.foo(...args));
                ^
error: info[7].IsNumber()
      at /home/runner/work/utils4linux/utils4linux/napibun/main.js:4:13

> node
[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19]
success

Additional information

napi_assert(info[5].IsNumber()); // fine
napi_assert(info[6].IsNumber()); // the info[6] is the 7th, failed
napi_assert(info[7].IsNumber()); // if more than 7 args, the last one will failed.
napi_assert(info[8].IsNumber()); // if more than 7 args, the last one will failed.
@kkocdko kkocdko added the bug Something isn't working label Dec 15, 2023
@Jarred-Sumner Jarred-Sumner changed the title NAPI in native function can not receive more than 6 args. node-addon-api cannot receive more than 6 args Dec 17, 2023
@Electroid Electroid added the napi Compatibility with the native layer of Node.js label Dec 19, 2023
Jarred-Sumner added a commit that referenced this issue Dec 21, 2023
ryoppippi pushed a commit to ryoppippi/bun that referenced this issue Feb 1, 2024
* napi fixes

* Make bcrypt work

* Always return this

* Fixes oven-sh#7685

* [autofix.ci] apply automated fixes

* Update napi.cpp

* Make it clearer what this is doing

---------

Co-authored-by: Jarred Sumner <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working napi Compatibility with the native layer of Node.js
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants