Added loads of error handling to the new server list download code. Added several new error messages.

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.<br>Please go to <a href=\"http://www.pokerth.net/\" target=\"_blank\">http://www.pokerth.net</a> and download the latest version."),

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<DownloadHelper> 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<DownloadHelper> m_downloadHelper;
 };
 
 // State: Reading the server list.

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.reset(helper);
-	m_downloadHelper = 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.reset(helper);
-	m_downloadHelper = 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<boost::iostreams::input> 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())
+
-			{
+			if (!addrNode || !addrNode->ToElement() || !portNode || !portNode->ToElement())
-				int tmpPort = 0;
+				throw ClientException(__FILE__, __LINE__, ERR_SOCK_INVALID_SERVERLIST_XML, 0);
-				portNode->ToElement()->QueryIntAttribute("value", &tmpPort);
+
-				context.SetServerPort((unsigned)tmpPort);
+			context.SetServerAddr(addrNode->ToElement()->Attribute("value"));
-				retVal = MSG_SOCK_SERVER_LIST_DONE;
+
-			}
+			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;

src/net/common/downloadhelper.cpp

@@ -19,6 +19,8 @@
 
 #include <net/socket_helper.h>
 #include <net/downloadhelper.h>
+#include <net/netexception.h>
+#include <net/socket_msg.h>
 #include <curl/curl.h>
 
 #include <cstdio>
@@ -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();
+	if (!m_data->targetFile)
-	m_data->curlMultiHandle = curl_multi_init();
+		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);
+		CURLMsg *tmpMsg;
-		CURLcode code = tmpMsg->data.result;
+		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;

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