Patch for a hole in loglevel='silent' when using npm programmatically #4320

Closed
wants to merge 1 commit into
from

Projects

None yet
@mikermcneil

Issue

When using npm.commands.install programmatically, a "pretty" console.log describing the installed modules is run, even when config.loglevel=silent.

Here's a simple test script which demonstrates the issue (modified from the example here)

var npm = require('npm');
npm.load({
    registry: 'https://registry.npmjs.org',
    loglevel: 'silent'
}, function(err) {
    if (err) throw new Error(err);

    npm.commands.install(['lodash'], function(err, data) {
        // ...
    });
});

Proposed Solution

Checks if npm.config.loglevel === 'silent', and skips writing a log if it does.

@isaacs If there's another way you'd rather go about approaching this, I'm happy to-- just figured this was the intended behavior ( it's what I was expecting when I configured silent, at least ) Thanks!

@domenic
Member
domenic commented Dec 16, 2013

I think the issue is that we're using console.log instead of whatever the correct logging function is. We do that a lot throughout npm, though I am not sure why.

@mikermcneil

hmm that would make sense-- wonder what was going on here?

@isaacs
Member
isaacs commented Dec 16, 2013

This is not a hole. The "pretty" (or --json or --parseable) output is the "real" output here, not a stderr debug logging. --loglevel=silent silences all logging but not all output.

To do that, you can use this option instead: >/dev/null

@isaacs isaacs closed this Dec 16, 2013
@mikermcneil

Ah, I see. Just thought it was worth mentioning since this particular output is skipped when you run npm install --loglevel=silent, but not when you do the same thing programmatically.

@sethkinast

I just experienced the same issue in that I'd like to run npm from within my application and not have anything be output (but I of course want normal output from my program so I can't redirect all of its stdout to /dev/null).

@domenic
Member
domenic commented Jan 14, 2014

In the end npm is not very well designed for programmatic use; this is just one of many symptoms thereof :(

@mikermcneil

@sethkinast Something that didn't occur to me at first was that doing require('npm') adds a lot of weight to your dependencies-- in our case it was prohibitive. But as @domenic mentioned, npm wasn't really designed for that.

I put together this wrapper (enpeem) for our team to use-- by no means complete, but at least allows you to do the most basic thing (install a module programmatically without logging anything). It only works if npm is already installed on the system (which it almost certainly is-- otherwise how did you get enpeem?).

It just runs npm -v with require('child-process').exec() and then, once it's confident npm knows how to hang, npm installs your things, passing programmatic options through to the command-line interface using -- flags.

Since the silent option works with command-line usage, that problem takes care of itself. The README also covers another slip-up I kept running into w/ cache settings-- the fastest I could get it was a couple of seconds (seems npm likes to send network requests), so I ended up going back to using my old method there- since I know where a copy of the dependencies MIGHT be on the user's filesystem (see sails /bin for an example-- you have to resolve nested dependencies)

@sethkinast

I just have npm link npm as a postinstall directive in my package, so no problem adding dep weight. Just wish it didn't pollute the console, but no huge deal.

@Shogun147

+1 for this!

@mikermcneil

@sethkinast nice! Good idea

@jdespatis

I'm using the following command line:
npm --global --loglevel=warn outdated

and it's not quiet at all, I get for example, some debug information on stderr:

info trying registry request attempt 1 at 11:47:04
http GET https://registry.npmjs.org/bower/latest
info trying registry request attempt 1 at 11:47:04
http GET https://registry.npmjs.org/grunt-cli/latest
...

Maybe I'm misusing npm params?

@thejohnfreeman

Just to be clear for anyone else visiting: this issue is closed but not fixed. npm does not handle logging correctly, and has no intention to fix it. Instead, here is a workaround:

    // npm.commands.install is not silent. It prints a tree to stdout. Stash
    // console.log to thwart it.
    var log = console.log
    console.log = function(){}
    npm.commands.install([package], function() {
      // Restore console.log for future use.
      console.log = log
@mikermcneil

@thejohnfreeman indeed.. thanks for sharing that, I'm sure it will help other folks like myself running into the problem

@paulcuth

The issue with @thejohnfreeman's fix is that the operation is asynchronous and therefore any number of other things could be happening at the same time. Setting the global console.log method to an empty function kills the output from all of those other processes and callbacks too.

I'm surprised that this is not considered a bug. There is still no explanation of why --loglevel=silent on the command line silences output that { loglevel: 'silent' } doesn't silence programmatically.

+1 for a solution in npm.

@rlidwka
rlidwka commented Jul 20, 2014

console.log should be on a separate loglevel imho

@othiym23
Collaborator

At this point, I think @isaacs's position on this is very clear – this is an integral part of how npm uses stdout and stderr and is unlikely to change. The good news is that as more of these functions are broken out into the various smaller modules, more control over the output will be given to the callers of those modules, rather than using npm.commands.* and accepting its choices as far as output and formatting.

@mikermcneil

broken out into the various smaller modules

👍

In the mean time, @globegitter and I are working on machinepack-npm.

ps. we'll just wrap said modules directly when you guys snap your fingers-- hitting Couch directly for now in some cases (i.e. for package metadata / search)

@trusktr
trusktr commented Jul 26, 2015

@mikermcneil

It only works if npm is already installed on the system (which it almost certainly is-- otherwise how did you get enpeem?).

People who install Meteor don't need to have npm installed globally. Node and NPM are bundled with Meteor.

@thejohnfreeman That workaround:

This was referenced Jul 26, 2015
@kevin-smets

For me, the workaround of overriding console.log did not work... Would've been nice though..

I had to resort to more overriding in order to have npm be silent:

const LOG_PREFIX = "%log-prefix%";

function log(message) {
    console.log(LOG_PREFIX() + message)
}

// Overwrite stdout so npm will shut up
process.stdout.write = (function (write) {
    return function (string, encoding, fd) {
        if (string.startsWith(LOG_PREFIX)) {
            arguments[0] = string.replace(LOG_PREFIX, "");
            write.apply(process.stdout, arguments);
        }
    }
})(process.stdout.write);

log("blerb");

That way, anything not going through log() will not reach stdout at all. You can easily store the process.stdout.write to a variable or as a return function to restore it.

@mikermcneil

@kevin-smets what worked for us was spinning up the npm install as a child process then gobbling up the output from that:

// Run `npm install`
require('machinepack-process').spawn({
  command: 'npm install',
  dir: '/code/sandbox/some-project/'
}).exec(function (err, bufferedOutput) {
  // ...
});

(see also http://node-machine.org/machinepack-process/spawn)

So when all wrapped up, you end up with:

// Install NPM dependencies of local package at the specified path.
require('machinepack-npm').installDependencies({
  dir: '/code/sandbox/some-project/',
}).exec(function (err) {
  // ...
});
@kevin-smets

@mikermcneil that's indeed a possibility, but I need to use it programmatically, because I depend on the json output. So running it as a cmd is not really an option for my use case sadly. But thanks for the suggestion.

@othiym23
Collaborator
othiym23 commented Feb 9, 2016

@kevin-smets is this a case when the --json option doesn't produce the output you want? The CLI team strongly discourages programmatic use of npm's internal APIs, as they're poorly designed for third-party use, and are a poison forest of jagged edge cases if you don't understand everything about npm's internal architecture.

@kevin-smets

Ugh, can't believe I missed that flag. I will run it as a child process >_<

Thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment