From cffaad250b4ff171b6aa72af32740573a85296ee Mon Sep 17 00:00:00 2001 From: Chloe Date: Fri, 31 Jul 2020 15:50:49 -0700 Subject: [PATCH] Revert "add error details for all server communication errors (#645)" This reverts commit c11125d752fdd5554608de170a3688dcd4ad544c. --- sql-odbc/src/odfesqlodbc/es_communication.cpp | 141 ++++++------------ sql-odbc/src/odfesqlodbc/es_communication.h | 3 - sql-odbc/src/odfesqlodbc/es_connection.cpp | 14 +- sql-odbc/src/odfesqlodbc/es_types.h | 6 - 4 files changed, 51 insertions(+), 113 deletions(-) diff --git a/sql-odbc/src/odfesqlodbc/es_communication.cpp b/sql-odbc/src/odfesqlodbc/es_communication.cpp index 8f895c6b09..b18af24f33 100644 --- a/sql-odbc/src/odfesqlodbc/es_communication.cpp +++ b/sql-odbc/src/odfesqlodbc/es_communication.cpp @@ -173,23 +173,6 @@ std::shared_ptr< ErrorDetails > ESCommunication::ParseErrorResponse( } } -void ESCommunication::SetErrorDetails(std::string reason, std::string message, - ConnErrorType error_type) { - // Prepare document and validate schema - auto error_details = std::make_shared< ErrorDetails >(); - error_details->reason = reason; - error_details->details = message; - error_details->source_type = "Dummy type"; - error_details->type = error_type; - m_error_details = error_details; -} - -void ESCommunication::SetErrorDetails(ErrorDetails details) { - // Prepare document and validate schema - auto error_details = std::make_shared< ErrorDetails >(details); - m_error_details = error_details; -} - void ESCommunication::GetJsonSchema(ESResult& es_result) { // Prepare document and validate schema try { @@ -232,15 +215,10 @@ ESCommunication::~ESCommunication() { std::string ESCommunication::GetErrorMessage() { // TODO #35 - Check if they expect NULL or "" when there is no error. - if (m_error_details) { - m_error_details->details = std::regex_replace( - m_error_details->details, std::regex("\\n"), "\\\\n"); - return ERROR_MSG_PREFIX + m_error_details->reason + ": " - + m_error_details->details; - } else { - return ERROR_MSG_PREFIX - + "No error details available; check the driver logs."; - } + m_error_details->details = std::regex_replace(m_error_details->details, + std::regex("\\n"), "\\\\n"); + return ERROR_MSG_PREFIX + m_error_details->reason + ": " + + m_error_details->details; } ConnErrorType ESCommunication::GetErrorType() { @@ -265,11 +243,9 @@ bool ESCommunication::ConnectDBStart() { LogMsg(ES_ALL, "Starting DB connection."); m_status = ConnStatusType::CONNECTION_BAD; if (!m_valid_connection_options) { - // TODO: get error message from CheckConnectionOptions + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Invalid connection options, unable to connect to DB."; - SetErrorDetails("Invalid connection options", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); DropDBConnection(); return false; @@ -277,9 +253,8 @@ bool ESCommunication::ConnectDBStart() { m_status = ConnStatusType::CONNECTION_NEEDED; if (!EstablishConnection()) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Failed to establish connection to DB."; - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); DropDBConnection(); return false; @@ -312,21 +287,18 @@ bool ESCommunication::CheckConnectionOptions() { if (m_rt_opts.auth.auth_type == AUTHTYPE_BASIC) { if (m_rt_opts.auth.username.empty() || m_rt_opts.auth.password.empty()) { + m_error_type = ConnErrorType::CONN_ERROR_INVALID_AUTH; m_error_message = AUTHTYPE_BASIC " authentication requires a username and password."; - SetErrorDetails("Auth error", m_error_message, - ConnErrorType::CONN_ERROR_INVALID_AUTH); } } else { + m_error_type = ConnErrorType::CONN_ERROR_INVALID_AUTH; m_error_message = "Unknown authentication type: '" + m_rt_opts.auth.auth_type + "'"; - SetErrorDetails("Auth error", m_error_message, - ConnErrorType::CONN_ERROR_INVALID_AUTH); } } else if (m_rt_opts.conn.server == "") { + m_error_type = ConnErrorType::CONN_ERROR_UNABLE_TO_ESTABLISH; m_error_message = "Host connection option was not specified."; - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_UNABLE_TO_ESTABLISH); } if (m_error_message != "") { @@ -430,18 +402,17 @@ bool ESCommunication::IsSQLPluginInstalled(const std::string& plugin_response) { if (!plugin_name.compare(OPENDISTRO_SQL_PLUGIN_NAME)) { std::string sql_plugin_version = it.at("version").as_string(); - LogMsg(ES_INFO, std::string("Found SQL plugin version '" - + sql_plugin_version + "'.") - .c_str()); + LogMsg(ES_ERROR, std::string("Found SQL plugin version '" + + sql_plugin_version + "'.") + .c_str()); return true; } } else { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Could not find all necessary fields in the plugin " "response object. " "(\"component\", \"version\")"; - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); throw std::runtime_error(m_error_message.c_str()); } } @@ -449,23 +420,18 @@ bool ESCommunication::IsSQLPluginInstalled(const std::string& plugin_response) { m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Error parsing endpoint response: " + std::string(e.what()); - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); } catch (const rabbit::parse_error& e) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Error parsing endpoint response: " + std::string(e.what()); - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); } catch (const std::exception& e) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Error parsing endpoint response: " + std::string(e.what()); - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); } catch (...) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Unknown exception thrown when parsing plugin endpoint response."; - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); } LogMsg(ES_ERROR, m_error_message.c_str()); @@ -486,35 +452,30 @@ bool ESCommunication::EstablishConnection() { IssueRequest(PLUGIN_ENDPOINT_FORMAT_JSON, Aws::Http::HttpMethod::HTTP_GET, "", "", ""); if (response == nullptr) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "The SQL plugin must be installed in order to use this driver. " "Received NULL response."; - SetErrorDetails("HTTP client error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); } else { AwsHttpResponseToString(response, m_response_str); if (response->GetResponseCode() != Aws::Http::HttpResponseCode::OK) { - if (response->HasClientError()) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; + m_error_message = + "The SQL plugin must be installed in order to use this driver."; + if (response->HasClientError()) m_error_message += " Client error: '" + response->GetClientErrorMessage() + "'."; - SetErrorDetails("HTTP client error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); - } - if (!m_response_str.empty()) { + if (!m_response_str.empty()) m_error_message += " Response error: '" + m_response_str + "'."; - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); - } } else { if (IsSQLPluginInstalled(m_response_str)) { return true; } else { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "The SQL plugin must be installed in order to use this " "driver. Response body: '" + m_response_str + "'"; - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); } } } @@ -544,11 +505,10 @@ std::vector< std::string > ESCommunication::GetColumnsWithSelectQuery( // Validate response if (response == nullptr) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Failed to receive response from query. " "Received NULL response."; - SetErrorDetails("HTTP client error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); return list_of_column; } @@ -571,8 +531,6 @@ std::vector< std::string > ESCommunication::GetColumnsWithSelectQuery( m_error_message += " Response error: '" + result->result_json + "'."; } - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); return list_of_column; } @@ -592,15 +550,13 @@ std::vector< std::string > ESCommunication::GetColumnsWithSelectQuery( int ESCommunication::ExecDirect(const char* query, const char* fetch_size_) { m_error_details.reset(); if (!query) { + m_error_type = ConnErrorType::CONN_ERROR_INVALID_NULL_PTR; m_error_message = "Query is NULL"; - SetErrorDetails("Execution error", m_error_message, - ConnErrorType::CONN_ERROR_INVALID_NULL_PTR); LogMsg(ES_ERROR, m_error_message.c_str()); return -1; } else if (!m_http_client) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Unable to connect. Please try connecting again."; - SetErrorDetails("Execution error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); return -1; } @@ -618,11 +574,10 @@ int ESCommunication::ExecDirect(const char* query, const char* fetch_size_) { // Validate response if (response == nullptr) { + m_error_type = ConnErrorType::CONN_ERROR_QUERY_SYNTAX; m_error_message = "Failed to receive response from query. " "Received NULL response."; - SetErrorDetails("Execution error", m_error_message, - ConnErrorType::CONN_ERROR_QUERY_SYNTAX); LogMsg(ES_ERROR, m_error_message.c_str()); return -1; } @@ -654,13 +609,12 @@ int ESCommunication::ExecDirect(const char* query, const char* fetch_size_) { try { ConstructESResult(*result); } catch (std::runtime_error& e) { + m_error_type = ConnErrorType::CONN_ERROR_QUERY_SYNTAX; m_error_message = "Received runtime exception: " + std::string(e.what()); if (!result->result_json.empty()) { m_error_message += " Result body: " + result->result_json; } - SetErrorDetails("Execution error", m_error_message, - ConnErrorType::CONN_ERROR_QUERY_SYNTAX); LogMsg(ES_ERROR, m_error_message.c_str()); return -1; } @@ -695,11 +649,10 @@ void ESCommunication::SendCursorQueries(std::string cursor) { SQL_ENDPOINT_FORMAT_JDBC, Aws::Http::HttpMethod::HTTP_POST, ctype, "", "", cursor); if (response == nullptr) { + m_error_type = ConnErrorType::CONN_ERROR_QUERY_SYNTAX; m_error_message = "Failed to receive response from cursor. " "Received NULL response."; - SetErrorDetails("Cursor error", m_error_message, - ConnErrorType::CONN_ERROR_QUERY_SYNTAX); LogMsg(ES_ERROR, m_error_message.c_str()); return; } @@ -725,10 +678,9 @@ void ESCommunication::SendCursorQueries(std::string cursor) { result.release(); } } catch (std::runtime_error& e) { + m_error_type = ConnErrorType::CONN_ERROR_QUERY_SYNTAX; m_error_message = "Received runtime exception: " + std::string(e.what()); - SetErrorDetails("Cursor error", m_error_message, - ConnErrorType::CONN_ERROR_QUERY_SYNTAX); LogMsg(ES_ERROR, m_error_message.c_str()); } @@ -744,11 +696,10 @@ void ESCommunication::SendCloseCursorRequest(const std::string& cursor) { IssueRequest(SQL_ENDPOINT_CLOSE_CURSOR, Aws::Http::HttpMethod::HTTP_POST, ctype, "", "", cursor); if (response == nullptr) { + m_error_type = ConnErrorType::CONN_ERROR_QUERY_SYNTAX; m_error_message = - "Failed to receive response from cursor close request. " + "Failed to receive response from cursor. " "Received NULL response."; - SetErrorDetails("Cursor error", m_error_message, - ConnErrorType::CONN_ERROR_QUERY_SYNTAX); LogMsg(ES_ERROR, m_error_message.c_str()); } } @@ -831,11 +782,10 @@ std::string ESCommunication::GetServerVersion() { std::shared_ptr< Aws::Http::HttpResponse > response = IssueRequest("", Aws::Http::HttpMethod::HTTP_GET, "", "", ""); if (response == nullptr) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = - "Failed to receive response from server version query. " + "Failed to receive response from query. " "Received NULL response."; - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); return ""; } @@ -851,22 +801,19 @@ std::string ESCommunication::GetServerVersion() { } } catch (const rabbit::type_mismatch& e) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Error parsing main endpoint response: " + std::string(e.what()); - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); } catch (const rabbit::parse_error& e) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Error parsing main endpoint response: " + std::string(e.what()); - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); } catch (const std::exception& e) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Error parsing main endpoint response: " + std::string(e.what()); - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); } catch (...) { LogMsg(ES_ERROR, @@ -887,11 +834,10 @@ std::string ESCommunication::GetClusterName() { std::shared_ptr< Aws::Http::HttpResponse > response = IssueRequest("", Aws::Http::HttpMethod::HTTP_GET, "", "", ""); if (response == nullptr) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = - "Failed to receive response from cluster name query. " + "Failed to receive response from query. " "Received NULL response."; - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); return ""; } @@ -907,22 +853,19 @@ std::string ESCommunication::GetClusterName() { } } catch (const rabbit::type_mismatch& e) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Error parsing main endpoint response: " + std::string(e.what()); - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); } catch (const rabbit::parse_error& e) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Error parsing main endpoint response: " + std::string(e.what()); - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); } catch (const std::exception& e) { + m_error_type = ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE; m_error_message = "Error parsing main endpoint response: " + std::string(e.what()); - SetErrorDetails("Connection error", m_error_message, - ConnErrorType::CONN_ERROR_COMM_LINK_FAILURE); LogMsg(ES_ERROR, m_error_message.c_str()); } catch (...) { LogMsg(ES_ERROR, diff --git a/sql-odbc/src/odfesqlodbc/es_communication.h b/sql-odbc/src/odfesqlodbc/es_communication.h index f910d0aec7..ca8f623199 100644 --- a/sql-odbc/src/odfesqlodbc/es_communication.h +++ b/sql-odbc/src/odfesqlodbc/es_communication.h @@ -87,9 +87,6 @@ class ESCommunication { void GetJsonSchema(ESResult& es_result); void PrepareCursorResult(ESResult& es_result); std::shared_ptr< ErrorDetails > ParseErrorResponse(ESResult& es_result); - void SetErrorDetails(std::string reason, std::string message, - ConnErrorType error_type); - void SetErrorDetails(ErrorDetails details); // TODO #35 - Go through and add error messages on exit conditions std::string m_error_message; diff --git a/sql-odbc/src/odfesqlodbc/es_connection.cpp b/sql-odbc/src/odfesqlodbc/es_connection.cpp index fb13d3131f..5699fb0a10 100644 --- a/sql-odbc/src/odfesqlodbc/es_connection.cpp +++ b/sql-odbc/src/odfesqlodbc/es_connection.cpp @@ -125,11 +125,15 @@ int LIBES_connect(ConnectionClass *self) { std::string msg = GetErrorMsg(esconn); char error_message_out[ERROR_BUFF_SIZE] = ""; if (!msg.empty()) - SPRINTF_FIXED(error_message_out, "Connection error: %s", - msg.c_str()); + SPRINTF_FIXED( + error_message_out, + "elasticsearch connection status was not CONNECTION_OK: %s", + msg.c_str()); else STRCPY_FIXED(error_message_out, - "Connection error: No message available."); + "elasticsearch connection status was not " + "CONNECTION_OK. No error message " + "available."); CC_set_error(self, CONN_OPENDB_ERROR, error_message_out, "LIBES_connect"); ESDisconnect(esconn); @@ -147,8 +151,8 @@ int LIBES_connect(ConnectionClass *self) { return 1; } -// TODO #36 - When we fix encoding, we should look into returning a code here. -// This is called in connection.c and the return code isn't checked +// TODO #36 - When we fix encoding, we should look into returning a code here. This +// is called in connection.c and the return code isn't checked void CC_set_locale_encoding(ConnectionClass *self, const char *encoding) { if (self == NULL) return; diff --git a/sql-odbc/src/odfesqlodbc/es_types.h b/sql-odbc/src/odfesqlodbc/es_types.h index ab133ac9c8..dcb73398c2 100644 --- a/sql-odbc/src/odfesqlodbc/es_types.h +++ b/sql-odbc/src/odfesqlodbc/es_types.h @@ -299,12 +299,6 @@ typedef struct ErrorDetails { std::string details; std::string source_type; ConnErrorType type; - ErrorDetails() { - reason = ""; - details = ""; - source_type = ""; - type = ConnErrorType::CONN_ERROR_SUCCESS; - } } ErrorDetails; #define INVALID_OID 0