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.

Sign up for GitHub

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

Jump to bottom

net: throw error to object mode in Socket #40344

Closed
wants to merge 5 commits into from
Closed

net: throw error to object mode in Socket #40344

wants to merge 5 commits into from

Conversation

watilde
Copy link
Member

@watilde watilde commented Oct 6, 2021

Fixes: #40336

@watilde watilde added the net Issues and PRs related to the net subsystem. label Oct 6, 2021
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 6, 2021
@watilde watilde requested a review from lpinca October 6, 2021 10:13
lpinca
lpinca reviewed Oct 6, 2021
View reviewed changes
lib/net.js Outdated Show resolved Hide resolved
@watilde watilde force-pushed the fixes-net-arg branch 2 times, most recently from aa726dd to 3964bc3 Compare October 7, 2021 06:42
@watilde watilde changed the title net: throw error to given objectMode in connection net: throw error to object mode in Socket Oct 7, 2021
lpinca
lpinca reviewed Oct 7, 2021
View reviewed changes
lib/net.js Outdated Show resolved Hide resolved
lpinca
lpinca reviewed Oct 7, 2021
View reviewed changes
lib/net.js Outdated Show resolved Hide resolved
lpinca
lpinca reviewed Oct 7, 2021
View reviewed changes
lib/net.js Outdated Show resolved Hide resolved
@watilde watilde added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 8, 2021
@Ayase-252 Ayase-252 added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 8, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 8, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Oct 9, 2021
Open
21 tasks
@Mesteery
Copy link
Contributor

@watilde The PR didn't land because the CI was running. I think you could remove the commit-queue label and put it back.

pr 40344 skipped, CI still running

https://github.com/nodejs/node/runs/3863311768?check_suite_focus=true#step:8:64

@watilde watilde added commit-queue Add this label to land a pull request using GitHub Actions. request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 12, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 12, 2021
@github-actions github-actions bot mentioned this pull request Oct 13, 2021
Open
22 tasks
@watilde watilde added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 13, 2021
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 13, 2021
@github-actions
Copy link
Contributor

Landed in c350c21...f570109

@github-actions github-actions bot closed this Oct 13, 2021
nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2021
@watilde @nodejs-github-bot
net: throw error to object mode in Socket
4644662 
Fixes: #40336

PR-URL: #40344
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2021
@watilde @nodejs-github-bot
net: check objectMode first and then readble || writable
c7cd8ef 
Co-authored-by: Luigi Pinca <[email protected]>

PR-URL: #40344
Fixes: #40336
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2021
@watilde @nodejs-github-bot
net: remove unused ObjectKeys
119558b 
Co-authored-by: Luigi Pinca <[email protected]>

PR-URL: #40344
Fixes: #40336
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2021
@watilde @nodejs-github-bot
test: replace common port with specific number
e0faf8c 
PR-URL: #40344
Fixes: #40336
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Oct 13, 2021
@watilde @nodejs-github-bot
net: check if option is undefined
f570109 
PR-URL: #40344
Fixes: #40336
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
@watilde watilde deleted the fixes-net-arg branch October 13, 2021 01:57
@targos targos mentioned this pull request Oct 13, 2021
Closed
targos pushed a commit that referenced this pull request Oct 13, 2021
@watilde @targos
net: throw error to object mode in Socket
a672be5 
Fixes: #40336

PR-URL: #40344
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
targos pushed a commit that referenced this pull request Oct 13, 2021
@watilde @targos
net: check objectMode first and then readble || writable
dbb2e6f 
Co-authored-by: Luigi Pinca <[email protected]>

PR-URL: #40344
Fixes: #40336
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
targos pushed a commit that referenced this pull request Oct 13, 2021
@watilde @targos
net: remove unused ObjectKeys
4564a93 
Co-authored-by: Luigi Pinca <[email protected]>

PR-URL: #40344
Fixes: #40336
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
targos pushed a commit that referenced this pull request Oct 13, 2021
@watilde @targos
test: replace common port with specific number
1e15137 
PR-URL: #40344
Fixes: #40336
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
targos pushed a commit that referenced this pull request Oct 13, 2021
@watilde @targos
net: check if option is undefined
bc86084 
PR-URL: #40344
Fixes: #40336
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
@github-actions github-actions bot mentioned this pull request Oct 14, 2021
Open
22 tasks
@BethGriggs BethGriggs mentioned this pull request Oct 14, 2021
Merged
2 tasks
This was referenced Oct 15, 2021
Open
Open
Open
Open
@richardlau richardlau mentioned this pull request Oct 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lxxyx Lxxyx Lxxyx approved these changes

@JungMinu JungMinu JungMinu approved these changes

@ronag ronag ronag approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell 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. net Issues and PRs related to the net subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Setting the objectMode option in the net.Socket() constructor causes an abort on write
9 participants
@watilde @nodejs-github-bot @Mesteery @jasnell @lpinca @ronag @JungMinu @Lxxyx @Ayase-252