stream: writableEnded#28934

ronag · 2019-08-02T06:11:01Z

This is work towards resolving the response.finished confusion and future deprecation.

Note that implementation wise streams have both an ending and ended state. However, in this case (in order to avoid confusion in user space) writableEnded is equal to writable.ending. The ending vs ended situation is internal state required for internal stream logic.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

ronag · 2019-08-02T06:20:18Z

ping @mcollina @benjamingr

mcollina · 2019-08-02T07:00:19Z

Could you add some text that shows the relation between this and writableFinished?

Good work!

ronag · 2019-08-02T07:11:37Z

@mcollina Updated

mcollina

LGTM

vsemozhetbyt

Some doc nits.

doc/api/stream.md

doc/api/http2.md

doc/api/http.md

doc/api/http2.md

nodejs-github-bot · 2019-08-03T09:56:42Z

CI: https://ci.nodejs.org/job/node-test-pull-request/24835/

mcollina · 2019-08-03T16:54:47Z

Would you mind rebase on top of master?

ronag · 2019-08-03T19:39:51Z

done

btw, what's up with the defineProperty configs that seem to be a bit all over?

Trott · 2019-08-17T06:12:36Z

A request: Your PR descriptions are usually very good, but your commit messages are usually very terse. I invariably end up amending the commit message using a bunch of stuff from the PR description. If you could be descriptive in the commit messages (or at least the first commit message--subsequent fixup commits get squashed away anyway), that would make landing a little bit easier. If you have a good reason why your commit messages are terse, that's fine, I can keep updating the commit messages upon landing. But if it's all the same to you, more info in the commit messages please! (Putting this publicly because you're not the only person for whom this is true so maybe others will see it and respond accordingly. Not trying to shame anyone or anything. I've certainly had more than my own share of commit messages that didn't even contain a verb.)

Trott

LGTM if CI passes.

ronag · 2019-08-17T06:18:20Z

A request:

... I have no reason other than that's how I'm used to do it :). I'll try to improve in future PR's & commits.

nodejs-github-bot · 2019-08-17T06:41:59Z

CI: https://ci.nodejs.org/job/node-test-pull-request/25045/

Trott · 2019-08-17T07:28:29Z

Landed in 6f613d8

This is work towards resolving the response.finished confusion and future deprecation. Note that implementation-wise, streams have both an ending and ended state. However, in this case (in order to avoid confusion in user space) writableEnded is equal to writable.ending. The ending vs ended situation is internal state required for internal stream logic. PR-URL: #28934 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Notable changes: * **crypto**: * Added an oaepHash option to asymmetric encryption which allows users to specify a hash function when using OAEP padding (Tobias Nießen) [#28335](#28335). * **deps**: * Updated V8 to 7.6.303.29 (Michaël Zasso) [#28955](#28955). * Improves the performance of various APIs such as `JSON.parse` and methods called on frozen arrays. * Adds the [`Promise.allSettled`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/allSettled) method. * Improves support of `BigInt` in `Intl` methods. * For more information: https://v8.dev/blog/v8-release-76 * Updated libuv to 1.31.0 (cjihrig) [#29070](#29070). * `UV_FS_O_FILEMAP` has been added for faster access to memory mapped files on Windows. * `uv_fs_mkdir()` now returns `UV_EINVAL` for invalid filenames on Windows. It previously returned `UV_ENOENT`. * The `uv_fs_statfs()` API has been added. * The `uv_os_environ()` and `uv_os_free_environ()` APIs have been added. * **fs**: * Added `fs.writev` and `fs.writevSync` methods. They allow to write an array of `ArrayBufferView`s to a file descriptor (Anas Aboureada) [#25925](#25925). * **http**: * Added three properties to `OutgoingMessage.prototype`: `writableObjectMode`, `writableLength` and `writableHighWaterMark` [#29018](#29018). * **stream**: * Added an new property `writableEnded` to writable streams. Its value is set to `true` after `writable.end()` has been called. (Robert Nagy) [#28934](#28934). PR-URL: #29210

Notable changes: * crypto: * Added an oaepHash option to asymmetric encryption which allows users to specify a hash function when using OAEP padding. #28335 * deps: * Updated V8 to 7.6.303.29. #28955 * Improves the performance of various APIs such as `JSON.parse` and methods called on frozen arrays. * Adds the Promise.allSettled method. * Improves support of `BigInt` in `Intl` methods. * For more information: https://v8.dev/blog/v8-release-76 * Updated libuv to 1.31.0. #29070 * `UV_FS_O_FILEMAP` has been added for faster access to memory mapped files on Windows. * `uv_fs_mkdir()` now returns `UV_EINVAL` for invalid filenames on Windows. It previously returned `UV_ENOENT`. * The `uv_fs_statfs()` API has been added. * The `uv_os_environ()` and `uv_os_free_environ()` APIs have been added. * fs: * Added `fs.writev`, `fs.writevSync` and `filehandle.writev` (promise version) methods. They allow to write an array of `ArrayBufferView`s to a file descriptor. #25925 https://github.com/nodejs/node/pull/29186/files * http: * Added three properties to `OutgoingMessage.prototype`: `writableObjectMode`, `writableLength` and `writableHighWaterMark` #29018 * stream: * Added an new property `readableEnded` to readable streams. Its value is set to `true` when the `'end'` event is emitted. #28814 * Added an new property `writableEnded` to writable streams. Its value is set to `true` after `writable.end()` has been called. #28934 PR-URL: #29210

Notable changes: * crypto: * Added an oaepHash option to asymmetric encryption which allows users to specify a hash function when using OAEP padding. nodejs#28335 * deps: * Updated V8 to 7.6.303.29. nodejs#28955 * Improves the performance of various APIs such as `JSON.parse` and methods called on frozen arrays. * Adds the Promise.allSettled method. * Improves support of `BigInt` in `Intl` methods. * For more information: https://v8.dev/blog/v8-release-76 * Updated libuv to 1.31.0. nodejs#29070 * `UV_FS_O_FILEMAP` has been added for faster access to memory mapped files on Windows. * `uv_fs_mkdir()` now returns `UV_EINVAL` for invalid filenames on Windows. It previously returned `UV_ENOENT`. * The `uv_fs_statfs()` API has been added. * The `uv_os_environ()` and `uv_os_free_environ()` APIs have been added. * fs: * Added `fs.writev`, `fs.writevSync` and `filehandle.writev` (promise version) methods. They allow to write an array of `ArrayBufferView`s to a file descriptor. nodejs#25925 https://github.com/nodejs/node/pull/29186/files * http: * Added three properties to `OutgoingMessage.prototype`: `writableObjectMode`, `writableLength` and `writableHighWaterMark` nodejs#29018 * stream: * Added an new property `readableEnded` to readable streams. Its value is set to `true` when the `'end'` event is emitted. nodejs#28814 * Added an new property `writableEnded` to writable streams. Its value is set to `true` after `writable.end()` has been called. nodejs#28934 PR-URL: nodejs#29210

Notable changes: * crypto: * Added an oaepHash option to asymmetric encryption which allows users to specify a hash function when using OAEP padding. #28335 * deps: * Updated V8 to 7.6.303.29. #28955 * Improves the performance of various APIs such as `JSON.parse` and methods called on frozen arrays. * Adds the Promise.allSettled method. * Improves support of `BigInt` in `Intl` methods. * For more information: https://v8.dev/blog/v8-release-76 * Updated libuv to 1.31.0. #29070 * `UV_FS_O_FILEMAP` has been added for faster access to memory mapped files on Windows. * `uv_fs_mkdir()` now returns `UV_EINVAL` for invalid filenames on Windows. It previously returned `UV_ENOENT`. * The `uv_fs_statfs()` API has been added. * The `uv_os_environ()` and `uv_os_free_environ()` APIs have been added. * fs: * Added `fs.writev`, `fs.writevSync` and `filehandle.writev` (promise version) methods. They allow to write an array of `ArrayBufferView`s to a file descriptor. #25925 #29186 * http: * Added three properties to `OutgoingMessage.prototype`: `writableObjectMode`, `writableLength` and `writableHighWaterMark` #29018 * stream: * Added an new property `readableEnded` to readable streams. Its value is set to `true` when the `'end'` event is emitted. #28814 * Added an new property `writableEnded` to writable streams. Its value is set to `true` after `writable.end()` has been called. #28934 PR-URL: #29210

nodejs-github-bot added http Issues or PRs related to the http subsystem. http2 Issues or PRs related to the http2 subsystem. stream Issues and PRs related to the stream subsystem. labels Aug 2, 2019

ronag force-pushed the stream-writableEnded branch from f07dd71 to b5047faCompare August 2, 2019 06:18

ronag force-pushed the stream-writableEnded branch 2 times, most recently from 9781b2e to 2acbd15Compare August 2, 2019 06:24

ronag mentioned this pull request Aug 2, 2019
doc: deprecate finished #28679
Closed
4 tasks

ronag changed the title ~~Stream writable ended~~stream: writableEndedAug 2, 2019

ronag force-pushed the stream-writableEnded branch from 2acbd15 to d4dfee9Compare August 2, 2019 07:11

mcollina approved these changes Aug 2, 2019
View reviewed changes

ronag force-pushed the stream-writableEnded branch 3 times, most recently from a6b8aa1 to 80afc1eCompare August 2, 2019 09:22

vsemozhetbyt reviewed Aug 2, 2019
View reviewed changes

ronag force-pushed the stream-writableEnded branch 2 times, most recently from 7c29569 to 6ab4ad4Compare August 2, 2019 16:53

trivikr requested changes Aug 2, 2019
View reviewed changes

doc/api/http2.md Outdated Show resolvedHide resolved

trivikr approved these changes Aug 2, 2019
View reviewed changes

benjamingr approved these changes Aug 3, 2019
View reviewed changes

ronag force-pushed the stream-writableEnded branch 3 times, most recently from 2ad49f1 to 3c8930dCompare August 3, 2019 19:37

stream: writableEnded
e42af56

ronag force-pushed the stream-writableEnded branch from 3c8930d to 9dd1575Compare August 3, 2019 19:39

Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 17, 2019

Trott approved these changes Aug 17, 2019
View reviewed changes

Trott closed this Aug 17, 2019

targos mentioned this pull request Aug 19, 2019
v12.9.0 release proposal #29210
Merged

Uh oh!

stream: writableEnded#28934

stream: writableEnded #28934

Uh oh!

Conversation

ronag commented Aug 2, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

ronag commented Aug 2, 2019

Uh oh!

mcollina commented Aug 2, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ronag commented Aug 2, 2019

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 3, 2019

Uh oh!

mcollina commented Aug 3, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ronag commented Aug 3, 2019

Uh oh!

Trott commented Aug 17, 2019

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

ronag commented Aug 17, 2019

Uh oh!

nodejs-github-bot commented Aug 17, 2019

Uh oh!

Trott commented Aug 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ronag commented Aug 2, 2019•
edited
Loading

mcollina commented Aug 2, 2019•
edited
Loading

mcollina commented Aug 3, 2019•
edited
Loading