Give more feedback for YggdrasilTask network errors.

9 files changed, 139 insertions, 105 deletions

diff --git a/logic/auth/MojangAccount.cpp b/logic/auth/MojangAccount.cpp
index 6c937ef1..31a238ba 100644
--- a/logic/auth/MojangAccount.cpp
+++ b/logic/auth/MojangAccount.cpp
@@ -220,7 +220,7 @@ void MojangAccount::authFailed(QString reason)
 	auto session = m_currentTask->getAssignedSession();
 	// This is emitted when the yggdrasil tasks time out or are cancelled.
 	// -> we treat the error as no-op
-	if (reason == "Yggdrasil task cancelled.")
+	if (m_currentTask->state() == YggdrasilTask::STATE_FAILED_SOFT)
 	{
 		if (session)
 		{
diff --git a/logic/auth/YggdrasilTask.cpp b/logic/auth/YggdrasilTask.cpp
index 7679b11f..ab6ac5e4 100644
--- a/logic/auth/YggdrasilTask.cpp
+++ b/logic/auth/YggdrasilTask.cpp
@@ -29,11 +29,12 @@
 YggdrasilTask::YggdrasilTask(MojangAccount *account, QObject *parent)
 	: Task(parent), m_account(account)
 {
+	changeState(STATE_CREATED);
 }
 
 void YggdrasilTask::executeTask()
 {
-	setStatus(getStateMessage(STATE_SENDING_REQUEST));
+	changeState(STATE_SENDING_REQUEST);
 
 	// Get the content of the request we're going to send to the server.
 	QJsonDocument doc(getRequestContent());
@@ -73,12 +74,16 @@ void YggdrasilTask::heartbeat()
 void YggdrasilTask::abort()
 {
 	progress(timeout_max, timeout_max);
+	// TODO: actually use this in a meaningful way
+	m_aborted = YggdrasilTask::BY_USER;
 	m_netReply->abort();
 }
 
 void YggdrasilTask::abortByTimeout()
 {
 	progress(timeout_max, timeout_max);
+	// TODO: actually use this in a meaningful way
+	m_aborted = YggdrasilTask::BY_TIMEOUT;
 	m_netReply->abort();
 }
 
@@ -96,11 +101,21 @@ void YggdrasilTask::sslErrors(QList<QSslError> errors)
 
 void YggdrasilTask::processReply()
 {
-	setStatus(getStateMessage(STATE_PROCESSING_RESPONSE));
+	changeState(STATE_PROCESSING_RESPONSE);
 
-	if (m_netReply->error() == QNetworkReply::SslHandshakeFailedError)
+	switch (m_netReply->error())
 	{
-		emitFailed(
+	case QNetworkReply::NoError:
+		break;
+	case QNetworkReply::TimeoutError:
+		changeState(STATE_FAILED_SOFT, tr("Authentication operation timed out."));
+		return;
+	case QNetworkReply::OperationCanceledError:
+		changeState(STATE_FAILED_SOFT, tr("Authentication operation cancelled."));
+		return;
+	case QNetworkReply::SslHandshakeFailedError:
+		changeState(
+			STATE_FAILED_SOFT,
 			tr("<b>SSL Handshake failed.</b><br/>There might be a few causes for it:<br/>"
 			   "<ul>"
 			   "<li>You use Windows XP and need to <a "
@@ -111,16 +126,13 @@ void YggdrasilTask::processReply()
 			   "<li>Possibly something else. Check the MultiMC log file for details</li>"
 			   "</ul>"));
 		return;
-	}
-
-	// any network errors lead to offline mode right now
-	if (m_netReply->error() >= QNetworkReply::ConnectionRefusedError &&
-		m_netReply->error() <= QNetworkReply::UnknownNetworkError)
-	{
-		// WARNING/FIXME: the value here is used in MojangAccount to detect the cancel/timeout
-		emitFailed("Yggdrasil task cancelled.");
-		QLOG_ERROR() << "Yggdrasil task cancelled because of: " << m_netReply->error() << " : "
-					 << m_netReply->errorString();
+	// used for invalid credentials and similar errors. Fall through.
+	case QNetworkReply::ContentOperationNotPermittedError:
+		break;
+	default:
+		changeState(STATE_FAILED_SOFT,
+					tr("Authentication operation failed due to a network error: %1 (%2)")
+						.arg(m_netReply->errorString()).arg(m_netReply->error()));
 		return;
 	}
 
@@ -140,22 +152,16 @@ void YggdrasilTask::processReply()
 		// pass an empty json object to the processResponse function.
 		if (jsonError.error == QJsonParseError::NoError || replyData.size() == 0)
 		{
-			if (processResponse(replyData.size() > 0 ? doc.object() : QJsonObject()))
-			{
-				emitSucceeded();
-				return;
-			}
-
-			// errors happened anyway?
-			emitFailed(m_error ? m_error->m_errorMessageVerbose
-							   : tr("An unknown error occurred when processing the response "
-									"from the authentication server."));
+			processResponse(replyData.size() > 0 ? doc.object() : QJsonObject());
+			return;
 		}
 		else
 		{
-			emitFailed(tr("Failed to parse Yggdrasil JSON response: %1 at offset %2.")
-						   .arg(jsonError.errorString())
-						   .arg(jsonError.offset));
+			changeState(STATE_FAILED_SOFT, tr("Failed to parse authentication server response "
+											  "JSON response: %1 at offset %2.")
+											   .arg(jsonError.errorString())
+											   .arg(jsonError.offset));
+			QLOG_ERROR() << replyData;
 		}
 		return;
 	}
@@ -171,20 +177,21 @@ void YggdrasilTask::processReply()
 		// stuff there.
 		QLOG_DEBUG() << "The request failed, but the server gave us an error message. "
 						"Processing error.";
-		emitFailed(processError(doc.object()));
+		processError(doc.object());
 	}
 	else
 	{
 		// The server didn't say anything regarding the error. Give the user an unknown
 		// error.
-		QLOG_DEBUG() << "The request failed and the server gave no error message. "
-						"Unknown error.";
-		emitFailed(tr("An unknown error occurred when trying to communicate with the "
-					  "authentication server: %1").arg(m_netReply->errorString()));
+		QLOG_DEBUG()
+			<< "The request failed and the server gave no error message. Unknown error.";
+		changeState(STATE_FAILED_SOFT,
+					tr("An unknown error occurred when trying to communicate with the "
+					   "authentication server: %1").arg(m_netReply->errorString()));
 	}
 }
 
-QString YggdrasilTask::processError(QJsonObject responseData)
+void YggdrasilTask::processError(QJsonObject responseData)
 {
 	QJsonValue errorVal = responseData.value("error");
 	QJsonValue errorMessageValue = responseData.value("errorMessage");
@@ -194,24 +201,51 @@ QString YggdrasilTask::processError(QJsonObject responseData)
 	{
 		m_error = std::shared_ptr<Error>(new Error{
 			errorVal.toString(""), errorMessageValue.toString(""), causeVal.toString("")});
-		return m_error->m_errorMessageVerbose;
+		changeState(STATE_FAILED_HARD, m_error->m_errorMessageVerbose);
 	}
 	else
 	{
 		// Error is not in standard format. Don't set m_error and return unknown error.
-		return tr("An unknown Yggdrasil error occurred.");
+		changeState(STATE_FAILED_HARD, tr("An unknown Yggdrasil error occurred."));
 	}
 }
 
-QString YggdrasilTask::getStateMessage(const YggdrasilTask::State state) const
+QString YggdrasilTask::getStateMessage() const
 {
-	switch (state)
+	switch (m_state)
 	{
+	case STATE_CREATED:
+		return "Waiting...";
 	case STATE_SENDING_REQUEST:
 		return tr("Sending request to auth servers...");
 	case STATE_PROCESSING_RESPONSE:
 		return tr("Processing response from servers...");
+	case STATE_SUCCEEDED:
+		return tr("Authentication task succeeded.");
+	case STATE_FAILED_SOFT:
+		return tr("Failed to contact the authentication server.");
+	case STATE_FAILED_HARD:
+		return tr("Failed to authenticate.");
 	default:
-		return tr("Processing. Please wait...");
+		return tr("...");
+	}
+}
+
+void YggdrasilTask::changeState(YggdrasilTask::State newState, QString reason)
+{
+	m_state = newState;
+	setStatus(getStateMessage());
+	if (newState == STATE_SUCCEEDED)
+	{
+		emitSucceeded();
+	}
+	else if (newState == STATE_FAILED_HARD || newState == STATE_FAILED_SOFT)
+	{
+		emitFailed(reason);
 	}
 }
+
+YggdrasilTask::State YggdrasilTask::state()
+{
+	return m_state;
+}
diff --git a/logic/auth/YggdrasilTask.h b/logic/auth/YggdrasilTask.h
index b24f909f..ed70b957 100644
--- a/logic/auth/YggdrasilTask.h
+++ b/logic/auth/YggdrasilTask.h
@@ -60,17 +60,28 @@ public:
 		QString m_cause;
 	};
 
-protected:
+	enum AbortedBy
+	{
+		BY_NOTHING,
+		BY_USER,
+		BY_TIMEOUT
+	} m_aborted = BY_NOTHING;
+
 	/**
 	 * Enum for describing the state of the current task.
 	 * Used by the getStateMessage function to determine what the status message should be.
 	 */
 	enum State
 	{
+		STATE_CREATED,
 		STATE_SENDING_REQUEST,
 		STATE_PROCESSING_RESPONSE,
-		STATE_OTHER,
-	};
+		STATE_FAILED_SOFT, //!< soft failure. this generally means the user auth details haven't been invalidated
+		STATE_FAILED_HARD, //!< hard failure. auth is invalid
+		STATE_SUCCEEDED
+	} m_state = STATE_CREATED;
+
+protected:
 
 	virtual void executeTask();
 
@@ -94,21 +105,21 @@ protected:
 	 * Note: If the response from the server was blank, and the HTTP code was 200, this function is called with
 	 * an empty QJsonObject.
 	 */
-	virtual bool processResponse(QJsonObject responseData) = 0;
+	virtual void processResponse(QJsonObject responseData) = 0;
 
 	/**
 	 * Processes an error response received from the server.
 	 * The default implementation will read data from Yggdrasil's standard error response format and set it as this task's Error.
 	 * \returns a QString error message that will be passed to emitFailed.
 	 */
-	virtual QString processError(QJsonObject responseData);
+	virtual void processError(QJsonObject responseData);
 
 	/**
 	 * Returns the state message for the given state.
 	 * Used to set the status message for the task.
 	 * Should be overridden by subclasses that want to change messages for a given state.
 	 */
-	virtual QString getStateMessage(const State state) const;
+	virtual QString getStateMessage() const;
 
 protected
 slots:
@@ -117,10 +128,12 @@ slots:
 	void heartbeat();
 	void sslErrors(QList<QSslError>);
 
+	void changeState(State newState, QString reason=QString());
 public
 slots:
 	virtual void abort() override;
 	void abortByTimeout();
+	State state();
 protected:
 	// FIXME: segfault disaster waiting to happen
 	MojangAccount *m_account = nullptr;
diff --git a/logic/auth/flows/AuthenticateTask.cpp b/logic/auth/flows/AuthenticateTask.cpp
index 6548c4e9..340235e3 100644
--- a/logic/auth/flows/AuthenticateTask.cpp
+++ b/logic/auth/flows/AuthenticateTask.cpp
@@ -71,7 +71,7 @@ QJsonObject AuthenticateTask::getRequestContent() const
 	return req;
 }
 
-bool AuthenticateTask::processResponse(QJsonObject responseData)
+void AuthenticateTask::processResponse(QJsonObject responseData)
 {
 	// Read the response data. We need to get the client token, access token, and the selected
 	// profile.
@@ -84,16 +84,13 @@ bool AuthenticateTask::processResponse(QJsonObject responseData)
 	if (clientToken.isEmpty())
 	{
 		// Fail if the server gave us an empty client token
-		// TODO: Set an error properly to display to the user.
-		QLOG_ERROR() << "Server didn't send a client token.";
-		return false;
+		changeState(STATE_FAILED_HARD, tr("Authentication server didn't send a client token."));
+		return;
 	}
 	if (!m_account->m_clientToken.isEmpty() && clientToken != m_account->m_clientToken)
 	{
-		// The server changed our client token! Obey its wishes, but complain. That's what I do
-		// for my parents, so...
-		QLOG_WARN() << "Server changed our client token to '" << clientToken
-					<< "'. This shouldn't happen, but it isn't really a big deal.";
+		changeState(STATE_FAILED_HARD, tr("Authentication server attempted to change the client token. This isn't supported."));
+		return;
 	}
 	// Set the client token.
 	m_account->m_clientToken = clientToken;
@@ -104,8 +101,8 @@ bool AuthenticateTask::processResponse(QJsonObject responseData)
 	if (accessToken.isEmpty())
 	{
 		// Fail if the server didn't give us an access token.
-		// TODO: Set an error properly to display to the user.
-		QLOG_ERROR() << "Server didn't send an access token.";
+		changeState(STATE_FAILED_HARD, tr("Authentication server didn't send an access token."));
+		return;
 	}
 	// Set the access token.
 	m_account->m_accessToken = accessToken;
@@ -149,16 +146,13 @@ bool AuthenticateTask::processResponse(QJsonObject responseData)
 	QString currentProfileId = currentProfile.value("id").toString("");
 	if (currentProfileId.isEmpty())
 	{
-		// TODO: Set an error to display to the user.
-		QLOG_ERROR() << "Server didn't specify a currently selected profile.";
-		return false;
+		changeState(STATE_FAILED_HARD, tr("Authentication server didn't specify a currently selected profile. The account exists, but likely isn't premium."));
+		return;
 	}
 	if (!m_account->setCurrentProfile(currentProfileId))
 	{
-		// TODO: Set an error to display to the user.
-		QLOG_ERROR() << "Server specified a selected profile that wasn't in the available "
-						"profiles list.";
-		return false;
+		changeState(STATE_FAILED_HARD, tr("Authentication server specified a selected profile that wasn't in the available profiles list."));
+		return;
 	}
 
 	// this is what the vanilla launcher passes to the userProperties launch param
@@ -181,7 +175,7 @@ bool AuthenticateTask::processResponse(QJsonObject responseData)
 	// We've made it through the minefield of possible errors. Return true to indicate that
 	// we've succeeded.
 	QLOG_DEBUG() << "Finished reading authentication response.";
-	return true;
+	changeState(STATE_SUCCEEDED);
 }
 
 QString AuthenticateTask::getEndpoint() const
@@ -189,15 +183,15 @@ QString AuthenticateTask::getEndpoint() const
 	return "authenticate";
 }
 
-QString AuthenticateTask::getStateMessage(const YggdrasilTask::State state) const
+QString AuthenticateTask::getStateMessage() const
 {
-	switch (state)
+	switch (m_state)
 	{
 	case STATE_SENDING_REQUEST:
 		return tr("Authenticating: Sending request...");
 	case STATE_PROCESSING_RESPONSE:
 		return tr("Authenticating: Processing response...");
 	default:
-		return YggdrasilTask::getStateMessage(state);
+		return YggdrasilTask::getStateMessage();
 	}
 }
diff --git a/logic/auth/flows/AuthenticateTask.h b/logic/auth/flows/AuthenticateTask.h
index b6564657..13a000aa 100644
--- a/logic/auth/flows/AuthenticateTask.h
+++ b/logic/auth/flows/AuthenticateTask.h
@@ -33,13 +33,13 @@ public:
 	AuthenticateTask(MojangAccount *account, const QString &password, QObject *parent = 0);
 
 protected:
-	virtual QJsonObject getRequestContent() const;
+	virtual QJsonObject getRequestContent() const override;
 
-	virtual QString getEndpoint() const;
+	virtual QString getEndpoint() const override;
 
-	virtual bool processResponse(QJsonObject responseData);
+	virtual void processResponse(QJsonObject responseData) override;
 
-	QString getStateMessage(const YggdrasilTask::State state) const;
+	virtual QString getStateMessage() const override;
 
 private:
 	QString m_password;
diff --git a/logic/auth/flows/RefreshTask.cpp b/logic/auth/flows/RefreshTask.cpp
index 5a55ed91..7e926c2b 100644
--- a/logic/auth/flows/RefreshTask.cpp
+++ b/logic/auth/flows/RefreshTask.cpp
@@ -60,7 +60,7 @@ QJsonObject RefreshTask::getRequestContent() const
 	return req;
 }
 
-bool RefreshTask::processResponse(QJsonObject responseData)
+void RefreshTask::processResponse(QJsonObject responseData)
 {
 	// Read the response data. We need to get the client token, access token, and the selected
 	// profile.
@@ -73,17 +73,13 @@ bool RefreshTask::processResponse(QJsonObject responseData)
 	if (clientToken.isEmpty())
 	{
 		// Fail if the server gave us an empty client token
-		// TODO: Set an error properly to display to the user.
-		QLOG_ERROR() << "Server didn't send a client token.";
-		return false;
+		changeState(STATE_FAILED_HARD, tr("Authentication server didn't send a client token."));
+		return;
 	}
 	if (!m_account->m_clientToken.isEmpty() && clientToken != m_account->m_clientToken)
 	{
-		// The server changed our client token! Obey its wishes, but complain. That's what I do
-		// for my parents, so...
-		QLOG_ERROR() << "Server changed our client token to '" << clientToken
-					 << "'. This shouldn't happen, but it isn't really a big deal.";
-		return false;
+		changeState(STATE_FAILED_HARD, tr("Authentication server attempted to change the client token. This isn't supported."));
+		return;
 	}
 
 	// Now, we set the access token.
@@ -92,9 +88,8 @@ bool RefreshTask::processResponse(QJsonObject responseData)
 	if (accessToken.isEmpty())
 	{
 		// Fail if the server didn't give us an access token.
-		// TODO: Set an error properly to display to the user.
-		QLOG_ERROR() << "Server didn't send an access token.";
-		return false;
+		changeState(STATE_FAILED_HARD, tr("Authentication server didn't send an access token."));
+		return;
 	}
 
 	// we validate that the server responded right. (our current profile = returned current
@@ -103,9 +98,8 @@ bool RefreshTask::processResponse(QJsonObject responseData)
 	QString currentProfileId = currentProfile.value("id").toString("");
 	if (m_account->currentProfile()->id != currentProfileId)
 	{
-		// TODO: Set an error to display to the user.
-		QLOG_ERROR() << "Server didn't specify the same selected profile as ours.";
-		return false;
+		changeState(STATE_FAILED_HARD, tr("Authentication server didn't specify the same prefile as expected."));
+		return;
 	}
 
 	// this is what the vanilla launcher passes to the userProperties launch param
@@ -130,7 +124,7 @@ bool RefreshTask::processResponse(QJsonObject responseData)
 	QLOG_DEBUG() << "Finished reading refresh response.";
 	// Reset the access token.
 	m_account->m_accessToken = accessToken;
-	return true;
+	changeState(STATE_SUCCEEDED);
 }
 
 QString RefreshTask::getEndpoint() const
@@ -138,15 +132,15 @@ QString RefreshTask::getEndpoint() const
 	return "refresh";
 }
 
-QString RefreshTask::getStateMessage(const YggdrasilTask::State state) const
+QString RefreshTask::getStateMessage() const
 {
-	switch (state)
+	switch (m_state)
 	{
 	case STATE_SENDING_REQUEST:
 		return tr("Refreshing login token...");
 	case STATE_PROCESSING_RESPONSE:
 		return tr("Refreshing login token: Processing response...");
 	default:
-		return YggdrasilTask::getStateMessage(state);
+		return YggdrasilTask::getStateMessage();
 	}
 }
diff --git a/logic/auth/flows/RefreshTask.h b/logic/auth/flows/RefreshTask.h
index 0dadc025..ad07ba2d 100644
--- a/logic/auth/flows/RefreshTask.h
+++ b/logic/auth/flows/RefreshTask.h
@@ -33,12 +33,12 @@ public:
 	RefreshTask(MojangAccount * account);
 
 protected:
-	virtual QJsonObject getRequestContent() const;
+	virtual QJsonObject getRequestContent() const override;
 
-	virtual QString getEndpoint() const;
+	virtual QString getEndpoint() const override;
 
-	virtual bool processResponse(QJsonObject responseData);
+	virtual void processResponse(QJsonObject responseData) override;
 
-	QString getStateMessage(const YggdrasilTask::State state) const;
+	virtual QString getStateMessage() const override;
 };
 
diff --git a/logic/auth/flows/ValidateTask.cpp b/logic/auth/flows/ValidateTask.cpp
index 4f7323fd..f3fc1e71 100644
--- a/logic/auth/flows/ValidateTask.cpp
+++ b/logic/auth/flows/ValidateTask.cpp
@@ -38,11 +38,10 @@ QJsonObject ValidateTask::getRequestContent() const
 	return req;
 }
 
-bool ValidateTask::processResponse(QJsonObject responseData)
+void ValidateTask::processResponse(QJsonObject responseData)
 {
 	// Assume that if processError wasn't called, then the request was successful.
-	emitSucceeded();
-	return true;
+	changeState(YggdrasilTask::STATE_SUCCEEDED);
 }
 
 QString ValidateTask::getEndpoint() const
@@ -50,15 +49,15 @@ QString ValidateTask::getEndpoint() const
 	return "validate";
 }
 
-QString ValidateTask::getStateMessage(const YggdrasilTask::State state) const
+QString ValidateTask::getStateMessage() const
 {
-	switch (state)
+	switch (m_state)
 	{
-	case STATE_SENDING_REQUEST:
+	case YggdrasilTask::STATE_SENDING_REQUEST:
 		return tr("Validating access token: Sending request...");
-	case STATE_PROCESSING_RESPONSE:
+	case YggdrasilTask::STATE_PROCESSING_RESPONSE:
 		return tr("Validating access token: Processing response...");
 	default:
-		return YggdrasilTask::getStateMessage(state);
+		return YggdrasilTask::getStateMessage();
 	}
 }
diff --git a/logic/auth/flows/ValidateTask.h b/logic/auth/flows/ValidateTask.h
index 0e34f0c3..7bc2fe01 100644
--- a/logic/auth/flows/ValidateTask.h
+++ b/logic/auth/flows/ValidateTask.h
@@ -35,13 +35,13 @@ public:
 	ValidateTask(MojangAccount *account, QObject *parent = 0);
 
 protected:
-	virtual QJsonObject getRequestContent() const;
+	virtual QJsonObject getRequestContent() const override;
 
-	virtual QString getEndpoint() const;
+	virtual QString getEndpoint() const override;
 
-	virtual bool processResponse(QJsonObject responseData);
+	virtual void processResponse(QJsonObject responseData) override;
 
-	QString getStateMessage(const YggdrasilTask::State state) const;
+	virtual QString getStateMessage() const override;
 
 private:
 };