Skip to content

Support writing the logs to stderr when i/o is redirected#86

Merged
mcollina merged 1 commit intomasterfrom
stderr
Sep 13, 2016
Merged

Support writing the logs to stderr when i/o is redirected#86
mcollina merged 1 commit intomasterfrom
stderr

Conversation

@mcollina
Copy link
Copy Markdown
Member

@mcollina mcollina commented Sep 11, 2016 •
edited
Loading

Fixes #85.

Please review, cc @krazylek

@mcollina
Copy link
Copy Markdown
Member Author

mcollina commented Sep 13, 2016

@jsumners can you have a look here?

@jsumners
Copy link
Copy Markdown
Member

jsumners commented Sep 13, 2016

@mcollina I don't think this is affected by the extreme mode buffer flushing. _writableState doesn't track whether or not the backing file descriptor is blockable. Indeed, I pulled this patch into my feature branch and ran the tests. Everything is checking out just fine.

@mcollina
Copy link
Copy Markdown
Member Author

mcollina commented Sep 13, 2016

@jsumners the problem is that stderr do not have a _writableState, because it inherits directly from Stream.

@jsumners
Copy link
Copy Markdown
Member

jsumners commented Sep 13, 2016

It looks to have one to me:

> process.stderr
WriteStream {
  // *snip*
  _writableState:
   WritableState {
     objectMode: false,
     highWaterMark: 16384,
     needDrain: false,
     ending: false,
     ended: false,
     finished: false,
     decodeStrings: false,
     defaultEncoding: 'utf8',
     length: 0,
     writing: false,
     corked: 0,
     sync: true,
     bufferProcessing: false,
     onwrite: [Function],
     writecb: null,
     writelen: 0,
     bufferedRequest: null,
     lastBufferedRequest: null,
     pendingcb: 0,
     prefinished: false,
     errorEmitted: false,
     bufferedRequestCount: 0,
     corkedRequestsFree: CorkedRequest { next: null, entry: null, finish: [Function] } },
  writable: true,
  allowHalfOpen: false,
  destroyed: false,
  _bytesDispatched: 0,
  _sockname: null,
  _writev: null,
  _pendingData: null,
  _pendingEncoding: '',
  server: null,
  _server: null,
  columns: 130,
  rows: 50,
  _type: 'tty',
  fd: 2,
  _isStdio: true,
  destroySoon: [Function],
  destroy: [Function] }

I'm not clear what you want me to investigate here.

@mcollina
Copy link
Copy Markdown
Member Author

mcollina commented Sep 13, 2016

The original issue:

$ node a.js 2> out
SyncWriteStream {
  domain: null,
  _events: {},
  _eventsCount: 0,
  _maxListeners: undefined,
  fd: 2,
  writable: true,
  readable: false,
  autoClose: false,
  _type: 'fs',
  _isStdio: true,
  destroySoon: [Function],
  destroy: [Function] }

The content of a.js:

console.log(process.stderr)

As you see, it's our old friend SyncWriteStream.

@mcollina mcollina changed the title Support writing the logs to stderr Support writing the logs to stderr when i/o is redirected Sep 13, 2016
@jsumners
Copy link
Copy Markdown
Member

jsumners commented Sep 13, 2016

I see. I'll dig into it soon and see what I can figure out.

@mcollina
Copy link
Copy Markdown
Member Author

mcollina commented Sep 13, 2016

@jsumners this is already passing, just check if it's ok for you.

@jsumners
Copy link
Copy Markdown
Member

jsumners commented Sep 13, 2016

Yep. The tests pass and the following does what you'd expect:

var pino = require('./')
var log = pino(process.stderr)
log.info(process.stderr)

@mcollina mcollina merged commit ca72692 into master Sep 13, 2016
@mcollina mcollina deleted the stderr branch September 13, 2016 14:00
This was referenced Sep 28, 2016
Closed
Closed
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 9, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Feb 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Can't log to stderr in tricky case

2 participants

@mcollina @jsumners