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

Setting stream to process.stdout causes TypeError: s.stream.write is not a function #332

Closed
vrghost242 opened this issue Jan 15, 2016 · 9 comments

Comments

@vrghost242
Copy link

So thought I would test this logger as it seems really good.

Thought I would create the logger with three levels to start with, so set it us as follows:

var bunyan = require('bunyan');
var log = bunyan.createLogger({name: 'MAIN',
  streams: [
    {
      level: 'info',
      stream: 'process.stdout'
    },
    {
      level: 'warn',
      stream: 'process.stdout'
    },
    {
      level: 'error',
      path: 'process.stdout'
    }
]});

However, if I start the application with this code I get:

/Users/bengtbjorkberg/WebstormProjects/tentogive-https/node_modules/bunyan/lib/bunyan.js:875
            s.stream.write(s.raw ? rec : str);
                     ^

TypeError: s.stream.write is not a function
    at Logger._emit (/Users/bengtbjorkberg/WebstormProjects/tentogive-https/node_modules/bunyan/lib/bunyan.js:875:22)
    at Logger.info (/Users/bengtbjorkberg/WebstormProjects/tentogive-https/node_modules/bunyan/lib/bunyan.js:974:24)
    at Object.<anonymous> (/Users/bengtbjorkberg/WebstormProjects/tentogive-https/tentogive-https.js:27:5)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Function.Module.runMain (module.js:467:10)
    at startup (node.js:136:18)
    at node.js:963:3

Process finished with exit code 1

I also get a empty file called stdout.process

However, id I remove "streams"

var bunyan = require('bunyan');
var log = bunyan.createLogger({name: 'MAIN'});

I get the correct output to the terminal.
Not certain if this is key, but I am on a mac OSX develolpment machine

@msealand
Copy link

It looks like you just have a configuration issue. process.stdout should not be wrapped in quotes, and the last stream in your array should using stream instead of path (using path is why it created an empty file by the way).

So, I think what you want is this:

var bunyan = require('bunyan');
var log = bunyan.createLogger({name: 'MAIN',
  streams: [
    {
      level: 'info',
      stream: process.stdout
    },
    {
      level: 'warn',
      stream: process.stdout
    },
    {
      level: 'error',
      stream: process.stdout
    }
]});

@trentm
Copy link
Owner

trentm commented Jan 23, 2016

@msealand thanks for answering

I think I'll leave this ticket open to have Bunyan emit a clearer warning when that stream is first added... rather than obtusely error out later when writing the first record.

@Khez
Copy link

Khez commented Feb 2, 2016

@trentm We use a YAML config to generate configs for various loggers throughout our application. Because of this we had strings for process.stdout and process.stderr exactly like this issue.

We extended bunyan to support strings for these two streams, but we didn't like doing string checks. We ended up adding a new stream.type called process that supports the strings stdout and stderr

I could make a PR for strict checks on addStream for write method and/or a new config stream type that supports strings for process :?

@trentm
Copy link
Owner

trentm commented Feb 22, 2016

@Khez Yes, that has been discussed before. Here is a relevant other issue: #153

I've shied away from having Bunyan have a way to "spell" process.stdout/stderr as a string for loading Bunyan config from a JSON/YAML/whatever format config file.

I lean toward more fully specing a "bunyan config from config" (including things like specifying serializers to use, etc.), very likely as a separate module. If that module got popular, I'd consider merging it in (if that author wanted to, of course).

@Khez thanks for the PR offer, I think #335 from @michaelnisi will be that.

@michaelnisi
Copy link
Contributor

@Khez #355

@Khez
Copy link

Khez commented Feb 23, 2016

@michaelnisi This looks like an ok thing to add, we actually have a very similar check (amongst others)

Unfortunately this by itself does not fix the string process.stdout or process.stderr issue that @vrghost242 and me are trying to find a way to solve for future developers.

Also made a comment on #355. Keep up the good work. 👍

@michaelnisi
Copy link
Contributor

I find crashing early and loudly often the best choice. With #355 @vrghost242 would get:

assert.js:86
  throw new assert.AssertionError({
        ^
AssertionError: "stream" stream is not writable: 'process.stdout'
    at Logger.addStream (/Users/michael/workspace/node-bunyan/lib/bunyan.js:579:16)
    at /Users/michael/workspace/node-bunyan/lib/bunyan.js:469:18
    at Array.forEach (native)
    at new Logger (/Users/michael/workspace/node-bunyan/lib/bunyan.js:468:25)
    at Function.createLogger (/Users/michael/workspace/node-bunyan/lib/bunyan.js:1501:12)
    at Object.<anonymous> (/Users/michael/workspace/node-bunyan/t.js:2:18)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)

The error message still needs some work, I guess.

@trentm trentm closed this as completed in cf1926f Feb 29, 2016
@zamrokk
Copy link

zamrokk commented Jun 8, 2016

Well, I have the error message but how to finally make it work with a config file in JSON format ?

I am going to give up using "stream":process.stdout

@evgenymarkov
Copy link

evgenymarkov commented May 12, 2018

Same problem

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

7 participants