Skip to content

doc: update write callback documentation#38959

Closed
simoneb wants to merge 1 commit intonodejs:masterfrom
simoneb:patch-1
Closed

doc: update write callback documentation#38959
simoneb wants to merge 1 commit intonodejs:masterfrom
simoneb:patch-1

Conversation

@simoneb
Copy link
Contributor

@simoneb simoneb commented Jun 7, 2021 •
edited by aduh95
Loading

  • replace may or may not be called with will be called because the callback is always called
  • remove To reliably detect write errors, add a listener for the 'error' event because the error event will NOT be emitted if a write occurs after the stream has been closed

Fixes: #38704

@simoneb
doc: update write callback documentation
453d7db 
- replace _*may or may not* be called_ with _will be called_ because the callback is always called
- remove _To reliably detect write errors, add a listener for the `'error'` event_ because the `error` event will NOT be emitted if a write occurs after the stream has been closed
@github-actions github-actions bot added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Jun 7, 2021
@simoneb simoneb mentioned this pull request Jun 7, 2021
Closed
@aduh95 aduh95 added dont-land-on-v12.x author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jun 7, 2021
@aduh95
Copy link
Contributor

aduh95 commented Jun 7, 2021

If I understand the linked issue correctly, this is true only for Node.js v15+. Adding the labels so it doesn't get backported, feel free to remove them if I misunderstood the situation.

@benjamingr
Copy link
Member

benjamingr commented Jun 7, 2021

@nodejs/streams

mcollina
mcollina approved these changes Jun 8, 2021
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mmarchini mmarchini added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 10, 2021
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 10, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jun 10, 2021

Commit Queue failed
- Loading data for nodejs/node/pull/38959
✔  Done loading data for nodejs/node/pull/38959
----------------------------------- PR info ------------------------------------
Title      doc: update write callback documentation (#38959)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     simoneb:patch-1 -> nodejs:master
Labels     author ready, doc, dont-land-on-v12.x, dont-land-on-v14.x, stream
Commits    1
 - doc: update write callback documentation
Committers 1
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/38959
Fixes: https://github.com/nodejs/node/issues/38704
Reviewed-By: James M Snell 
Reviewed-By: Antoine du Hamel 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Robert Nagy 
Reviewed-By: Matteo Collina 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/38959
Fixes: https://github.com/nodejs/node/issues/38704
Reviewed-By: James M Snell 
Reviewed-By: Antoine du Hamel 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Robert Nagy 
Reviewed-By: Matteo Collina 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 07 Jun 2021 14:02:22 GMT
   ✔  Approvals: 5
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/38959#pullrequestreview-677503321
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/38959#pullrequestreview-677663819
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/38959#pullrequestreview-677841411
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/38959#pullrequestreview-677850823
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/38959#pullrequestreview-678297907
   ✖  Last GitHub CI failed
   ℹ  Green GitHub Actions CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/926267471

@github-actions github-actions bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jun 10, 2021
@mmarchini mmarchini added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 10, 2021
@github-actions github-actions bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 10, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jun 10, 2021

Commit Queue failed
- Loading data for nodejs/node/pull/38959
✔  Done loading data for nodejs/node/pull/38959
----------------------------------- PR info ------------------------------------
Title      doc: update write callback documentation (#38959)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     simoneb:patch-1 -> nodejs:master
Labels     author ready, doc, dont-land-on-v12.x, dont-land-on-v14.x, stream
Commits    1
 - doc: update write callback documentation
Committers 1
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/38959
Fixes: https://github.com/nodejs/node/issues/38704
Reviewed-By: James M Snell 
Reviewed-By: Antoine du Hamel 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Robert Nagy 
Reviewed-By: Matteo Collina 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/38959
Fixes: https://github.com/nodejs/node/issues/38704
Reviewed-By: James M Snell 
Reviewed-By: Antoine du Hamel 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Robert Nagy 
Reviewed-By: Matteo Collina 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 07 Jun 2021 14:02:22 GMT
   ✔  Approvals: 5
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/38959#pullrequestreview-677503321
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/38959#pullrequestreview-677663819
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/38959#pullrequestreview-677841411
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/38959#pullrequestreview-677850823
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/38959#pullrequestreview-678297907
   ✔  Last GitHub Actions successful
   ℹ  Green GitHub Actions CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/master up to date...
From https://github.com/nodejs/node
 * branch                  master     -> FETCH_HEAD
✔  origin/master is now up-to-date
- Downloading patch for 38959
From https://github.com/nodejs/node
 * branch                  refs/pull/38959/merge -> FETCH_HEAD
✔  Fetched commits as 52f8471b5281..453d7dbcbd99
--------------------------------------------------------------------------------
[master d77e235f62] doc: update write callback documentation
 Author: Simone Busoli 
 Date: Mon Jun 7 16:01:49 2021 +0200
 1 file changed, 2 insertions(+), 3 deletions(-)
   ✔  Patches applied
--------------------------------------------------------------------------------
--------------------------------- New Message ----------------------------------
doc: update write callback documentation

    

  • replace may or may not be called with will be called because the callback is always called
    • 

  • remove To reliably detect write errors, add a listener for the 'error' event because the error event will NOT be emitted if a write occurs after the stream has been closed
    • 



PR-URL: #38959

Fixes: #38704

Reviewed-By: James M Snell jasnell@gmail.com

Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com

Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com

Reviewed-By: Robert Nagy ronagy@icloud.com

Reviewed-By: Matteo Collina matteo.collina@gmail.com


[master 8806c955a2] doc: update write callback documentation

Author: Simone Busoli simone.busoli@nearform.com

Date: Mon Jun 7 16:01:49 2021 +0200

1 file changed, 2 insertions(+), 3 deletions(-)

✖  8806c955a270b602ab78330502cd8694e756ec8b

✔  5:7      Valid fixes URL.                          fixes-url

✔  0:0      blank line after title                    line-after-title

✖  1:72     Line should be <= 72 columns.             line-length

✖  2:72     Line should be <= 72 columns.             line-length

✔  0:0      metadata is at end of message             metadata-end

✔  4:8      PR-URL is valid.                          pr-url

✔  0:0      reviewers are valid                       reviewers

✔  0:0      valid subsystems                          subsystem

✔  0:0      Title is formatted correctly.             title-format

✔  0:0      Title is <= 50 columns.                   title-length

ℹ  Please fix the commit message and try again.
https://github.com/nodejs/node/actions/runs/926415209

mmarchini pushed a commit that referenced this pull request Jun 10, 2021
@simoneb @mmarchini
doc: update write callback documentation
cf609cc 
- replace _*may or may not* be called_ with _will be called_ because the
  callback is always called
- remove _To reliably detect write errors, add a listener for the
  `'error'` event_ because the `error` event will NOT be emitted if a
  write occurs after the stream has been closed

PR-URL: #38959
Fixes: #38704
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@mmarchini
Copy link
Contributor

mmarchini commented Jun 10, 2021

Landed in cf609cc

@mmarchini mmarchini closed this Jun 10, 2021
targos pushed a commit that referenced this pull request Jun 11, 2021
@simoneb @targos
doc: update write callback documentation
0bdeeda 
- replace _*may or may not* be called_ with _will be called_ because the
  callback is always called
- remove _To reliably detect write errors, add a listener for the
  `'error'` event_ because the `error` event will NOT be emitted if a
  write occurs after the stream has been closed

PR-URL: #38959
Fixes: #38704
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@danielleadams danielleadams mentioned this pull request Jun 14, 2021
Merged
danielleadams pushed a commit that referenced this pull request Jun 17, 2021
@simoneb @danielleadams
doc: update write callback documentation
c75b586 
- replace _*may or may not* be called_ with _will be called_ because the
  callback is always called
- remove _To reliably detect write errors, add a listener for the
  `'error'` event_ because the `error` event will NOT be emitted if a
  write occurs after the stream has been closed

PR-URL: #38959
Fixes: #38704
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@targos targos removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes

@jasnell jasnell jasnell approved these changes

@benjamingr benjamingr benjamingr approved these changes

@ronag ronag ronag approved these changes

@aduh95 aduh95 aduh95 approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

node 16.x does not raise error on write when server closes connection

8 participants

@simoneb @aduh95 @benjamingr @mmarchini @mcollina @jasnell @ronag @targos