Error messages when server closes connection #12437
Conversation
A server can close a connection while we're expecting a response. For example, the server may close the connection due to a keep-alive timeout. This results in a 502 response to the client. Log an error in this case, so that the administrator has a chance to adjust the keep-alive timeout on the ATS side to avoid this situation.
bneradt
left a comment
bneradt
left a comment
There was a problem hiding this comment.
It would be good to have @bryancall look at this when he is back from PTO. I think we were considering moving away from error.log and transitioning content to diags.log.
That said, if that is not the case, I agree that these connection-level messages thematically fit error.log. If we're keeping error.log.
bneradt
left a comment
bneradt
left a comment
There was a problem hiding this comment.
Looks like we're OK with this from our PR/issue meeting. But let's include the origin URL.
bneradt
left a comment
bneradt
left a comment
There was a problem hiding this comment.
Looks good. Thank you for adding the URL.
| switch (event) { | ||
| case VC_EVENT_EOS: | ||
| log_server_close_with_origin(t_state, "Server closed connection while reading PUSH response header."); | ||
| _ua.get_entry()->eos = true; | ||
| // Fall through | ||
|
|
||
| case VC_EVENT_READ_READY: | ||
| case VC_EVENT_READ_COMPLETE: | ||
| // More data to parse | ||
| break; | ||
|
|
||
| case VC_EVENT_ERROR: | ||
| case VC_EVENT_INACTIVITY_TIMEOUT: | ||
| case VC_EVENT_ACTIVE_TIMEOUT: | ||
| // The user agent is hosed. Send an error | ||
| set_ua_abort(HttpTransact::ABORTED, event); | ||
| call_transact_and_set_next_state(HttpTransact::HandleBadPushRespHdr); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
The new message says "Server closed...", but the code around these events seem to indicate client events while waiting for the response. Maybe "Client closed..." is appropriate? Or maybe I'm misunderstanding.
There was a problem hiding this comment.
You're right. This isn't a server close as I thought. I'm going to remove this message and only keep the one where the server closes.
src/proxy/http/HttpSM.cc
Outdated
|
|
||
| switch (event) { | ||
| case VC_EVENT_EOS: | ||
| log_server_close_with_origin(t_state, "Server closed connection while reading response header"); |
There was a problem hiding this comment.
Minor: maybe also end this in a period to be consistent with the other log.
2 participants
A server can close a connection while we're expecting a response. For example, the server may close the connection due to a keep-alive timeout. This results in a 502 response to the client. Log an error in this case, so that the administrator has a chance to adjust the keep-alive timeout on the ATS side to avoid this situation.