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

Require wrapper, broken with node 8.x.x #273

Closed
5 tasks done
Awarua- opened this issue Sep 3, 2017 · 3 comments
Closed
5 tasks done

Require wrapper, broken with node 8.x.x #273

Awarua- opened this issue Sep 3, 2017 · 3 comments

Comments

@Awarua-
Copy link
Member

Awarua- commented Sep 3, 2017

In raising this issue, I confirm to the best of my ability the following (please check boxes, e.g. [X]):

  • There are no duplicate issues.
  • The problem does not lie with an upstream dependency.
  • An installed module/integration/service/other is not causing your problem.
  • This is reproducible on the latest codebase with up to date NPM dependencies.
  • I have added appropriate labels to this issue.

Expected Behaviour:
Modules unload without error.
Unloading the KPM module (for example) should proceed without error.

Actual Behaviour:
Unloading modules (KPM and others) causes a stack overflow with Node 8.x.x

Steps to Reproduce:
Ensure you are running that latest version of Node 8.x.x

Start Concierge.
Shutdown Concierge, errors are printed to the console

error: Unloading module "kpm" failed.
debug: RangeError: Maximum call stack size exceeded
at m.children.forEach.child (C:\Users\wooll\dev\Concierge\Concierge\core\unsafe\require.js:234:36)
at Array.forEach (:null:null)
at run (C:\Users\wooll\dev\Concierge\Concierge\core\unsafe\require.js:234:28)
at m.children.forEach.child (C:\Users\wooll\dev\Concierge\Concierge\core\unsafe\require.js:235:21)
at Array.forEach (:null:null)

@Awarua-
Copy link
Member Author

Awarua- commented Sep 3, 2017

Changes in the node core of interest in 8.0.0
lib/modules.js

@drkno
Copy link
Member

drkno commented Sep 3, 2017

Best place to fix this would probably be #263.

@Awarua-
Copy link
Member Author

Awarua- commented Sep 21, 2017

Patch pushed through in #274
However, still to migrate to #263

@Awarua- Awarua- closed this as completed Sep 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants