From: Willy Tarreau <w@1wt.eu>
Date: Tue, 3 Dec 2013 21:48:23 +0000 (+0100)
Subject: BUG/MINOR: checks: don't consider errno and use conn->err_code
X-Git-Tag: v1.5-dev20~171
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ea292ae64946ecd75ac0aea4f0d2802ca14d4695;p=thirdparty%2Fhaproxy.git

BUG/MINOR: checks: don't consider errno and use conn->err_code

The last fix on checks (02b0f58: BUG/MEDIUM: checks: fix a long-standing
issue with reporting connection errors) tried to isolate error codes
retrieved from the socket in order to report appropriate messages. The
only thing is that we must not pre-initialize err to errno since we're
not in I/O context anymore and errno will be the one of the last syscall
(whatever it was). However we can complete the message with more info
from the transport layer (eg: SSL can inform us we were in a handshake).

Also add a catch-all case for CO_FL_ERROR when the connection was
established. No check currently seem to leave this case open, but better
catch it because it's hard to find all possible cases.

Error handling in checks is complex because some stuff must be done in
the central task (mandatory at least for timeouts) and other stuff is
done closer to the data.

Since checks have their own buffers now, we could move everything to
the main task and only keep the low-level I/O for sending/retrieving
data to/from this buffer. It would also avoid sending logs from the
I/O context!
---

diff --git a/src/checks.c b/src/checks.c
index 564eab4ab2..8b823bcc93 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -1467,31 +1467,58 @@ static struct task *process_chk(struct task *t)
 		 * which can happen on connect timeout or error.
 		 */
 		if (s->check.result == SRV_CHK_UNKNOWN) {
-			int skerr, err = errno;
+			int skerr, err;
 			socklen_t lskerr = sizeof(skerr);
+			const char *err_msg;
 
+			err = 0;
 			if (conn->ctrl) {
 				if (getsockopt(conn->t.sock.fd, SOL_SOCKET, SO_ERROR, &skerr, &lskerr) == 0 && skerr)
 					err = skerr;
+				/* EAGAIN is normal on a timeout */
+				if (err == EAGAIN)
+					err = 0;
 			}
 
-			/* eagain is normal on a timeout */
-			if (err == EAGAIN)
-				err = 0;
+			/* we'll try to build a meaningful error message
+			 * depending on the context of the error possibly
+			 * present in conn->err_type, and the socket error
+			 * possibly collected above.
+			 */
+			if (conn->err_code) {
+				if (err)
+					chunk_printf(&trash, "%s (%s)", conn_err_code_str(conn), strerror(err));
+				else
+					chunk_printf(&trash, "%s", conn_err_code_str(conn));
+				err_msg = trash.str;
+			}
+			else {
+				if (err) {
+					chunk_printf(&trash, "%s", strerror(err));
+					err_msg = trash.str;
+				}
+				else {
+					err_msg = NULL;
+				}
+			}
 
 			if ((conn->flags & (CO_FL_CONNECTED|CO_FL_WAIT_L4_CONN)) == CO_FL_WAIT_L4_CONN) {
 				/* L4 not established (yet) */
 				if (conn->flags & CO_FL_ERROR)
-					set_server_check_status(check, HCHK_STATUS_L4CON, err ? strerror(err) : NULL);
+					set_server_check_status(check, HCHK_STATUS_L4CON, err_msg);
 				else if (expired)
-					set_server_check_status(check, HCHK_STATUS_L4TOUT, NULL);
+					set_server_check_status(check, HCHK_STATUS_L4TOUT, err_msg);
 			}
 			else if ((conn->flags & (CO_FL_CONNECTED|CO_FL_WAIT_L6_CONN)) == CO_FL_WAIT_L6_CONN) {
 				/* L6 not established (yet) */
 				if (conn->flags & CO_FL_ERROR)
-					set_server_check_status(check, HCHK_STATUS_L6RSP, err ? strerror(err) : NULL);
+					set_server_check_status(check, HCHK_STATUS_L6RSP, err_msg);
 				else if (expired)
-					set_server_check_status(check, HCHK_STATUS_L6TOUT, NULL);
+					set_server_check_status(check, HCHK_STATUS_L6TOUT, err_msg);
+			}
+			else if (conn->flags & CO_FL_ERROR) {
+				/* I/O error after connection was established and before we could diagnose */
+				set_server_check_status(check, HCHK_STATUS_SOCKERR, err_msg);
 			}
 			else if (!check->type) {
 				/* good connection is enough for pure TCP check */