From b286b9328158ad7686b7787d54c857e973c5b74c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= Date: Sun, 6 Apr 2014 20:31:02 +0200 Subject: Give more feedback for YggdrasilTask network errors. --- logic/auth/MojangAccount.cpp | 2 +- logic/auth/YggdrasilTask.cpp | 110 ++++++++++++++++++++++------------ logic/auth/YggdrasilTask.h | 25 ++++++-- logic/auth/flows/AuthenticateTask.cpp | 36 +++++------ logic/auth/flows/AuthenticateTask.h | 8 +-- logic/auth/flows/RefreshTask.cpp | 32 ++++------ logic/auth/flows/RefreshTask.h | 8 +-- logic/auth/flows/ValidateTask.cpp | 15 +++-- logic/auth/flows/ValidateTask.h | 8 +-- 9 files changed, 139 insertions(+), 105 deletions(-) (limited to 'logic') 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 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("SSL Handshake failed.
There might be a few causes for it:
" "")); 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(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); + 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: }; -- cgit v1.2.3