Skip to content

Commit

Permalink
stream: emit .write() end error in next tick.
Browse files Browse the repository at this point in the history
This changes the behaviour of error event emitted when writing to a stream
after it has ended, from synchronously to asynchronously.

PR-URL: #4749
  • Loading branch information
phillipj committed Jan 21, 2016
1 parent eee9dc7 commit 1899689
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 11 deletions.
11 changes: 5 additions & 6 deletions lib/_stream_writable.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,10 @@ Writable.prototype.pipe = function() {
};


function writeAfterEnd(stream, cb) {
var er = new Error('write after end');
// TODO: defer error events consistently everywhere, not just the cb
stream.emit('error', er);
process.nextTick(cb, er);
function writeAfterEndErr(stream, cb) {
const err = new Error('write after end');
stream.emit('error', err);
cb(err);
}

// If we get something that is not a buffer, string, null, or undefined,
Expand Down Expand Up @@ -201,7 +200,7 @@ Writable.prototype.write = function(chunk, encoding, cb) {
cb = nop;

if (state.ended)
writeAfterEnd(this, cb);
process.nextTick(writeAfterEndErr, this, cb);
else if (validChunk(this, state, chunk, cb)) {
state.pendingcb++;
ret = writeOrBuffer(this, state, chunk, encoding, cb);
Expand Down
14 changes: 9 additions & 5 deletions test/parallel/test-file-write-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ var EXPECTED = '012345678910';
var callbacks = {
open: -1,
drain: -2,
close: -1
close: -1,
error: -1
};

file
Expand All @@ -25,6 +26,10 @@ file
assert.equal('number', typeof fd);
})
.on('error', function(err) {
// we're expecting write after end error
if (err.message === 'write after end') {
return callbacks.error++;
}
throw err;
})
.on('drain', function() {
Expand All @@ -43,10 +48,9 @@ file
assert.strictEqual(file.bytesWritten, EXPECTED.length * 2);

callbacks.close++;
assert.throws(function() {
console.error('write after end should not be allowed');
file.write('should not work anymore');
});

console.error('write after end should not be allowed');
file.write('should not work anymore');

fs.unlinkSync(fn);
});
Expand Down
31 changes: 31 additions & 0 deletions test/parallel/test-stream-writeable-write-ended-err.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';

require('../common');

const assert = require('assert');
const Writable = require('stream').Writable;

const stream = new Writable({ write });

let errorsRecorded = 0;

function write() {
throw new Error('write() should not have been called!');
}

function errorRecorder(err) {
if (err.message === 'write after end') {
errorsRecorded++;
}
}

// should trigger ended errors when writing later
stream.end();

stream.on('error', errorRecorder);
stream.write('this should explode', errorRecorder);

assert.equal(errorsRecorded, 0,
'Waits until next tick before emitting error');

process.nextTick(() => assert.equal(errorsRecorded, 2));

0 comments on commit 1899689

Please sign in to comment.