Stream.pipeline doesn't report failures if process.stdout is in the pipeline.

Version: v11.11.0
Platform: Linux 3.10.0-693.21.1.el7.x86_64
Subsystem:

Below is a simple example of a pipeline starting with a Readable, Transform, and then process.stdout. Failures in the Transform do not get reported by Stream.pipeline.

const { Transform, Readable, pipeline } = require('stream')

const reader = new Readable({
  read(size) { this.push("foo") }
})

let count = 0;

const transform = new Transform({
  transform(chunk, enc, cb) {
    if (count++ >= 5)
      this.emit("error", new Error("this-error-gets-hidden"))
    else
      cb(null, count.toString() + "\n")
  }
})

pipeline(
  reader,
  transform,
  process.stdout,
  e => e ? console.error(e) : console.log("done")
)

The error that is emitted never shows up in the pipeline's callback. The pipeline does stop on the 5th iteration as expected, but the error gets gobbled up somehow. If process.stdout is removed from the pipeline, the error is displayed as expected.

Interestingly, the following works as expected:

pipeline(
  reader,
  transform,
  e => e ? console.error(e) : console.log("done")
).pipe(
  process.stdout
)

you can handle the error by yourself to fix it

const { Transform, Readable, pipeline } = require('stream')

const reader = new Readable({
  read (size) {
    this.push('foo')
  }
})

let count = 0

const transform = new Transform({
  transform (chunk, enc, cb) {
    if (count++ >= 5) {
      this.emit('error', new Error('this-error-gets-hidden'))
    } else {
      cb(null, count.toString() + '\n')
    }
  }
})

// handle it
transform.on('error', (e) => {
  console.log(e)
})

pipeline(
  reader,
  transform,
  process.stdout,
  e => e ? console.error(e) : console.log('done')
)

I found that onerror handler only just destroy itself and not to deliver the error message 😕

node/lib/_stream_readable.js

Lines 715 to 721 in 6e81a95

    
           function onerror(er) { 
        
             debug('onerror', er); 
        
             unpipe(); 
        
             dest.removeListener('error', onerror); 
        
             if (EE.listenerCount(dest, 'error') === 0) 
        
               errorOrDestroy(dest, er); 
        
           }

process.stdout actually ends up with two 'error' listeners, the other one being:

node/lib/internal/streams/end-of-stream.js

Lines 52 to 54 in 6e81a95

    
           const onerror = (err) => { 
        
             callback.call(stream, err); 
        
           };

I speculate errorOrDestroy() is never actually called.

@tadjik1

I don't think this is a bug in pipeline, but rather in

node/lib/internal/fs/sync_write_stream.js

Lines 27 to 36 in 6f77af5

    
           SyncWriteStream.prototype._destroy = function() { 
        
             if (this.fd === null) // already destroy()ed 
        
               return; 
        
             if (this.autoClose) 
        
               closeSync(this.fd); 
        
             this.fd = null; 
        
             return true; 
        
           };

.

cc @tadjik1

@mcollina

@mcollina my PR fixes this problem as well:

const { Transform, Readable, pipeline } = require('stream')
const fs = require('fs')

const reader = new Readable({
  read(size) { this.push("foo") }
})

let count = 0;

const transform = new Transform({
  transform(chunk, enc, cb) {
    if (count++ >= 5)
      this.emit("error", new Error("this-error-gets-hidden"))
    else
      cb(null, count.toString() + "\n")
  }
})

pipeline(
  reader,
  transform,
  process.stdout,
  e => fs.writeFileSync('out.txt', `done with ${e ? e.message : 'no error'}`)
)

note: I'm not using console.log because when stdout destroyed it's no longer possible to print messages

Here is a fix for this specific issue: #26691.

The problem was caused by net.Socket having set emitClose: false because it was emitted its own event in _destroy(), and stdio.js was overriding _destroy without emitting 'close'. That PR fixes that.

process: make stdout and stderr emit 'close' on destroy

Doesn't seem to be fixed using the following versions:

$ npm -v
6.9.0
$ node --version
v10.16.3

npm outdated returns nothing.

As a workaround I used this:

class Workaround extends stream.Writable {
  constructor(){
    super()
  }
  _write(chunk,enc,callback){
    process.stdout.write(chunk)
    callback(null)
  }
  _final(callback){
    process.stdout.write('Workaround _final was called\n')
    callback(null)
  }  
}

The _final not necessary, it is only there to see if it got called. It seems that _final doesn't get called. Unsure whether that is meaningful or not.

Is it just a matter of delay between fix and stable version?

I think it's a separate bug, because the script at the top reports the error on both 10.16.3 and 12.11, but it does not on 10.16.0.

Can you open a new issue with a full script to reproduce?

I think it's a separate bug, because the script at the top reports the error on both 10.16.3 and 12.11, but it does not on 10.16.0.

Can you open a new issue with a full script to reproduce?

On the other hand, maybe the default of trying to close the last stream is OK, and for those specials cases (like stdout) where its not desirable, the workaround is pretty simple.

I do not think I can help without a full repro of the problem you are experiencing. Can you open a new issue with a way to reproduce and tag me? Thanks

process: make stdout and stderr emit 'close' on destroy

Navigation Menu

Repository navigation

Stream.pipeline doesn't report failures if process.stdout is in the pipeline. #26550

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	function onerror(er) {
	debug('onerror', er);
	unpipe();
	dest.removeListener('error', onerror);
	if (EE.listenerCount(dest, 'error') === 0)
	errorOrDestroy(dest, er);
	}

	SyncWriteStream.prototype._destroy = function() {
	if (this.fd === null) // already destroy()ed
	return;

	if (this.autoClose)
	closeSync(this.fd);

	this.fd = null;
	return true;
	};

Stream.pipeline doesn't report failures if process.stdout is in the pipeline. #26550

Description

Activity

himself65 commented on Mar 10, 2019

himself65 commented on Mar 10, 2019

bnoordhuis commented on Mar 12, 2019

mcollina commented on Mar 15, 2019

tadjik1 commented on Mar 15, 2019

mcollina commented on Mar 15, 2019

craigphicks commented on Sep 25, 2019

mcollina commented on Sep 26, 2019

mcollina commented on Sep 26, 2019

craigphicks commented on Nov 5, 2019

mcollina commented on Nov 5, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions