From 6d7beb17968a540ed12bcd273cb228b208ed6cc7 Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Mon, 19 Jun 2023 11:55:32 -0500 Subject: [PATCH] [Database] Fix multi-statement error reporting (#3425) --- common/database/database_update.cpp | 2 +- common/dbcore.cpp | 54 ++++++++++++++++++++++++----- common/mysql_request_result.cpp | 21 +++++++++++ common/mysql_request_result.h | 18 ++++++++-- 4 files changed, 83 insertions(+), 12 deletions(-) diff --git a/common/database/database_update.cpp b/common/database/database_update.cpp index 8fa5524b9..08f1e51b9 100644 --- a/common/database/database_update.cpp +++ b/common/database/database_update.cpp @@ -183,7 +183,7 @@ bool DatabaseUpdate::UpdateManifest( // ignore empty query result "errors" if (r.ErrorNumber() != 1065 && !r.ErrorMessage().empty()) { - LogError("[{}]", r.ErrorMessage()); + LogError("(#{}) [{}]", r.ErrorNumber(), r.ErrorMessage()); errored_migration = true; LogInfo("Required database update failed. This could be a problem"); diff --git a/common/dbcore.cpp b/common/dbcore.cpp index 8358e123c..1db87afac 100644 --- a/common/dbcore.cpp +++ b/common/dbcore.cpp @@ -323,8 +323,11 @@ MySQLRequestResult DBcore::QueryDatabaseMulti(const std::string &query) if (pStatus != Connected) { Open(); } + auto r = MySQLRequestResult{}; int status = mysql_real_query(mysql, query.c_str(), query.length()); + + // process single result if (status != 0) { unsigned int error_number = mysql_errno(mysql); @@ -337,11 +340,35 @@ MySQLRequestResult DBcore::QueryDatabaseMulti(const std::string &query) std::string error_raw = fmt::format("{}", mysql_error(mysql)); std::string mysql_err = Strings::Trim(error_raw); std::string clean_query = Strings::Replace(query, "\n", ""); - LogMySQLQuery("[{}] ({}) query [{}]", mysql_err, mysql_errno(mysql), clean_query); + LogMySQLError("[{}] ({}) query [{}]", mysql_err, mysql_errno(mysql), clean_query); + + MYSQL_RES *res = mysql_store_result(mysql); + + uint32 row_count = 0; + if (res) { + row_count = (uint32) mysql_num_rows(res); + } + + r = MySQLRequestResult( + res, + (uint32) mysql_affected_rows(mysql), + row_count, + (uint32) mysql_field_count(mysql), + (uint32) mysql_insert_id(mysql) + ); + + std::string error_message = mysql_error(mysql); + r.SetErrorMessage(error_message); + r.SetErrorNumber(mysql_errno(mysql)); + + if (res) { + mysql_free_result(res); + } + + return r; } } - auto result = MySQLRequestResult{}; int index = 0; @@ -355,7 +382,7 @@ MySQLRequestResult DBcore::QueryDatabaseMulti(const std::string &query) uint32 row_count = 0; MYSQL_RES *res = mysql_store_result(mysql); - result = MySQLRequestResult( + r = MySQLRequestResult( res, (uint32) mysql_affected_rows(mysql), row_count, @@ -368,15 +395,14 @@ MySQLRequestResult DBcore::QueryDatabaseMulti(const std::string &query) LogMySQLQuery( "{} -- ({} row{} affected) ({}s)", piece, - result.RowsAffected(), - result.RowsAffected() == 1 ? "" : "s", + r.RowsAffected(), + r.RowsAffected() == 1 ? "" : "s", std::to_string(timer.elapsed()) ); } if (res) { row_count = (uint32) mysql_num_rows(res); - mysql_free_result(res); } // more results? -1 = no, >0 = error, 0 = yes (keep looping) @@ -385,13 +411,25 @@ MySQLRequestResult DBcore::QueryDatabaseMulti(const std::string &query) LogMySQLError("[{}] [{}]", mysql_errno(mysql), mysql_error(mysql)); } + mysql_free_result(res); + + // error logging + std::string error_message = mysql_error(mysql); + r.SetErrorMessage(error_message); + r.SetErrorNumber(mysql_errno(mysql)); + // we handle errors elsewhere - return result; + return r; } + + if (res) { + mysql_free_result(res); + } + index++; } while (status == 0); SetMultiStatementsOff(); - return result; + return r; } diff --git a/common/mysql_request_result.cpp b/common/mysql_request_result.cpp index e7f147b51..fbe8fc758 100644 --- a/common/mysql_request_result.cpp +++ b/common/mysql_request_result.cpp @@ -51,6 +51,7 @@ void MySQLRequestResult::ZeroOut() m_RowCount = 0; m_RowsAffected = 0; m_LastInsertedID = 0; + m_error_message = ""; } MySQLRequestResult::~MySQLRequestResult() @@ -137,3 +138,23 @@ MySQLRequestResult& MySQLRequestResult::operator=(MySQLRequestResult&& other) other.ZeroOut(); return *this; } + +uint32 MySQLRequestResult::GetErrorNumber() const +{ + return m_ErrorNumber; +} + +void MySQLRequestResult::SetErrorNumber(uint32 m_error_number) +{ + m_ErrorNumber = m_error_number; +} + +const std::string &MySQLRequestResult::GetErrorMessage() const +{ + return m_error_message; +} + +void MySQLRequestResult::SetErrorMessage(const std::string &m_error_message) +{ + MySQLRequestResult::m_error_message = m_error_message; +} diff --git a/common/mysql_request_result.h b/common/mysql_request_result.h index ec9b3e57f..cb2a4e719 100644 --- a/common/mysql_request_result.h +++ b/common/mysql_request_result.h @@ -33,30 +33,42 @@ private: uint32 m_LastInsertedID; uint32 m_ErrorNumber; + std::string m_error_message; public: MySQLRequestResult(MYSQL_RES* result, uint32 rowsAffected = 0, uint32 rowCount = 0, uint32 columnCount = 0, uint32 lastInsertedID = 0, uint32 errorNumber = 0, char *errorBuffer = nullptr); - MySQLRequestResult(); + MySQLRequestResult(); MySQLRequestResult(MySQLRequestResult&& moveItem); ~MySQLRequestResult(); MySQLRequestResult& operator=(MySQLRequestResult&& other); bool Success() const { return m_Success;} - std::string ErrorMessage() const {return m_ErrorBuffer ? std::string(m_ErrorBuffer) : std::string("");} + std::string ErrorMessage() const { + if (!m_error_message.empty()) { + return m_error_message; + } + + return m_ErrorBuffer ? std::string(m_ErrorBuffer) : std::string(""); + } uint32 ErrorNumber() const {return m_ErrorNumber;} uint32 RowsAffected() const {return m_RowsAffected;} uint32 RowCount() const {return m_RowCount;} uint32 ColumnCount() const {return m_ColumnCount;} uint32 LastInsertedID() const {return m_LastInsertedID;} // default to 0 index since we mostly use it that way anyways. - uint32 LengthOfColumn(int columnIndex = 0); + uint32 LengthOfColumn(int columnIndex = 0); const std::string FieldName(int columnIndex); MySQLRequestRow& begin() { return m_CurrentRow; } MySQLRequestRow& end() { return m_OneBeyondRow; } + uint32 GetErrorNumber() const; + void SetErrorNumber(uint32 m_error_number); + const std::string &GetErrorMessage() const; + void SetErrorMessage(const std::string &m_error_message); + private: void FreeInternals(); void ZeroOut();