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

Duplex stream is not returning whether it is in object mode correctly #33388

Closed
alesmenzel opened this issue May 13, 2020 · 4 comments
Closed
Labels
confirmed-bug Issues with confirmed bugs. stream Issues and PRs related to the stream subsystem.

Comments

@alesmenzel
Copy link

  • Version: v12.16.0
  • Platform: MacOS
  • Subsystem: Darwin ... 19.4.0 Darwin Kernel Version 19.4.0: Wed Mar 4 22:28:40 PST 2020; root:xnu-6153.101.6~15/RELEASE_X86_64 x86_64

What steps will reproduce the bug?

// open the node repl
x = new stream.Duplex({writableObjectMode: true})
x.writableObjectMode // undefined - should be true

but the mode is set correctly:

...
_writableState: WritableState {
    objectMode: true,
   ...

it does work on a Writable stream:

x = new stream.Writable({objectMode: true})
x.writableObjectMode // true

How often does it reproduce? Is there a required condition?

100%

What is the expected behavior?

Return the actual value of whether the stream is in object mode.

@addaleax addaleax added stream Issues and PRs related to the stream subsystem. confirmed-bug Issues with confirmed bugs. labels May 13, 2020
@ArfatSalman
Copy link

ArfatSalman commented May 13, 2020

Version: v14.2.0

x = new stream.Duplex({ writableObjectMode: true })
// console.log(x.writable);

const prop = 'writableObjectMode';
const hasProp = Object.prototype.hasOwnProperty;

function listProtoChain(x) {
  while (x !== null) {
    console.log(hasProp.call(x, prop));
    x = Object.getPrototypeOf(x);
  }
}

listProtoChain(x);

// Output
// false
// false
// false
// false
// false
// false

I was investigating this and for some reason this code was giving false 6 times. Which leads me to conclude that property writableObjectMode was not found any where in the chain. Which was surprising since this property is set on Writable.prototype in the _stream_writable.js file. I don't know whether this is correct or not.

If I can get some help in this, and if this is somewhat an easy issue, I would like to make my first contribution. 😊

@jasnell
Copy link
Member

jasnell commented May 13, 2020

Yep, definitely a bug. If you look here you'll find where writableObjectMode is defined on the stream.Writable. Then here where the various property definitions are copied into stream.Duplex. All that would be needed to fix this I think is adding the following into _stream_duplex:

  writableObjectMode:
    ObjectGetOwnPropertyDescriptor(Writable.prototype, 'writableObjectMode'),

Then add a regression test in test/parallel that tests for the presence of the property on stream.Duplex

@ronag
Copy link
Member

ronag commented May 13, 2020

@jasnell: Ah, sorry, guess this would have been a good first issue and I was a bit too fast :/.

ronag added a commit to nxtedition/node that referenced this issue May 13, 2020
Duplex did not properly forward writableObjectMode.

Fixes: nodejs#33388
@ArfatSalman
Copy link

@jasnell Thanks a lot for the guidelines. I see @ronag has already fixed it! 😊

@ronag ronag closed this as completed in 2361b5c May 18, 2020
codebytere pushed a commit that referenced this issue May 21, 2020
Duplex did not properly forward writableObjectMode.

Fixes: #33388

PR-URL: #33390
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants