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

HttpConnector should not error if a socket option fails #2359

Closed
seanmonstar opened this issue Dec 14, 2020 · 2 comments · Fixed by #2361
Closed

HttpConnector should not error if a socket option fails #2359

seanmonstar opened this issue Dec 14, 2020 · 2 comments · Fixed by #2361
Labels
A-client Area: client. E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@seanmonstar
Copy link
Member

An error trying to set a socket option should not error the entire connect attempt. Instead, they should just be logged (perhaps at warn level).
@seanmonstar seanmonstar added A-client Area: client. E-easy Effort: easy. A task that would be a great starting point for a new contributor. labels Dec 14, 2020
@seanmonstar seanmonstar mentioned this issue Dec 14, 2020
Merged
@aeryz
Copy link
Contributor

aeryz commented Dec 16, 2020

I am interested in this but I have a question. Should we just log all socket option failures and continue to connect? Are there any fatal ones?

@seanmonstar
Copy link
Member Author

What do you propose? I think if the option is required to function, then of course error. But most aren't.

@aeryz aeryz mentioned this issue Dec 16, 2020
Merged
seanmonstar pushed a commit that referenced this issue Dec 23, 2020
@aeryz
fix(client): log socket option errors instead of returning error (#2361)
dad5c87 
Closes #2359
@davepacheco davepacheco mentioned this issue Jan 4, 2021
Merged
@CfirTsabari CfirTsabari mentioned this issue Feb 19, 2021
Open
@dependabot dependabot bot mentioned this issue Mar 16, 2021
Closed
BenxiangGe pushed a commit to BenxiangGe/hyper that referenced this issue Jul 26, 2021
@aeryz
fix(client): log socket option errors instead of returning error (hyp…
7226b70 
…erium#2361)

Closes hyperium#2359
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-client Area: client. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Milestone
No milestone
2 participants
@seanmonstar @aeryz