[Mentorship Diary] Matteo and Chetan

[Bamieh]

[Mentorship Diary] Matteo and Chetan

(Mentorship Round 2) @mcollina will be mentoring @ckarande about streams in Node.js

[ckarande]

Kick off Meeting Notes: July 15 2019

Introductions

Matteo introduced himself and scope of his work related to Node streams, http, and readable-stream package. He shared his goal of mentorship to have more contributors on these.

Chetan gave introduction to his experience with Node, work related to Node.js security, and his goals to contribute to Node core and share the learning to have more contributors.

Getting started with Node Core:

As next steps Matteo recommended Chetan to:

• check out https://www.nodetodo.org/ to, setup local fork, running tests, get familiarized with the node core etc.
• Look into some of the open issues at https://github.com/nodejs/node/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22+sort%3Acreated-asc and send PR with fixes
• Get prepared for further discussion on https://github.com/nodejs/node/issues/445 to clean up _writableState and _readableState

Next meeting:

We decided to meet in two weeks (July 29th)

[ankibalyan]

@Bamieh Can these meeting be recorded? This may help to people who want to followup in the same scope?

[ckarande]

Notes: July 29, 2019

Discussion on updates since last meeting

Matteo and Chetan briefly went over the PR Chetan made for issue #28758 and command to run linter locally - make -j4 test

Discussion on Issue 445

As per the option b and a reference fix, Chetan to find out instances where _writableState and _readableState are being used, and start replacing them one at a time. Exclude those references for which the PR already exists.

Next meeting:

Meeting in around two weeks (August 9th)

[Bamieh]

@ankibalyan sorry for the late reply. its up to the mentor/mentee whichever they feel comfortable with doing. I'd certainly would love to see the meetings recorded!

[ckarande]

Notes: August 26, 2019

Matteo and Chetan went over the findings related to use of these two private states of stream:

resumeScheduled

This internal state is used in lib/_http_server.js and lib/internal/http2/compat.js files use this state primarily to check if the stream is actually consumed using .pipe(), resume(), or .on('data')). It is unsafe use this property for this purpose because : 1) This property doesn't update status if a stream is consumed using readable event as .on('readable', cb) 2) The property doesn't revert back to false value when called .unpipe(), .removeAllListerner('data'), or .pause() methods on stream which prevent it from resuming.

As an alternative, Chetan to identify how to create a separate property / method that consistently reflect subscriptions to a stream.

readableListening

It is currently used in lib/internal/child_process.js as below: The purpose of the condition is to give users a chance to consume data from any paused stdio streams before the child process terminates. The check for stream._readableState.readableListening is not necessary here as the resume() method has no effect on the streams subscribed with readable event even if it gets executed.

Chetan to open a PR for this.

Next meeting:

Meeting in around two weeks (Sept 2nd)