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

PipeWrap::Connect() should handle errors from uv_pipe_connect2() #50652

Closed
bnoordhuis opened this issue Nov 10, 2023 · 0 comments · Fixed by #50657
Closed

PipeWrap::Connect() should handle errors from uv_pipe_connect2() #50652

bnoordhuis opened this issue Nov 10, 2023 · 0 comments · Fixed by #50657
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.

Comments

@bnoordhuis
Copy link
Member

553169f switched from uv_pipe_connect() to uv_pipe_connect2(). The former doesn't return errors (it can't because void) but the latter can and does.

The comment here is no longer applicable:

node/src/pipe_wrap.cc

Line 237 in 89c66ae

args.GetReturnValue().Set(0); // uv_pipe_connect() doesn't return errors.

@bnoordhuis bnoordhuis added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 10, 2023
deokjinkim added a commit to deokjinkim/node that referenced this issue Nov 10, 2023
@deokjinkim
src: handle errors from uv_pipe_connect2()
76acb04 
We need to handle errors from uv_pipe_connect2()
because return type is `int`.

Fixes: nodejs#50652
Refs: nodejs#49667
Refs: libuv/libuv#4030
deokjinkim added a commit to deokjinkim/node that referenced this issue Nov 10, 2023
@deokjinkim
src: handle errors from uv_pipe_connect2()
68ce7f9 
We need to handle errors from uv_pipe_connect2()
because return type is `int`.

Fixes: nodejs#50652
Refs: nodejs#49667
Refs: libuv/libuv#4030
@deokjinkim deokjinkim mentioned this issue Nov 10, 2023
Merged
nodejs-github-bot pushed a commit that referenced this issue Nov 12, 2023
@deokjinkim
src: handle errors from uv_pipe_connect2()
9134f4a 
We need to handle errors from uv_pipe_connect2()
because return type is `int`.

Fixes: #50652
Refs: #49667
Refs: libuv/libuv#4030
PR-URL: #50657
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
targos pushed a commit that referenced this issue Nov 23, 2023
@deokjinkim @targos
src: handle errors from uv_pipe_connect2()
57587de 
We need to handle errors from uv_pipe_connect2()
because return type is `int`.

Fixes: #50652
Refs: #49667
Refs: libuv/libuv#4030
PR-URL: #50657
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
UlisesGascon pushed a commit that referenced this issue Dec 11, 2023
@deokjinkim @UlisesGascon
src: handle errors from uv_pipe_connect2()
c253e39 
We need to handle errors from uv_pipe_connect2()
because return type is `int`.

Fixes: #50652
Refs: #49667
Refs: libuv/libuv#4030
PR-URL: #50657
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: theanarkh <theratliter@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

src: handle errors from uv_pipe_connect2() deokjinkim/node
1 participant
@bnoordhuis