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

stderr redirection breaks require #11257

Closed
michaelglass opened this issue Feb 9, 2017 · 16 comments
Closed

stderr redirection breaks require #11257

michaelglass opened this issue Feb 9, 2017 · 16 comments
Assignees
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system.

Comments

@michaelglass
Copy link

michaelglass commented Feb 9, 2017

  • Version: 7.5.0
  • Platform: Darwin Kernel Version 16.5.0 (mac os sierra)

related workaround: rtfeldman/node-test-runner#106, rtfeldman/node-test-runner@959fda0

We noticed that when redirecting stderr AND receiving input script via pipe, require doesn't work with packages in package.json.

Minimal test case: (given a package.json that includes ajv).

echo "require('ajv')" | node &> js.out || cat js.out

expected output: nothing, it's just a require

actual output:

fs.js:56
    assertEncoding(options.encoding);
    ^

TypeError: assertEncoding is not a function
    at getOptions (fs.js:56:5)
    at Object.realpathSync (fs.js:1468:13)
    at toRealPath (module.js:130:13)
    at tryFile (module.js:126:22)
    at tryPackage (module.js:107:10)
    at Function.Module._findPath (module.js:183:20)
    at Function.Module._resolveFilename (module.js:468:25)
    at Function.Module._load (module.js:418:25)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)

also tested on another Ubuntu 14.04 server. Was not a problem using node 6.9.5

@addaleax addaleax added confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. labels Feb 9, 2017
@addaleax
Copy link
Member

addaleax commented Feb 9, 2017

Thanks for reporting this! Seems like some kind of circular dependency in the core module to me…

@evanlucas evanlucas self-assigned this Feb 9, 2017
evanlucas added a commit to evanlucas/node that referenced this issue Feb 9, 2017
The fs module requires internal/fs and internal/fs requires fs which
causes a circular dependency. This change lazy loads fs inside of
internal/fs to prevent assertEncoding from breaking when requiring via
stdin.

Fixes: nodejs#11257
evanlucas added a commit to evanlucas/node that referenced this issue Mar 23, 2017
Previously, there was a circular dependency on the public fs module in
SyncWriteStream. Moving the implementations of fs.writeSync and
fs.closeSync to internal/fs allows us to avoid that circular dependency.

Fixes: nodejs#11257
antun added a commit to antun/ndebug that referenced this issue May 16, 2017
nodejs/node#11257

Instead of piping the output to node, create a temporary file.
@avatar-lavventura
Copy link

avatar-lavventura commented Jun 12, 2017

I have the same error. Running a python code on the background, which makes pipe to node.
nohup python hello.py & . Inside python code I just called a node script. Original code works without nohup.

Something like this:
os.popen( val + "| node & echo $! >" + constants.LOG_PATH + "/my-app.pid").read().

        assertEncoding(options.encoding);
        ^
    
    TypeError: assertEncoding is not a function
        at getOptions (fs.js:58:5)
        at Object.realpathSync (fs.js:1564:13)
        at toRealPath (module.js:130:13)
        at Function.Module._findPath (module.js:178:22)
        at Function.Module._resolveFilename (module.js:468:25)
        at Function.Module._load (module.js:418:25)
        at Module.require (module.js:498:17)
        at require (internal/module.js:20:19)
        at [stdin]:1:13
        at ContextifyScript.Script.runInThisContext (vm.js:23:33)
    fs.js:58

@p3x-robot
Copy link

i have this error as well, so it is because of 8.5?

@BridgeAR
Copy link
Member

I can not reproduce this from 8.4 on but I do not know what might have fixed it.

@p3x-robot this is still occuring for you on 8.5? Do you have a minimal test case?

@p3x-robot
Copy link

Yes it is still error:

Updating 4d3e58a..532812e
Fast-forward
 package.json | 2 +-
 yarn.lock    | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)


 9878 root      1192 S    grep fail2ban


WAN DNS: 192.168.78.20 74.82.42.42 8.8.8.8


/opt/router-scripts-lede/settings/common/override/
/opt/router-scripts-lede/settings/d-link.router/override/









module.js:473
      throw err;
      ^

TypeError: assertEncoding is not a function
    at getOptions (fs.js:81:5)
    at Object.realpathSync (fs.js:1584:15)
    at toRealPath (module.js:154:13)
    at Function.Module._findPath (module.js:203:22)
    at Function.Module._resolveFilename (module.js:525:25)
    at Function.Module._load (module.js:453:25)
    at Function.Module.runMain (module.js:665:10)
    at startup (bootstrap_node.js:201:16)
    at bootstrap_node.js:626:3


*/5	* * * *   /opt/router-scripts-lede/update-domains >>/var/log/update-domains.log 2>>/var/log/update-domains.error.log
10 3 * * *      /opt/router-scripts-lede/daily >>/var/log/daily.log 2>>/var/log/daily.error.log
8 4 * * *       /opt/router-scripts-lede/backup-daily >>/var/log/backup-daily.log 2>>/var/log/backup-daily.error.log
6 5 * * 0       /opt/router-scripts-lede/backup-weekly >>/var/log/backup-weekly.log 2>>/var/log/backup-weekly.error.log
5 6 1 * *       /opt/router-scripts-lede/backup-monthly >>/var/log/backup-monthly.log 2>>/var/log/backup-monthly.error.log
9 * * * *      /opt/router-scripts-lede/cron-fail2ban-check  >>/var/log/fail2ban-check.log 2>>/var/log/fail2ban-check.error.log

@p3x-robot
Copy link

this is my error.

@p3x-robot
Copy link

I am using 8.5

@BridgeAR
Copy link
Member

@p3x-robot thanks a lot for that. I am still not sure what you executed though. Can you please show the exact used command and only that?

@p3x-robot
Copy link

for me it is working with 8.6

@BridgeAR
Copy link
Member

BridgeAR commented Oct 2, 2017

Hm weird. It would still be good to know what happened but I am closing this as it seems to be resolved.

@BridgeAR BridgeAR closed this as completed Oct 2, 2017
@avatar-lavventura
Copy link

avatar-lavventura commented Mar 12, 2018

I am using node version v8.2.1 and still having the same error. Is it solved on higher versions? I guess this bug is not resolved. @BridgeAR

@BridgeAR
Copy link
Member

@avatar-lavventura yes, using a newer version should resolve the problem.

@BridgeAR BridgeAR reopened this Mar 12, 2018
@gireeshpunathil
Copy link
Member

this is resolved in 8.11.1 LTS and in the current (10.0), I have verified it. So I guess this can be closed.

@gireeshpunathil
Copy link
Member

ping @evanlucas

@addaleax
Copy link
Member

@gireeshpunathil I’d guess that this was fixed by accident and we maybe want a regression test for it?

bcoe pushed a commit to bcoe/node-1 that referenced this issue May 4, 2018
mhdawson pushed a commit that referenced this issue May 10, 2018
Refs: #11257

PR-URL: #20391
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
targos pushed a commit that referenced this issue May 12, 2018
Refs: #11257

PR-URL: #20391
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
@gireeshpunathil
Copy link
Member

this is fixed in the latest, and a regression test is added now through #20391

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
7 participants