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

importing a native library (e.g. node-tree-sitter) multiple in multiple test files results in a "TypeError: illegal invocation" #9206

Closed
rien opened this issue Nov 19, 2019 · 11 comments

Comments

@rien
Copy link

rien commented Nov 19, 2019

🐛 Bug Report

I am using node-tree-sitter in one of my projects. This library is in essence a thin layer wrapped around the tree-sitter binary (tree sitter is an incremental parser for multiple programming languages).

This library works fine, until I write tests for our program using. If two tests (indirectly) import the tree-sitter library, the second import sometimes fails with TypeError: illegal invocation. Running each test separately doesn't exhibit this behaviour. Running jest --clearCache fixes this issue sometimes.

However, I am unable to reproduce this bug outside Jest. When using ava I am able to re-import this library over 100 times without issues.

I have already submitted a bug report with the folks of node-tree-sitter. But because I am only encountering this bug with Jest, I'm thinking there could be going something wrong at Jest's side.

To Reproduce

Steps to reproduce the behavior:

# Clone minimal reproducible example
git clone [email protected]:rien/node-tree-sitter-bug-minimal-reproducible-example.git
cd node-tree-sitter-bug-minimal-reproducible-example

# Install dependencies
yarn install

# Run jest with all tests
yarn run jest # will possibly succeed
yarn run jest # will fail with 'TypeError: invalid invocation'

# Run jest a single test at a time
yarn run jest src/lib/__test__/one.test.ts # succeeds
yarn run jest src/lib/__test__/two.test.ts # succeeds

# Run all 100+ ava tests
yarn run ava # succeeds

Expected behavior

Re-importing the library works just fine.

Link to repl or repo (highly encouraged)

https://github.com/rien/node-tree-sitter-bug-minimal-reproducible-example/

envinfo

npx: installed 1 in 0.872s

  System:
    OS: Linux 5.3 void undefined
    CPU: (4) x64 Intel(R) Core(TM) i7-7560U CPU @ 2.40GHz
  Binaries:
    Node: 10.17.0 - ~/.nvm/versions/node/v10.17.0/bin/node
    Yarn: 1.19.1 - /bin/yarn
    npm: 6.11.3 - ~/.nvm/versions/node/v10.17.0/bin/npm
  npmPackages:
    jest: ^24.9.0 => 24.9.0
@rien rien changed the title "TypeError: illegal invocation" when importing a native library (e.g. node-tree-sitter) in multiple tests. importing a native library (e.g. node-tree-sitter) multiple in multiple test files results in a "TypeError: illegal invocation" Dec 3, 2019
@rien
Copy link
Author

rien commented Dec 3, 2019

@SimenB as you seem to be the most active developer lately, do you know what could be causing this behaviour?

I would like to look for a fix and create a PR. Our only solution right now is to run each test file separately, which is slowing our tests down a lot.

@rien
Copy link
Author

rien commented Dec 3, 2019

I have started debugging this and have found where the error is thrown when executing this line for the second time for the same native library:
https://github.com/facebook/jest/blob/baafcd48387227d81b1f8b61c9c2881e80e4db67/packages/jest-runtime/src/index.ts#L767

The error is caught by the try-catch on line 507 and says this:

{ stack: 'TypeError: Illegal invocation\n    at Object.get [a…/node_modules/jest-runtime/build/index.js:433:10)',
  message: 'Illegal invocation' }

Where line 433 in the compiled Javascript is the following line:
https://github.com/facebook/jest/blob/baafcd48387227d81b1f8b61c9c2881e80e4db67/packages/jest-runtime/src/index.ts#L440

@rien
Copy link
Author

rien commented Dec 3, 2019

The error is actually thrown when the node-tree-sitter module requires a native binding:

let binding;
try {
  binding = require('./build/Release/tree_sitter_runtime_binding');
} catch (e) {
  try {
    binding = require('./build/Debug/tree_sitter_runtime_binding');
  } catch (_) {
    throw e;
  }
}

const vm = require('vm');
const util = require('util')
const {Parser, NodeMethods, Tree, TreeCursor} = binding;

const {rootNode, edit} = Tree.prototype;
// ...

It's the Tree.prototype line that is throwing the error.

@rien
Copy link
Author

rien commented Dec 3, 2019

Removing the tree_sitter_runtime from the node Module cache fixed this issue in one run. But now this results in a Module did not self-register. error. But that's kind of expected if you start poking in node's internals...

@maxbrunsfeld
Copy link

maxbrunsfeld commented Dec 3, 2019

Can you put a console.log statement in that file? It sounds like the file is being evaluated multiple times (such that Tree.prototype.rootNode is being "wrapped" more than one time!). I would consider that to be a bug in the Node/test-runner environment that you're using.

@rien
Copy link
Author

rien commented Dec 4, 2019

@maxbrunsfeld I have been debugging yesterday and can confirm the index.js is evaluated multiple times.

I'm now looking into solving this issue by binding tree-sitter to a global variable.

@rien
Copy link
Author

rien commented Dec 4, 2019

Binding the module which includes tree-sitter to a global variable works. So this bug is indeed caused by Jest overriding the require behaviour.

However, the actual project using this bug uses typescript. Fixing our code to conditionally import or use a global variable will make our code messier than we'd like. So we'll be switching testing framework.

I've updated the example with the 'fix'.

@Hocdoc
Copy link

Hocdoc commented May 2, 2020

I am struggling with exactly the same issue and hope this will be fixed.

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 17, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 19, 2023
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants