In writable stream, should we always set state.errorEmitted to true when executing stream.emit('error')?

I have read the source code of lib/_stream_writable.js. I have a doubt about the meaning of state.errorEmitted.

Why do we set state.errorEmitted to true in some situations of stream.emit('error'), but doesn't set it in other situations. Moreover, it doesn't achieve emitting 'error' event only one time. For example, as shown below, the code will emit 'error' event two times :

const { Writable } = require('stream');

w = new Writable();
w.on('error', error => {
  console.log('on error.', error);
});
w.write(null);
w.write(null);

@mcollina

@mcollina

@MoonBall

@MoonBall as a Node.js user, it means nothing. Internally it is used in some checks (in destroy()) to avoid doing certain things if the condition is met. This state variable is also used throughout core (which we should refactor according to #445).

Making the change you propose is hard, but if you want to take a stab at it I will be happy review it.

if it only work in destroy(), we can modify it's name or it's comments, so that we can better understand it. The comment(https://github.com/nodejs/node/blob/master/lib/_stream_writable.js#L144) is not appropriate.

It is not just destroy: https://github.com/nodejs/node/search?utf8=%E2%9C%93&q=errorEmitted&type=.

Ideally all of that should not be needed, would you like to investigate?

ok. I try to do it.

@MoonBall

/ping @MoonBall @mcollina Should this remain open?

I think this should be closed.

Navigation Menu

Repository navigation

In writable stream, should we always set `state.errorEmitted` to true when executing `stream.emit('error')`? #18181

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

In writable stream, should we always set state.errorEmitted to true when executing stream.emit('error')? #18181

Description

Activity

MoonBall commented on Jan 17, 2018

mcollina commented on Jan 17, 2018

MoonBall commented on Jan 17, 2018

mcollina commented on Jan 17, 2018

MoonBall commented on Jan 18, 2018

Trott commented on Nov 25, 2018

mcollina commented on Nov 26, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

In writable stream, should we always set `state.errorEmitted` to true when executing `stream.emit('error')`? #18181