From 17d68255b636cfadd7933ce62b9774e4c5b4616e Mon Sep 17 00:00:00 2001 From: lotodore Date: Sat, 10 May 2008 18:07:46 +0000 Subject: [PATCH] Added loads of error handling to the new server list download code. Added several new error messages. --- src/gui/qt/gametable/gametableimpl.cpp | 38 +++++++++++++ src/net/clientstate.h | 8 +-- src/net/common/clientstate.cpp | 77 +++++++++++--------------- src/net/common/downloadhelper.cpp | 57 ++++++++++++++++--- src/net/socket_msg.h | 12 +++- 5 files changed, 132 insertions(+), 60 deletions(-) diff --git a/src/gui/qt/gametable/gametableimpl.cpp b/src/gui/qt/gametable/gametableimpl.cpp index d0455a78..64290ba5 100755 --- a/src/gui/qt/gametable/gametableimpl.cpp +++ b/src/gui/qt/gametable/gametableimpl.cpp @@ -3148,6 +3148,44 @@ void gameTableImpl::networkError(int errorID, int /*osErrorID*/) { tr("Internal state error.\nPlease make sure that all players use the same version of PokerTH."), QMessageBox::Close); } break; + case ERR_SOCK_INVALID_SERVERLIST_URL: + case ERR_SOCK_DOWNLOAD_INVALID_URL: + { QMessageBox::warning(this, tr("Network Error"), + tr("Invalid server list URL.\nPlease correct the address in the settings."), + QMessageBox::Close); } + break; + case ERR_SOCK_OPEN_MD5_FAILED: + { QMessageBox::warning(this, tr("Network Error"), + tr("Could not open the server list MD5 file.\nPlease make sure that the server list URL is correct."), + QMessageBox::Close); } + break; + case ERR_SOCK_INVALID_SERVERLIST_MD5: + { QMessageBox::warning(this, tr("Network Error"), + tr("Synchronization of the PokerTH internet server list has failed.\nPlease make sure that the server list URL is correct."), + QMessageBox::Close); } + break; + case ERR_SOCK_INVALID_SERVERLIST_XML: + { QMessageBox::warning(this, tr("Network Error"), + tr("The PokerTH internet server list contains invalid data.\nIf you use a custom server list, please make sure its format is correct."), + QMessageBox::Close); } + break; + case ERR_SOCK_UNZIP_FAILED: + { QMessageBox::warning(this, tr("Network Error"), + tr("Could not unzip the PokerTH internet server list."), + QMessageBox::Close); } + break; + case ERR_SOCK_DOWNLOAD_INIT_FAILED: + case ERR_SOCK_DOWNLOAD_SELECT_FAILED: + case ERR_SOCK_DOWNLOAD_FAILED: + { QMessageBox::warning(this, tr("Network Error"), + tr("Could not download the PokerTH internet server list.\nPlease make sure you are directly connected to the internet."), + QMessageBox::Close); } + break; + case ERR_SOCK_DOWNLOAD_OPEN_FAILED: + { QMessageBox::warning(this, tr("Network Error"), + tr("Could not open the target file when downloading the server list."), + QMessageBox::Close); } + break; case ERR_NET_VERSION_NOT_SUPPORTED: { QMessageBox msgBox(QMessageBox::Warning, tr("Network Error"), tr("The PokerTH server does not support this version of the game.
Please go to http://www.pokerth.net and download the latest version."), diff --git a/src/net/clientstate.h b/src/net/clientstate.h index 9936c953..606dc13f 100644 --- a/src/net/clientstate.h +++ b/src/net/clientstate.h @@ -143,11 +143,9 @@ protected: // Protected constructor - this is a singleton. ClientStateSynchronizingServerList(); - void Cleanup(); - private: - DownloadHelper *m_downloadHelper; + std::auto_ptr m_downloadHelper; }; // State: Downloading the server list. @@ -169,11 +167,9 @@ protected: // Protected constructor - this is a singleton. ClientStateDownloadingServerList(); - void Cleanup(); - private: - DownloadHelper *m_downloadHelper; + std::auto_ptr m_downloadHelper; }; // State: Reading the server list. diff --git a/src/net/common/clientstate.cpp b/src/net/common/clientstate.cpp index ed62bcd8..200502a7 100644 --- a/src/net/common/clientstate.cpp +++ b/src/net/common/clientstate.cpp @@ -260,9 +260,9 @@ ClientStateStartServerListDownload::Process(ClientThread &client) string serverListUrl(context.GetServerListUrl()); // Retrieve the file name from the URL. size_t pos = serverListUrl.find_last_of('/'); - if (pos == string::npos || ++pos >= serverListUrl.length()) + if (serverListUrl.empty() || pos == string::npos || ++pos >= serverListUrl.length()) { - // TODO throw exception. + throw ClientException(__FILE__, __LINE__, ERR_SOCK_INVALID_SERVERLIST_URL, 0); } tmpServerListPath /= serverListUrl.substr(pos); if (exists(tmpServerListPath)) @@ -296,20 +296,17 @@ ClientStateSynchronizingServerList::Instance() } ClientStateSynchronizingServerList::ClientStateSynchronizingServerList() -: m_downloadHelper(NULL) { } ClientStateSynchronizingServerList::~ClientStateSynchronizingServerList() { - Cleanup(); } void ClientStateSynchronizingServerList::SetDownloadHelper(DownloadHelper *helper) { - Cleanup(); - m_downloadHelper = helper; + m_downloadHelper.reset(helper); } int @@ -319,25 +316,28 @@ ClientStateSynchronizingServerList::Process(ClientThread &client) if (m_downloadHelper->Process()) { - Cleanup(); + m_downloadHelper.reset(NULL); ClientContext &context = client.GetContext(); path md5ServerListPath(context.GetCacheDir()); // No more checking needed as this was done before. md5ServerListPath /= context.GetServerListUrl().substr(context.GetServerListUrl().find_last_of('/') + 1) + ".md5"; - path zippedServerListPath = change_extension(md5ServerListPath, ""); + path serverListPath = change_extension(md5ServerListPath, ""); // Compare the md5 sums. string tmpMd5; { ifstream inFile(md5ServerListPath.directory_string().c_str(), ios_base::in); + if (inFile.fail()) + throw ClientException(__FILE__, __LINE__, ERR_SOCK_OPEN_MD5_FAILED, 0); inFile >> tmpMd5; - // TODO error handling } MD5Buf downloadedMd5; - downloadedMd5.FromString(tmpMd5); + if (!downloadedMd5.FromString(tmpMd5)) + throw ClientException(__FILE__, __LINE__, ERR_SOCK_INVALID_SERVERLIST_MD5, 0); MD5Buf currentMd5; - CryptHelper::MD5Sum(zippedServerListPath.directory_string(), currentMd5); + if (!CryptHelper::MD5Sum(serverListPath.directory_string(), currentMd5)) + throw ClientException(__FILE__, __LINE__, ERR_SOCK_INVALID_SERVERLIST_MD5, 0); if (downloadedMd5 == currentMd5) { // Server list is still current. @@ -346,7 +346,7 @@ ClientStateSynchronizingServerList::Process(ClientThread &client) else { // Download new server list. - remove(zippedServerListPath); + remove(serverListPath); client.SetState(ClientStateStartServerListDownload::Instance()); } } @@ -354,14 +354,6 @@ ClientStateSynchronizingServerList::Process(ClientThread &client) return retVal; } - -void -ClientStateSynchronizingServerList::Cleanup() -{ - delete m_downloadHelper; - m_downloadHelper = NULL; -} - //----------------------------------------------------------------------------- ClientStateDownloadingServerList & @@ -372,20 +364,17 @@ ClientStateDownloadingServerList::Instance() } ClientStateDownloadingServerList::ClientStateDownloadingServerList() -: m_downloadHelper(NULL) { } ClientStateDownloadingServerList::~ClientStateDownloadingServerList() { - Cleanup(); } void ClientStateDownloadingServerList::SetDownloadHelper(DownloadHelper *helper) { - Cleanup(); - m_downloadHelper = helper; + m_downloadHelper.reset(helper); } int @@ -395,21 +384,13 @@ ClientStateDownloadingServerList::Process(ClientThread &client) if (m_downloadHelper->Process()) { - Cleanup(); + m_downloadHelper.reset(NULL); client.SetState(ClientStateReadingServerList::Instance()); } return retVal; } - -void -ClientStateDownloadingServerList::Cleanup() -{ - delete m_downloadHelper; - m_downloadHelper = NULL; -} - //----------------------------------------------------------------------------- ClientStateReadingServerList & @@ -441,13 +422,16 @@ ClientStateReadingServerList::Process(ClientThread &client) xmlServerListPath = change_extension(zippedServerListPath, ""); // Unzip the file using zlib. - { + try { ifstream inFile(zippedServerListPath.directory_string().c_str(), ios_base::in | ios_base::binary); ofstream outFile(xmlServerListPath.directory_string().c_str(), ios_base::out); boost::iostreams::filtering_streambuf in; in.push(boost::iostreams::zlib_decompressor()); in.push(inFile); boost::iostreams::copy(in, outFile); + } catch (...) + { + throw ClientException(__FILE__, __LINE__, ERR_SOCK_UNZIP_FAILED, 0); } } else @@ -467,19 +451,24 @@ ClientStateReadingServerList::Process(ClientThread &client) addrNode = firstServer->FirstChild("IPv6Address"); else addrNode = firstServer->FirstChild("IPv4Address"); - if (addrNode && addrNode->ToElement()) - context.SetServerAddr(addrNode->ToElement()->Attribute("value")); const TiXmlNode *portNode = firstServer->FirstChild("Port"); - if (portNode && portNode->ToElement()) - { - int tmpPort = 0; - portNode->ToElement()->QueryIntAttribute("value", &tmpPort); - context.SetServerPort((unsigned)tmpPort); - retVal = MSG_SOCK_SERVER_LIST_DONE; - } + + if (!addrNode || !addrNode->ToElement() || !portNode || !portNode->ToElement()) + throw ClientException(__FILE__, __LINE__, ERR_SOCK_INVALID_SERVERLIST_XML, 0); + + context.SetServerAddr(addrNode->ToElement()->Attribute("value")); + + int tmpPort = 0; + portNode->ToElement()->QueryIntAttribute("value", &tmpPort); + context.SetServerPort((unsigned)tmpPort); + retVal = MSG_SOCK_SERVER_LIST_DONE; } + else + throw ClientException(__FILE__, __LINE__, ERR_SOCK_INVALID_SERVERLIST_XML, 0); } - // TODO error handling + else + throw ClientException(__FILE__, __LINE__, ERR_SOCK_INVALID_SERVERLIST_XML, 0); + client.SetState(ClientStateStartResolve::Instance()); return retVal; diff --git a/src/net/common/downloadhelper.cpp b/src/net/common/downloadhelper.cpp index 56d3dc9d..ff65b537 100644 --- a/src/net/common/downloadhelper.cpp +++ b/src/net/common/downloadhelper.cpp @@ -19,6 +19,8 @@ #include #include +#include +#include #include #include @@ -50,17 +52,30 @@ DownloadHelper::~DownloadHelper() void DownloadHelper::Init(const string &url, const string &targetFileName) { + // Open target file for writing. m_data->targetFile = fopen(targetFileName.c_str(), "wb"); - m_data->curlHandle = curl_easy_init(); - m_data->curlMultiHandle = curl_multi_init(); + if (!m_data->targetFile) + throw NetException(__FILE__, __LINE__, ERR_SOCK_DOWNLOAD_OPEN_FAILED, 0); - // TODO throw exception on error + // Initialise curl. + m_data->curlHandle = curl_easy_init(); + if (!m_data->curlHandle) + throw NetException(__FILE__, __LINE__, ERR_SOCK_DOWNLOAD_INIT_FAILED, 0); + m_data->curlMultiHandle = curl_multi_init(); + if (!m_data->curlMultiHandle) + throw NetException(__FILE__, __LINE__, ERR_SOCK_DOWNLOAD_INIT_FAILED, 0); + + // Use a copy of the url string, because some curl versions require a copy. m_data->curlUrl = url; - curl_easy_setopt(m_data->curlHandle, CURLOPT_URL, m_data->curlUrl.c_str()); + if (curl_easy_setopt(m_data->curlHandle, CURLOPT_URL, m_data->curlUrl.c_str()) != CURLE_OK) + throw NetException(__FILE__, __LINE__, ERR_SOCK_DOWNLOAD_INVALID_URL, 0); + // Assume that the following calls never fail. curl_easy_setopt(m_data->curlHandle, CURLOPT_WRITEFUNCTION, NULL); curl_easy_setopt(m_data->curlHandle, CURLOPT_WRITEDATA, m_data->targetFile); - curl_multi_add_handle(m_data->curlMultiHandle, m_data->curlHandle); + // Use the multi interface for better abort handling. + if (curl_multi_add_handle(m_data->curlMultiHandle, m_data->curlHandle) != CURLM_OK) + throw NetException(__FILE__, __LINE__, ERR_SOCK_DOWNLOAD_INIT_FAILED, 0); } bool @@ -74,13 +89,16 @@ DownloadHelper::Process() curlResult = curl_multi_perform(m_data->curlMultiHandle, &runningHandles); } while (curlResult == CURLM_CALL_MULTI_PERFORM); + if (curlResult != CURLM_OK) + throw NetException(__FILE__, __LINE__, ERR_SOCK_DOWNLOAD_FAILED, 0); + if (runningHandles) { struct timeval timeout; fd_set readSet; fd_set writeSet; fd_set exceptSet; - int maxfd; + int maxfd = -1; FD_ZERO(&readSet); FD_ZERO(&writeSet); @@ -92,15 +110,36 @@ DownloadHelper::Process() curl_multi_fdset(m_data->curlMultiHandle, &readSet, &writeSet, &exceptSet, &maxfd); if (maxfd >= 0) + { int selectResult = select(maxfd+1, &readSet, &writeSet, &exceptSet, &timeout); - // TODO throw exception on error + if (!IS_VALID_SELECT(selectResult)) + throw NetException(__FILE__, __LINE__, ERR_SOCK_DOWNLOAD_SELECT_FAILED, SOCKET_ERRNO()); + } } else { + // Retrieve actual error code. int numMsgs; - CURLMsg *tmpMsg = curl_multi_info_read(m_data->curlMultiHandle, &numMsgs); - CURLcode code = tmpMsg->data.result; + CURLMsg *tmpMsg; + CURLcode code = CURLE_FAILED_INIT; + do { + tmpMsg = curl_multi_info_read(m_data->curlMultiHandle, &numMsgs); + if (tmpMsg) + code = tmpMsg->data.result; + } while (tmpMsg && tmpMsg->msg != CURLMSG_DONE); + + // Clean up the curl handles. Cleanup(); + + // Throw exception if an error occured. + if (code != CURLE_OK) + { + if (code == CURLE_URL_MALFORMAT) + throw NetException(__FILE__, __LINE__, ERR_SOCK_DOWNLOAD_INVALID_URL, 0); + else + throw NetException(__FILE__, __LINE__, ERR_SOCK_DOWNLOAD_FAILED, 0); + } + retVal = true; } return retVal; diff --git a/src/net/socket_msg.h b/src/net/socket_msg.h index a90fdbda..c8cadbdc 100644 --- a/src/net/socket_msg.h +++ b/src/net/socket_msg.h @@ -20,7 +20,7 @@ #ifndef _SOCKET_MSG_H_ #define _SOCKET_MSG_H_ -// Socket or socket related errors. +// Socket or connection related errors. #define ERR_SOCK_INTERNAL 1 #define ERR_SOCK_SERVERADDR_NOT_SET 2 #define ERR_SOCK_INVALID_PORT 3 @@ -41,6 +41,16 @@ #define ERR_SOCK_INVALID_PACKET 18 #define ERR_SOCK_INVALID_STATE 19 #define ERR_SOCK_INVALID_TYPE 20 +#define ERR_SOCK_INVALID_SERVERLIST_URL 21 +#define ERR_SOCK_OPEN_MD5_FAILED 22 +#define ERR_SOCK_INVALID_SERVERLIST_MD5 23 +#define ERR_SOCK_INVALID_SERVERLIST_XML 24 +#define ERR_SOCK_UNZIP_FAILED 25 +#define ERR_SOCK_DOWNLOAD_INIT_FAILED 26 +#define ERR_SOCK_DOWNLOAD_OPEN_FAILED 27 +#define ERR_SOCK_DOWNLOAD_INVALID_URL 28 +#define ERR_SOCK_DOWNLOAD_SELECT_FAILED 29 +#define ERR_SOCK_DOWNLOAD_FAILED 30 // The following errors are game errors. #define ERR_NET_VERSION_NOT_SUPPORTED 101 #define ERR_NET_SERVER_MAINTENANCE 102