-
Notifications
You must be signed in to change notification settings - Fork 2.6k
More robust error handling #2262
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.
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
base: master
Are you sure you want to change the base?
Conversation
- Check if socket is alive before waiting to write to it to prevent SIGPIPE - Consume SIGPIPE on client - Read response line and headers after bad request
|
I'll see if I can do a little more due dilligence before marking for review. Also
Should be
|
httplib.h
Outdated
| client_cert_path_(client_cert_path), client_key_path_(client_key_path) {} | ||
| client_cert_path_(client_cert_path), client_key_path_(client_key_path) { | ||
| #ifndef _WIN32 | ||
| signal(SIGPIPE, SIG_IGN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A C/C++ library shall not call signal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd been wondering why eg the README has this note:
If your program needs to avoid being terminated on SIGPIPE, the only fully general way might be to set up a signal handler for SIGPIPE to handle or ignore it yourself.
While the server code consumes SIGPIPE.. :
Lines 7322 to 7328 in 4b2b851
| inline Server::Server() | |
| : new_task_queue( | |
| [] { return new ThreadPool(CPPHTTPLIB_THREAD_POOL_COUNT); }) { | |
| #ifndef _WIN32 | |
| signal(SIGPIPE, SIG_IGN); | |
| #endif | |
| } |
In any case, I'd concede that the other two changes probably make the client-side signal unnecessary for what I was trying to do. Been meaning to play around a little more to better understand what I'm doing. Will revert particular change in the meantime.
Crossing some t's and dotting some i's following #2260, I decided to try this suggestion from Copilot:
I found that the client SIGPIPEs. And even after handling these, the response object doesn't get populated when the socket is closed mid-transfer (ie for sufficiently large transfers).
So I made the following changes: