search

denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.64k stars 5.25k forks source link

[`napi`] current `require()` implement `stackTrace` may not correct #19130

Closed loynoir closed 2 months ago

loynoir commented 1 year ago

Given

module.exports.foobar = require('bindings')('foobar')

Actual

Looking for wrong bindings/1.5.0/build/foobar.node.

detail ```js > var mod = await import('npm:napi-example@1.0.0') Uncaught Error: Could not locate the bindings file. Tried: → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/build/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/build/Debug/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/build/Release/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/out/Debug/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/Debug/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/out/Release/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/Release/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/build/default/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/compiled/18.12.1/linux/x64/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/addon-build/release/install-root/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/addon-build/debug/install-root/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/addon-build/default/install-root/foobar.node → /home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/lib/binding/node-v108-linux-x64/foobar.node at bindings (file:///home/vscode/.cache/deno/npm/private-npm/bindings/1.5.0/bindings.js:126:9) at Object. (file:///home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/internal/extension/cjs-wrapper.cjs:1:286) at Object. (file:///home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/internal/extension/cjs-wrapper.cjs:17:4) at Module._compile (ext:deno_node/01_require.js:968:34) at Object.Module._extensions..js (ext:deno_node/01_require.js:1001:10) at Module.load (ext:deno_node/01_require.js:879:32) at Function.Module._load (ext:deno_node/01_require.js:713:12) at Module.require (ext:deno_node/01_require.js:901:19) at require (ext:deno_node/01_require.js:1041:16) at file:///home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/internal/extension/cjs-wrapper.cjs:3:13 ```

Expected

Looking for correct napi-example/1.0.0/build/foobar.node.

detail ```txt → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/build/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/build/Debug/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/build/Release/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/out/Debug/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/Debug/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/out/Release/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/Release/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/build/default/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/compiled/18.12.1/linux/x64/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/addon-build/release/install-root/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/addon-build/debug/install-root/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/addon-build/default/install-root/foobar.node → /home/vscode/.cache/deno/npm/private-npm/napi-example/1.0.0/lib/binding/node-v108-linux-x64/foobar.node ```

Related

https://github.com/TooTallNate/node-bindings/blob/1.5.0/bindings.js#L82

Workaround

module.exports.foobar = require('../hard/coded/path/to/foobar.node')
ealib commented 1 year ago

Perhaps it is related to the missing functionality of importing native Node.js modules. See issue #17246 .

loynoir commented 1 year ago

@ealib

No, I think core logic behind two issues are totally different.

First, I think you made a mistake that testing deno and node without controling variates.

Did you try Node ESM import('/path/to/reproduce.node')?

$ node 
Welcome to Node.js v18.15.0.
Type ".help" for more information.
> await import('./node_modules/foobar/build/Release/foobar.node')
Uncaught:
TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".node" for /path/to/node_modules/foobar/build/Release/foobar.node
    at __node_internal_captureLargerStackTrace (node:internal/errors:490:5)
    at new NodeError (node:internal/errors:399:5)
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:79:11)
    at defaultGetFormat (node:internal/modules/esm/get_format:121:38)
    at defaultLoad (node:internal/modules/esm/load:81:20)
    at nextLoad (node:internal/modules/esm/loader:163:28)
    at ESMLoader.load (node:internal/modules/esm/loader:605:26)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:457:22)
    at new ModuleJob (node:internal/modules/esm/module_job:64:26)
    at #createModuleJob (node:internal/modules/esm/loader:480:17)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:434:34)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'
}

That is not supported in Node, which makes it no sense to let deno support it.

Perhaps it is related to the missing functionality of importing native Node.js modules

No, it is not missing. It is supported, but not perfect.

$ deno repl -A 
Deno 1.33.3
exit using ctrl+d, ctrl+c, or close()
> var mod = await import('npm:private-napi-foobar@1.0.0/cjs-wrapper.cjs')
undefined
> mod.some_fn('...')
"...ok..."

The reason why I say not perfect, is the reason why I open this issue.

In node, require('bindings')('foobar')

In deno, should use workaround require('../relative/path/to/Release/foobar.node')

ealib commented 1 year ago

@loynoir excuse me, but I do not understand what you mean by "controling variates".

I understand, however, that the bug you report here has nothing to do with #17246 , but is related to the implementation of import in Deno.

I did not intentionally try Node ESM import, because I do not want to maintain double-sided code since Deno should make the use of Node.js modules prefixed with "npm:..." transparent.

I mean, if @bartlomieju says that import ... from "npm: ..." will never be able to distinguish between ES/TS source modules and native binary modules, then I'll be content and use import.

bartlomieju commented 1 year ago

@loynoir any chance you could provide a reproducible example (or a repo) that I could try for debugging this problem?

reggi commented 6 months ago

@bartlomieju This?

➜  ~ deno eval 'import "npm:sqlite3"'
error: Uncaught (in promise) Error: Could not locate the bindings file. Tried:
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/build/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/build/Debug/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/build/Release/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/out/Debug/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/Debug/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/out/Release/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/Release/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/build/default/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/compiled/20.11.1/darwin/arm64/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/addon-build/release/install-root/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/addon-build/debug/install-root/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/addon-build/default/install-root/node_sqlite3.node
 → /Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/lib/binding/node-v108-darwin-arm64/node_sqlite3.node
    at bindings (file:///Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/bindings/1.5.0/bindings.js:126:9)
    at Object.<anonymous> (file:///Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/sqlite3/5.1.7/lib/sqlite3-binding.js:1:291)
    at Object.<anonymous> (file:///Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/sqlite3/5.1.7/lib/sqlite3-binding.js:3:4)
    at Module._compile (node:module:718:34)
    at Object.Module._extensions..js (node:module:732:10)
    at Module.load (node:module:643:32)
    at Function.Module._load (node:module:524:12)
    at Module.require (node:module:662:19)
    at require (node:module:776:16)
    at Object.<anonymous> (file:///Users/thomasreggi/Library/Caches/deno/npm/registry.npmjs.org/sqlite3/5.1.7/lib/sqlite3.js:2:17)
➜  ~