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.

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

CA-401075: remove misleading logs from HTTP client #6158

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

robhoes
Copy link
Member

@robhoes robhoes commented Dec 4, 2024

Recent changes to Http_client were made to log errors and backtraces in case of unusual errors, in order to better diagnose problems. Unfortunately, this also led to benign exceptions being logged, which caused people to suspect problems where the exception was actually "expected". In particular the following error started appearing in the logs many times:

Raised Http_client.Parse_error("Expected initial header []")

The case here is that the client has sent an HTTP request, but the server disconnects before sending a response.

It turns out that this happens commonly when an external connection is received by xapi and handed over to another process, such as xcp-rrdd. The connection's file descriptor is passed to the other process, and the original HTTP request is forwarded over a local socket. The other process continues to handle the request and sends responses to the forwarded socket, but never to the local socket, where xapi is waiting for the response.

This is a quick change that makes the caller of the receive function aware that no response was sent, without logging scary errors, to fix the immediate problem. In addition, we should enhance the handover process to actually send responses, although funcionally this is not an issue.

Recent changes to Http_client were made to log errors and backtraces in
case of unusual errors, in order to better diagnose problems.
Unfortunately, this also led to benign exceptions being logged, which
caused people to suspect problems where the exception was actually
"expected". In particular the following error started appearing in the
logs many times:

    Raised Http_client.Parse_error("Expected initial header []")

The case here is that the client has sent an HTTP request, but the
server disconnects before sending a response.

It turns out that this happens commonly when an external connection is
received by xapi and handed over to another process, such as xcp-rrdd.
The connection's file descriptor is passed to the other process, and the
original HTTP request is forwarded over a local socket. The other process
continues to handle the request and sends responses to the forwarded
socket, but never to the local socket, where xapi is waiting for the
response.

This is a quick change that makes the caller of the receive function
aware that no response was sent, without logging scary errors, to fix
the immediate problem. In addition, we should enhance the handover
process to actually send responses, although funcionally this is not an
issue.

Signed-off-by: Rob Hoes <[email protected]>
@@ -119,6 +119,8 @@ let response_of_fd_exn_slow fd =
; additional_headers= !headers
; body= None
}
| [] ->
raise End_of_file
Copy link
Contributor

@edwintorok edwintorok Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be raise_notrace if we only use this for control-flow and don't want to log a stacktrace here. (raise_notrace is slightly faster because it doesn't capture a backtrace, whereas raise does (even if we don't query it log it))

@robhoes robhoes added this pull request to the merge queue Dec 4, 2024
Merged via the queue into xapi-project:master with commit 9998658 Dec 4, 2024
15 checks passed
@robhoes robhoes deleted the ca401075 branch December 4, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants