Bug: binding resolves wrong path

[marvinhagemeister]

Looks like we're looking for the wrong paths or maybe need to look at an additional path. This works in Node.

Steps to reproduce

1. Clone https://github.com/TryGhost/node-sqlite3
2. Run npm i && npm run install
3. Run node test/support/createdb.js
4. Run DENO_FUTURE=1 deno run -A npm:mocha -R spec --timeout 480000

Error: Could not locate the bindings file. Tried:
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/build/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/build/Debug/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/build/Release/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/out/Debug/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/Debug/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/out/Release/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/Release/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/build/default/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/compiled/20.11.1/darwin/arm64/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/addon-build/release/install-root/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/addon-build/debug/install-root/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/addon-build/default/install-root/node_sqlite3.node
 → /Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/lib/binding/node-v108-darwin-arm64/node_sqlite3.node
    at bindings (file:///Users/marvinh/dev/oss/node-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/bindings.js:126:9)

The file it's looking for is located at: /Users/marvinh/dev/oss/node-sqlite3/build/Release/node_sqlite3.node

Version: Deno 1.44.4

[marvinhagemeister]

Simpler reproduction:

import * as sql3 from "sqlite3";
console.log(sql3)

EDIT: These are the paths that are looked up in Node vs Deno inside the bindings module

Node (npm):
  /Users/marvinh/dev/test/deno-sqlite3/node_modules/sqlite3
  /Users/marvinh/dev/test/deno-sqlite3/node_modules/sqlite3/lib/sqlite3-binding.js
  
Node (pnpm):
  /Users/marvinh/dev/test/deno-sqlite3/node_modules/.pnpm/sqlite3@5.1.7/node_modules/sqlite3
  /Users/marvinh/dev/test/deno-sqlite3/node_modules/.pnpm/sqlite3@5.1.7/node_modules/sqlite3/lib/sqlite3-binding.js

Deno:
  /Users/marvinh/dev/test/deno-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings
  /Users/marvinh/dev/test/deno-sqlite3/node_modules/.deno/bindings@1.5.0/node_modules/bindings/bindings.js

[marvinhagemeister]

Ah the bindings dependency parses the error stack trace to look up the file path. It cannot deal with file:// urls inside the stack trace. In this exact line the stack trace location contains file:// but the __filename variable does not. So the check bails out too early.

https://github.com/TooTallNate/node-bindings/blob/c8033dcfc04c34397384e23f7399a30e6c13830d/bindings.js#L158

  Error.prepareStackTrace = function(e, st) {
    for (var i = 0, l = st.length; i < l; i++) {
      fileName = st[i].getFileName();
      // Here we compare file://foo/bar.js with /foo/bar.js in Deno and bail out too early
      if (fileName !== __filename) {
        if (calling_file) {
          if (fileName !== calling_file) {
            return;
          }
        } else {
          return;
        }
      }
    }
  };

[nathanwhit]

We discussed this and came to an agreement that there are two potential approaches:

• Make the stack trace file names paths instead of file URLs
  • requires changes deeper in our stack (deno_core), but (if it works) less suprising
• Patch the bindings package at runtime
  • could result in suprising behavior (e.g. you bundle your code into one file and all of a sudden you're not getting the patched version)

I'll explore the first approach, and if that doesn't work out we can try the runtime patch.