From e057c751cdee68164e75b8a4347659965ce00312 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Thu, 12 Dec 2019 21:12:11 +0200
Subject: [PATCH] Improve handleError error messages

The hangup and error handlers now have unique messages. Although the
behavior in the handlers is practically the same in both cases, the cause
of the error is not the same.

If a socket error is present, it is added to the error message. If an
error is present, it should clearly show the reason why the TCP socket was
closed.

The is_fake_event boolean helps distinguish fake events from real
ones. This makes figuring out the real source of hangup events easier.
---
 include/maxscale/dcb.h                        |  3 ++-
 server/core/dcb.cc                            |  5 ++++
 .../MySQL/mariadbbackend/mysql_backend.cc     | 26 ++++++++++++++++---
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/include/maxscale/dcb.h b/include/maxscale/dcb.h
index 21933b795..411ab8bb3 100644
--- a/include/maxscale/dcb.h
+++ b/include/maxscale/dcb.h
@@ -229,7 +229,8 @@ typedef struct dcb
     uint32_t n_close;           /** How many times dcb_close has been called. */
     char*    path;              /** If a Unix socket, the path it was bound to. */
 
-    uint64_t m_uid; /**< Unique identifier for this DCB */
+    uint64_t m_uid;     /**< Unique identifier for this DCB */
+    bool     is_fake_event;
 } DCB;
 
 /**
diff --git a/server/core/dcb.cc b/server/core/dcb.cc
index 456b081cf..d6828cd3f 100644
--- a/server/core/dcb.cc
+++ b/server/core/dcb.cc
@@ -204,6 +204,7 @@ DCB* dcb_alloc(dcb_role_t role, SERV_LISTENER* listener)
     newdcb->low_water = config_writeq_low_water();
     newdcb->high_water = config_writeq_high_water();
     newdcb->m_uid = this_unit.uid_generator.fetch_add(1, std::memory_order_relaxed);
+    newdcb->is_fake_event = false;
 
     if (role == DCB_ROLE_SERVICE_LISTENER)
     {
@@ -2062,7 +2063,9 @@ static void dcb_hangup_foreach_worker(MXB_WORKER* worker, struct server* server)
         {
             this_thread.current_dcb = dcb;
             dcb->flags |= DCBF_HUNG;
+            dcb->is_fake_event = true;
             dcb->func.hangup(dcb);
+            dcb->is_fake_event = false;
         }
     }
 
@@ -3249,7 +3252,9 @@ static uint32_t dcb_handler(DCB* dcb, uint32_t events)
         events = dcb->fake_event;
         dcb->fake_event = 0;
 
+        dcb->is_fake_event = true;
         rv |= dcb_process_poll_events(dcb, events);
+        dcb->is_fake_event = false;
     }
 
     this_thread.current_dcb = NULL;
diff --git a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc
index d00eef6b7..ef904c4f4 100644
--- a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc
+++ b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc
@@ -25,6 +25,8 @@
 #include <maxscale/server.hh>
 #include <maxscale/utils.h>
 
+#include <sstream>
+
 /*
  * MySQL Protocol module for handling the protocol between the gateway
  * and the backend MySQL database.
@@ -585,6 +587,23 @@ static int gw_read_backend_event(DCB* dcb)
     return rc;
 }
 
+static std::string get_detailed_error(DCB* dcb)
+{
+    std::ostringstream ss;
+
+    if (int err = gw_getsockerrno(dcb->fd))
+    {
+        ss << " (" << err << ", " << mxs_strerror(err) << ")";
+    }
+    else if (dcb->is_fake_event)
+    {
+        // Fake events should not have TCP socket errors
+        ss << " (Generated event)";
+    }
+
+    return ss.str();
+}
+
 static void do_handle_error(DCB* dcb, mxs_error_action_t action, const char* errmsg)
 {
     bool succp = true;
@@ -592,7 +611,7 @@ static void do_handle_error(DCB* dcb, mxs_error_action_t action, const char* err
 
     if (!dcb->dcb_errhandle_called)
     {
-        GWBUF* errbuf = mysql_create_custom_error(1, 0, errmsg);
+        GWBUF* errbuf = mysql_create_custom_error(1, 0, (errmsg + get_detailed_error(dcb)).c_str());
         MXS_ROUTER_SESSION* rsession = static_cast<MXS_ROUTER_SESSION*>(session->router_session);
         MXS_ROUTER_OBJECT* router = session->service->router;
         MXS_ROUTER* router_instance = session->service->router_instance;
@@ -1340,7 +1359,7 @@ static int gw_error_backend_event(DCB* dcb)
     }
     else
     {
-        do_handle_error(dcb, ERRACT_NEW_CONNECTION, "Lost connection to backend server.");
+        do_handle_error(dcb, ERRACT_NEW_CONNECTION, "Lost connection to backend server: network error");
     }
 
     return 1;
@@ -1381,7 +1400,8 @@ static int gw_backend_hangup(DCB* dcb)
     }
     else
     {
-        do_handle_error(dcb, ERRACT_NEW_CONNECTION, "Lost connection to backend server.");
+        do_handle_error(dcb, ERRACT_NEW_CONNECTION,
+                        "Lost connection to backend server: connection closed by peer");
     }
 
     return 1;