Merge lp:~mzanetti/reminders-app/improve-error-handling into lp:reminders-app

[Michael Zanetti]

improve some error handling

* Rate limit exceeded
* Upload quota exceeded
* Authentication expired

To test/reproduce the errors:

* Rate limit exceeded
Delete ~/.local/share/com.ubuntu.reminders/, connect to the evernote300 account, scroll from top to bottom and wait until everything is synced. Repeat twice and you'll run into the rate limit.

* Upload quota exceeded
On evernote website, go to account settings, see the quota status. Upload stuff until the quota is reached. Then on the phone, create a note and attach a picture. The app should show the quota warning.

* Authentication expired
With a properly set up account, go to the evernote webbsite -> settings -> applications, and delete the auth for the account. Then do something with the app.

Pleas try to create other error situations and let me know if you manage to get into some error state where we should show something to the user or that is otherwise unhandled correctly.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:387
http://91.189.93.70:8080/job/reminders-app-ci/710/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2310
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2310/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/312
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-vivid-amd64-ci/131

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/710/rebuild

review: Approve (continuous-integration)

[lp:~mzanetti/reminders-app/improve-error-handling]

388. By Michael Zanetti on 2015-03-08

update user errors in all callbacks

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:388
http://91.189.93.70:8080/job/reminders-app-ci/711/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2311
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2311/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/313
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-vivid-amd64-ci/132

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/711/rebuild

review: Approve (continuous-integration)

[Riccardo Padovani (rpadovani)]

Looks good to me, but you discarded 4 gettext, why?

155 - message = gettext("Authentication expired.");
156 + message = "Authentication expired.";

review: Needs Information

[Michael Zanetti (mzanetti)]

> Looks good to me, but you discarded 4 gettext, why?
>
> 155 - message = gettext("Authentication expired.");
> 156 + message = "Authentication expired.";

Those are in the "EDAMSystemException" case. That one will only throw errors which are not to be presented to the user. The ones to be shown are in the EDAMUserException case. It's a bit odd that both have the same range of error codes, as I believe the "Authentication expired" error can never happen inside the SystemException block. Anyhow, as the api does in theory allow it, I kept the handling and still print this message to the debug output. It is not directly shown to the user any more though, so no need in making translators translate it.

[Riccardo Padovani (rpadovani)]

Okay, then looks good to me, thanks for the explanation!

review: Approve

=== modified file 'src/app/qml/components/StatusBar.qml'
--- src/app/qml/components/StatusBar.qml	2015-03-04 00:23:45 +0000
+++ src/app/qml/components/StatusBar.qml	2015-03-08 22:06:13 +0000
@@ -38,6 +38,7 @@
                 id: label
                 width: parent.width - x
                 wrapMode: Text.WordWrap
+                anchors.verticalCenter: parent.verticalCenter
             }
         }
     }
 
=== modified file 'src/libqtevernote/evernoteconnection.cpp'
--- src/libqtevernote/evernoteconnection.cpp	2015-03-08 22:06:13 +0000
+++ src/libqtevernote/evernoteconnection.cpp	2015-03-08 22:06:13 +0000
@@ -38,6 +38,7 @@
 #include <Errors_types.h>
 
 #include <QUrl>
+#include <QTime>
 
 #include <libintl.h>
 
@@ -65,6 +66,9 @@
     m_userStoreHttpClient(0)
 {
     qRegisterMetaType<EvernoteConnection::ErrorCode>("EvernoteConnection::ErrorCode");
+
+    m_reconnectTimer.setSingleShot(true);
+    connect(&m_reconnectTimer, &QTimer::timeout, this, &EvernoteConnection::connectToEvernote);
 }
 
 void EvernoteConnection::setupUserStore()
@@ -148,6 +152,10 @@
 void EvernoteConnection::disconnectFromEvernote()
 {
     qCDebug(dcConnection) << "Disconnecting from Evernote.";
+
+    m_errorMessage.clear();
+    emit errorChanged();
+
     if (!isConnected()) {
         qCWarning(dcConnection()) << "Not connected. Can't disconnect.";
         return;
@@ -314,6 +322,36 @@
             emit errorChanged();
             return false;
         }
+    } catch (const evernote::edam::EDAMUserException &e) {
+        qCWarning(dcConnection) << "EDAMUserException getting note store path:" << e.what() << "EDAM Error Code:" << e.errorCode;
+        switch (e.errorCode) {
+        case evernote::edam::EDAMErrorCode::AUTH_EXPIRED:
+            m_errorMessage = gettext("Authentication for Evernote server expired. Please renew login information in the accounts settings.");
+            break;
+        default:
+            m_errorMessage = QString(gettext("Unknown error connecting to Evernote: %1")).arg(e.errorCode);
+            break;
+        }
+        emit errorChanged();
+        return false;
+    } catch (const evernote::edam::EDAMSystemException &e) {
+        qCWarning(dcConnection) << "EDAMSystemException getting note store path:" << e.what() << e.errorCode;
+        switch (e.errorCode) {
+        case evernote::edam::EDAMErrorCode::RATE_LIMIT_REACHED:
+            m_errorMessage = gettext("Error connecting to Evernote: Rate limit exceeded. Please try again later.");
+            m_reconnectTimer.stop();
+            m_reconnectTimer.start(e.rateLimitDuration * 1000);
+            {
+                QTime time = QTime::fromMSecsSinceStartOfDay(e.rateLimitDuration * 1000);
+                qCDebug(dcConnection) << "Cannot connect. Rate limit exceeded. Reconnecting in" << time.toString("mm:ss");
+            }
+            break;
+        default:
+            m_errorMessage = gettext("Unknown error connecting to Evernote: %1");
+            break;
+        }
+        emit errorChanged();
+        return false;
     } catch (const TTransportException & e) {
         qCWarning(dcConnection) << "Failed to fetch notestore path:" <<  e.what();
         m_errorMessage = QString(gettext("Error connecting to Evernote: Connection failure when downloading server information."));
@@ -321,7 +359,7 @@
         return false;
     } catch (const TException & e) {
         qCWarning(dcConnection) << "Generic Thrift exception when fetching notestore path:" << e.what();
-        m_errorMessage = gettext("Unknown error connecting to Evernote");
+        m_errorMessage = gettext("Unknown error connecting to Evernote.");
         emit errorChanged();
         return false;
     }
 
=== modified file 'src/libqtevernote/evernoteconnection.h'
--- src/libqtevernote/evernoteconnection.h	2015-03-08 22:06:13 +0000
+++ src/libqtevernote/evernoteconnection.h	2015-03-08 22:06:13 +0000
@@ -28,6 +28,7 @@
 #include <transport/THttpClient.h>
 
 #include <QObject>
+#include <QTimer>
 
 namespace evernote {
 namespace edam {
@@ -141,6 +142,8 @@
 
     evernote::edam::UserStoreClient *m_userstoreClient;
     boost::shared_ptr<THttpClient> m_userStoreHttpClient;
+
+    QTimer m_reconnectTimer;
 };
 
 #endif // EVERNOTECONNECTION_H
 
=== modified file 'src/libqtevernote/jobs/evernotejob.cpp'
--- src/libqtevernote/jobs/evernotejob.cpp	2015-03-08 22:06:13 +0000
+++ src/libqtevernote/jobs/evernotejob.cpp	2015-03-08 22:06:13 +0000
@@ -100,6 +100,7 @@
             }
         } catch (const evernote::edam::EDAMUserException &e) {
             QString message;
+            EvernoteConnection::ErrorCode errorCode = EvernoteConnection::ErrorCodeUserException;
             switch (e.errorCode) {
             case evernote::edam::EDAMErrorCode::UNKNOWN:
                 message = "Unknown Error: %1";
@@ -118,15 +119,18 @@
                 break;
             case evernote::edam::EDAMErrorCode::LIMIT_REACHED:
                 message = "Limit reached: %1";
+                errorCode = EvernoteConnection::ErrorCodeLimitExceeded;
                 break;
             case evernote::edam::EDAMErrorCode::QUOTA_REACHED:
                 message = "Quota reached: %1";
+                errorCode = EvernoteConnection::ErrorCodeQutaExceeded;
                 break;
             case evernote::edam::EDAMErrorCode::INVALID_AUTH:
                 message = "Invalid auth: %1";
                 break;
             case evernote::edam::EDAMErrorCode::AUTH_EXPIRED:
                 message = "Auth expired: %1";
+                errorCode = EvernoteConnection::ErrorCodeAuthExpired;
                 break;
             case evernote::edam::EDAMErrorCode::DATA_CONFLICT:
                 message = "Data conflict: %1";
@@ -161,26 +165,26 @@
             }
             message = message.arg(QString::fromStdString(e.parameter));
             qCWarning(dcJobQueue) << metaObject()->className() << "EDAMUserException:" << message;
-            emitJobDone(EvernoteConnection::ErrorCodeUserException, message);
+            emitJobDone(errorCode, message);
         } catch (const evernote::edam::EDAMSystemException &e) {
             qCWarning(dcJobQueue) << "EDAMSystemException in" << metaObject()->className() << e.what() << e.errorCode << QString::fromStdString(e.message);
             QString message;
             EvernoteConnection::ErrorCode errorCode;
             switch (e.errorCode) {
             case evernote::edam::EDAMErrorCode::AUTH_EXPIRED:
-                message = gettext("Authentication expired.");
+                message = "Authentication expired.";
                 errorCode = EvernoteConnection::ErrorCodeAuthExpired;
                 break;
             case evernote::edam::EDAMErrorCode::LIMIT_REACHED:
-                message = gettext("Limit exceeded.");
+                message = "Limit exceeded.";
                 errorCode = EvernoteConnection::ErrorCodeLimitExceeded;
                 break;
             case evernote::edam::EDAMErrorCode::RATE_LIMIT_REACHED:
-                message = gettext("Rate limit exceeded.");
+                message = "Rate limit exceeded.";
                 errorCode = EvernoteConnection::ErrorCodeRateLimitExceeded;
                 break;
             case evernote::edam::EDAMErrorCode::QUOTA_REACHED:
-                message = gettext("Quota exceeded.");
+                message = "Quota exceeded.";
                 errorCode = EvernoteConnection::ErrorCodeQutaExceeded;
                 break;
             default:
 
=== modified file 'src/libqtevernote/jobs/fetchnotejob.cpp'
--- src/libqtevernote/jobs/fetchnotejob.cpp	2015-02-26 22:47:10 +0000
+++ src/libqtevernote/jobs/fetchnotejob.cpp	2015-03-08 22:06:13 +0000
@@ -53,6 +53,8 @@
 
 void FetchNoteJob::startJob()
 {
+    // Just in case we error out, make sure the reply can be idenfied by note guid
+    m_result.guid = m_guid.toStdString();
     client()->getNote(m_result, token().toStdString(), m_guid.toStdString(), m_what == LoadContent, m_what == LoadResources, false, false);
 }
 
 
=== modified file 'src/libqtevernote/notesstore.cpp'
--- src/libqtevernote/notesstore.cpp	2015-03-08 22:06:13 +0000
+++ src/libqtevernote/notesstore.cpp	2015-03-08 22:06:13 +0000
@@ -307,9 +307,11 @@
 
     notebook->setLoading(false);
 
+    handleUserError(errorCode);
     if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "Error creating notebook:" << errorMessage;
         notebook->setSyncError(true);
+        emit notebookChanged(notebook->guid());
         return;
     }
     QString guid = QString::fromStdString(result.guid);
@@ -503,6 +505,8 @@
     }
 
     tag->setLoading(false);
+
+    handleUserError(errorCode);
     if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "Error creating tag on server:" << errorMessage;
         tag->setSyncError(true);
@@ -540,6 +544,8 @@
         return;
     }
     tag->setLoading(false);
+
+    handleUserError(errorCode);
     if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "Error updating tag on server" << errorMessage;
         tag->setSyncError(true);
@@ -625,26 +631,8 @@
 
 void NotesStore::fetchNotesJobDone(EvernoteConnection::ErrorCode errorCode, const QString &errorMessage, const evernote::edam::NotesMetadataList &results, const QString &filterNotebookGuid)
 {
-    switch (errorCode) {
-    case EvernoteConnection::ErrorCodeNoError:
-        // All is well...
-        break;
-    case EvernoteConnection::ErrorCodeUserException:
-        qCWarning(dcSync) << "FetchNotesJobDone: EDAMUserException:" << errorMessage;
-        m_loading = false;
-        emit loadingChanged();
-        return; // silently discarding...
-    case EvernoteConnection::ErrorCodeConnectionLost:
-        qCWarning(dcSync) << "FetchNotesJobDone: Connection with evernote lost:" << errorMessage;
-        m_loading = false;
-        emit loadingChanged();
-        return; // silently discarding...
-    case EvernoteConnection::ErrorCodeNotFoundExcpetion:
-        qCWarning(dcSync) << "FetchNotesJobDone: Item not found on server:" << errorMessage;
-        m_loading = false;
-        emit loadingChanged();
-        return; // silently discarding...
-    default:
+    handleUserError(errorCode);
+    if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "FetchNotesJobDone: Failed to fetch notes list:" << errorMessage << errorCode;
         m_loading = false;
         emit loadingChanged();
@@ -825,24 +813,10 @@
     QModelIndex noteIndex = index(m_notes.indexOf(note));
     QVector<int> roles;
 
-    switch (errorCode) {
-    case EvernoteConnection::ErrorCodeNoError:
-        // All is well
-        break;
-    case EvernoteConnection::ErrorCodeUserException:
-        qCWarning(dcSync) << "FetchNoteJobDone: EDAMUserException:" << errorMessage;
-        emit dataChanged(noteIndex, noteIndex, roles);
-        return; // silently discarding...
-    case EvernoteConnection::ErrorCodeConnectionLost:
-        qCWarning(dcSync) << "FetchNoteJobDone: Connection with evernote lost:" << errorMessage;
-        emit dataChanged(noteIndex, noteIndex, roles);
-        return; // silently discarding...
-    case EvernoteConnection::ErrorCodeNotFoundExcpetion:
-        qCWarning(dcSync) << "FetchNoteJobDone: Item not found on server:" << errorMessage;
-        emit dataChanged(noteIndex, noteIndex, roles);
-        return; // silently discarding...
-    default:
-        qCWarning(dcSync) << "FetchNoteJobDone: Failed to fetch note content:" << errorMessage << errorCode;
+    handleUserError(errorCode);
+    if (errorCode != EvernoteConnection::ErrorCodeNoError) {
+        note->setLoading(false);
+        roles << RoleLoading;
         note->setSyncError(true);
         roles << RoleSyncError;
         emit dataChanged(noteIndex, noteIndex, roles);
@@ -952,20 +926,10 @@
     m_notebooksLoading = false;
     emit notebooksLoadingChanged();
 
-    switch (errorCode) {
-    case EvernoteConnection::ErrorCodeNoError:
-        // All is well...
-        break;
-    case EvernoteConnection::ErrorCodeUserException:
-        qCWarning(dcSync) << "FetchNotebooksJobDone: EDAMUserException:" << errorMessage;
-        // silently discarding...
-        return;
-    case EvernoteConnection::ErrorCodeConnectionLost:
-        qCWarning(dcSync) << "FetchNotebooksJobDone: Connection lost:" << errorMessage;
-        return; // silently discarding
-    default:
+    handleUserError(errorCode);
+    if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "FetchNotebooksJobDone: Failed to fetch notes list:" << errorMessage << errorCode;
-        return; // silently discarding
+        return;
     }
 
     QList<Notebook*> unhandledNotebooks = m_notebooks;
@@ -1064,20 +1028,10 @@
     m_tagsLoading = false;
     emit tagsLoadingChanged();
 
-    switch (errorCode) {
-    case EvernoteConnection::ErrorCodeNoError:
-        // All is well...
-        break;
-    case EvernoteConnection::ErrorCodeUserException:
-        qCWarning(dcSync) << "FetchTagsJobDone: EDAMUserException:" << errorMessage;
-        // silently discarding...
-        return;
-    case EvernoteConnection::ErrorCodeConnectionLost:
-        qCWarning(dcSync) << "FetchTagsJobDone: Connection lost:" << errorMessage;
-        return; // silently discarding
-    default:
+    handleUserError(errorCode);
+    if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "FetchTagsJobDone: Failed to fetch notes list:" << errorMessage << errorCode;
-        return; // silently discarding
+        return;
     }
 
     QHash<QString, Tag*> unhandledTags = m_tagsHash;
@@ -1204,6 +1158,7 @@
     note->setLoading(false);
     roles << RoleLoading;
 
+    handleUserError(errorCode);
     if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "Error creating note on server:" << tmpGuid << errorMessage;
         note->setSyncError(true);
@@ -1305,13 +1260,13 @@
     int idx = m_notes.indexOf(note);
     note->setLoading(false);
 
+    handleUserError(errorCode);
     if (errorCode != EvernoteConnection::ErrorCodeNoError) {
-        qCWarning(dcSync) << "Error saving note:" << errorMessage;
+        qCWarning(dcSync) << "Unhandled error saving note:" << errorCode << "Message:" << errorMessage;
         note->setSyncError(true);
         emit dataChanged(index(idx), index(idx), QVector<int>() << RoleLoading << RoleSyncError);
         return;
     }
-    note->setSyncError(false);
 
     note->setUpdateSequenceNumber(result.updateSequenceNum);
     note->setLastSyncedSequenceNumber(result.updateSequenceNum);
@@ -1328,19 +1283,25 @@
 
 void NotesStore::saveNotebookJobDone(EvernoteConnection::ErrorCode errorCode, const QString &errorMessage, const evernote::edam::Notebook &result)
 {
-    if (errorCode != EvernoteConnection::ErrorCodeNoError) {
-        qCWarning(dcSync) << "Error saving notebook to server" << errorMessage;
-        return;
-    }
-
     Notebook *notebook = m_notebooksHash.value(QString::fromStdString(result.guid));
     if (!notebook) {
         qCWarning(dcSync) << "Save notebook job done but notebook can't be found any more!";
         return;
     }
+
+    handleUserError(errorCode);
+    if (errorCode != EvernoteConnection::ErrorCodeNoError) {
+        qCWarning(dcSync) << "Error saving notebook to server" << errorCode << errorMessage;
+        notebook->setSyncError(true);
+        emit notebookChanged(notebook->guid());
+        return;
+    }
+
+    notebook->setLoading(false);
+    notebook->setSyncError(false);
+
     qCDebug(dcSync) << "Notebooks saved to server:" << notebook->guid();
     updateFromEDAM(result, notebook);
-    notebook->setLoading(false);
     emit notebookChanged(notebook->guid());
     syncToCacheFile(notebook);
 }
@@ -1411,6 +1372,7 @@
 
 void NotesStore::deleteNoteJobDone(EvernoteConnection::ErrorCode errorCode, const QString &errorMessage, const QString &guid)
 {
+    handleUserError(errorCode);
     if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "Cannot delete note from server:" << errorMessage;
         return;
@@ -1431,6 +1393,7 @@
 
 void NotesStore::expungeNotebookJobDone(EvernoteConnection::ErrorCode errorCode, const QString &errorMessage, const QString &guid)
 {
+    handleUserError(errorCode);
     if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "Error expunging notebook:" << errorMessage;
         return;
@@ -1644,6 +1607,25 @@
     notebook->setLastSyncedSequenceNumber(evNotebook.updateSequenceNum);
 }
 
+bool NotesStore::handleUserError(EvernoteConnection::ErrorCode errorCode)
+{
+    switch (errorCode) {
+    case EvernoteConnection::ErrorCodeAuthExpired:
+        m_errorQueue.append(gettext("Authentication for Evernote server expired. Please renew login information in the accounts settings."));
+        break;
+    case EvernoteConnection::ErrorCodeLimitExceeded:
+        m_errorQueue.append(gettext("Rate limit for Evernote server exceeded. Please try again later."));
+        break;
+    case EvernoteConnection::ErrorCodeQutaExceeded:
+        m_errorQueue.append(gettext("Upload quota for Evernote server exceed. Please try again later."));
+        break;
+    default:
+        return false;
+    }
+    emit errorChanged();
+    return true;
+}
+
 
 void NotesStore::expungeTag(const QString &guid)
 {
 
=== modified file 'src/libqtevernote/notesstore.h'
--- src/libqtevernote/notesstore.h	2015-03-04 00:23:45 +0000
+++ src/libqtevernote/notesstore.h	2015-03-08 22:06:13 +0000
@@ -203,6 +203,8 @@
     QVector<int>    updateFromEDAM(const evernote::edam::NoteMetadata &evNote, Note *note);
     void updateFromEDAM(const evernote::edam::Notebook &evNotebook, Notebook *notebook);
 
+    bool handleUserError(EvernoteConnection::ErrorCode errorCode);
+
 private:
     explicit NotesStore(QObject *parent = 0);
     static NotesStore *s_instance;