BaseJob: "background" switch; more extensive error reporting

Running a request in background, aside from some tweaks on the network
layer (see QNetworkRequest::BackgroundRequestAttribute), allows to
distinguish jobs not immediately caused by user interaction (such as
fetching thumbnails). This can be used to show or not show certain
notifications in UI of clients.

Error reporting has been extended with more methods:
errorCaption() - a human-readable phrase calculated from the status
code; intended to be shown as a dialog caption and in similar
situations.
errorRawData() - former errorDetails(), returns the raw response from
the server.
errorUrl() - returns a URL that may be useful with the error (e.g. for
the upcoming "consent not given" error, this will have the policy URL).
Connection::resultFailed() - a new signal emitted when _any_
BaseJob::failure() is emitted (enables centralised error handling
across all network requests in clients).

As a part of matching changes in Connection, callApi has an overload
that allows to specify the policy; a custom enum instead of bool has
been chosen for the parameter type, to avoid clashes with (arbitrary)
types of job parameters.

4 files changed, 163 insertions, 47 deletions

diff --git a/lib/connection.cpp b/lib/connection.cpp
index 84557224..572ac76b 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -196,7 +196,7 @@ void Connection::doConnectToServer(const QString& user, const QString& password,
         });
     connect(loginJob, &BaseJob::failure, this,
         [this, loginJob] {
-            emit loginError(loginJob->errorString(), loginJob->errorDetails());
+            emit loginError(loginJob->errorString(), loginJob->errorRawData());
         });
 }
 
@@ -263,7 +263,7 @@ void Connection::sync(int timeout)
     // Raw string: http://en.cppreference.com/w/cpp/language/string_literal
     const QString filter { R"({"room": { "timeline": { "limit": 100 } } })" };
     auto job = d->syncJob =
-            callApi<SyncJob>(d->data->lastEvent(), filter, timeout);
+            callApi<SyncJob>(InBackground, d->data->lastEvent(), filter, timeout);
     connect( job, &SyncJob::success, this, [this, job] {
         onSyncSuccess(job->takeData());
         d->syncJob = nullptr;
@@ -272,15 +272,19 @@ void Connection::sync(int timeout)
     connect( job, &SyncJob::retryScheduled, this,
         [this,job] (int retriesTaken, int nextInMilliseconds)
         {
-            emit networkError(job->errorString(), job->errorDetails(),
+            emit networkError(job->errorString(), job->errorRawData(),
                               retriesTaken, nextInMilliseconds);
         });
     connect( job, &SyncJob::failure, this, [this, job] {
         d->syncJob = nullptr;
         if (job->error() == BaseJob::ContentAccessError)
-            emit loginError(job->errorString(), job->errorDetails());
+        {
+            qCWarning(SYNCJOB)
+                << "Sync job failed with ContentAccessError - login expired?";
+            emit loginError(job->errorString(), job->errorRawData());
+        }
         else
-            emit syncError(job->errorString(), job->errorDetails());
+            emit syncError(job->errorString(), job->errorRawData());
     });
 }
 
@@ -386,22 +390,24 @@ inline auto splitMediaId(const QString& mediaId)
     return idParts;
 }
 
-MediaThumbnailJob* Connection::getThumbnail(const QString& mediaId, QSize requestedSize) const
+MediaThumbnailJob* Connection::getThumbnail(const QString& mediaId,
+    QSize requestedSize, RunningPolicy policy) const
 {
     auto idParts = splitMediaId(mediaId);
-    return callApi<MediaThumbnailJob>(idParts.front(), idParts.back(),
-                                      requestedSize);
+    return callApi<MediaThumbnailJob>(policy,
+        idParts.front(), idParts.back(), requestedSize);
 }
 
-MediaThumbnailJob* Connection::getThumbnail(const QUrl& url, QSize requestedSize) const
+MediaThumbnailJob* Connection::getThumbnail(const QUrl& url,
+    QSize requestedSize, RunningPolicy policy) const
 {
-    return getThumbnail(url.authority() + url.path(), requestedSize);
+    return getThumbnail(url.authority() + url.path(), requestedSize, policy);
 }
 
-MediaThumbnailJob* Connection::getThumbnail(const QUrl& url, int requestedWidth,
-                                            int requestedHeight) const
+MediaThumbnailJob* Connection::getThumbnail(const QUrl& url,
+    int requestedWidth, int requestedHeight, RunningPolicy policy) const
 {
-    return getThumbnail(url, QSize(requestedWidth, requestedHeight));
+    return getThumbnail(url, QSize(requestedWidth, requestedHeight), policy);
 }
 
 UploadContentJob* Connection::uploadContent(QIODevice* contentSource,
diff --git a/lib/connection.h b/lib/connection.h
index 4ce1d127..197e4785 100644
--- a/lib/connection.h
+++ b/lib/connection.h
@@ -46,6 +46,13 @@ namespace QMatrixClient
     class GetContentJob;
     class DownloadFileJob;
 
+    /** Enumeration with flags defining the network job running policy
+     * So far only background/foreground flags are available.
+     *
+     * \sa Connection::callApi
+     */
+    enum RunningPolicy { InForeground = 0x0, InBackground = 0x1 };
+
     class Connection: public QObject {
             Q_OBJECT
 
@@ -191,20 +198,39 @@ namespace QMatrixClient
             bool cacheState() const;
             void setCacheState(bool newValue);
 
-            /**
+            /** Start a job of a specified type with specified arguments and policy
+             *
              * This is a universal method to start a job of a type passed
-             * as a template parameter. Arguments to callApi() are arguments
-             * to the job constructor _except_ the first ConnectionData*
-             * argument - callApi() will pass it automatically.
+             * as a template parameter. The policy allows to fine-tune the way
+             * the job is executed - as of this writing it means a choice
+             * between "foreground" and "background".
+             *
+             * \param runningPolicy controls how the job is executed
+             * \param jobArgs arguments to the job constructor
+             *
+             * \sa BaseJob::isBackground. QNetworkRequest::BackgroundRequestAttribute
              */
             template <typename JobT, typename... JobArgTs>
-            JobT* callApi(JobArgTs&&... jobArgs) const
+            JobT* callApi(RunningPolicy runningPolicy,
+                          JobArgTs&&... jobArgs) const
             {
                 auto job = new JobT(std::forward<JobArgTs>(jobArgs)...);
-                job->start(connectionData());
+                connect(job, &BaseJob::failure, this, &Connection::requestFailed);
+                job->start(connectionData(), runningPolicy&InBackground);
                 return job;
             }
 
+            /** Start a job of a specified type with specified arguments
+             *
+             * This is an overload that calls the job with "foreground" policy.
+             */
+            template <typename JobT, typename... JobArgTs>
+            JobT* callApi(JobArgTs&&... jobArgs) const
+            {
+                return callApi<JobT>(InForeground,
+                                     std::forward<JobArgTs>(jobArgs)...);
+            }
+
             /** Generates a new transaction id. Transaction id's are unique within
              * a single Connection object
              */
@@ -246,12 +272,12 @@ namespace QMatrixClient
             void stopSync();
 
             virtual MediaThumbnailJob* getThumbnail(const QString& mediaId,
-                                                    QSize requestedSize) const;
+                QSize requestedSize, RunningPolicy policy = InBackground) const;
             MediaThumbnailJob* getThumbnail(const QUrl& url,
-                                            QSize requestedSize) const;
+                QSize requestedSize, RunningPolicy policy = InBackground) const;
             MediaThumbnailJob* getThumbnail(const QUrl& url,
-                                            int requestedWidth,
-                                            int requestedHeight) const;
+                int requestedWidth, int requestedHeight,
+                RunningPolicy policy = InBackground) const;
 
             // QIODevice* should already be open
             UploadContentJob* uploadContent(QIODevice* contentSource,
@@ -349,9 +375,26 @@ namespace QMatrixClient
             void homeserverChanged(QUrl baseUrl);
 
             void connected();
-            void reconnected(); //< Unused; use connected() instead
+            void reconnected(); //< \deprecated Use connected() instead
             void loggedOut();
             void loginError(QString message, QByteArray details);
+
+            /** A network request (job) failed
+             *
+             * @param request - the pointer to the failed job
+             */
+            void requestFailed(BaseJob* request);
+
+            /** A network request (job) failed due to network problems
+             *
+             * This is _only_ emitted when the job will retry on its own;
+             * once it gives up, requestFailed() will be emitted.
+             *
+             * @param message - message about the network problem
+             * @param details - raw error details, if any available
+             * @param retriesTaken - how many retries have already been taken
+             * @param nextRetryInMilliseconds - when the job will retry again
+             */
             void networkError(QString message, QByteArray details,
                               int retriesTaken, int nextRetryInMilliseconds);
 
diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp
index 9a5460d4..37ecad18 100644
--- a/lib/jobs/basejob.cpp
+++ b/lib/jobs/basejob.cpp
@@ -46,13 +46,13 @@ class BaseJob::Private
     public:
         // Using an idiom from clang-tidy:
         // http://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html
-        Private(HttpVerb v, QString endpoint, QUrlQuery q, Data&& data, bool nt)
-            : verb(v), apiEndpoint(std::move(endpoint))
-            , requestQuery(std::move(q)), requestData(std::move(data))
-            , needsToken(nt)
+        Private(HttpVerb v, QString endpoint, const QUrlQuery& q,
+                Data&& data, bool nt)
+            : verb(v), apiEndpoint(std::move(endpoint)), requestQuery(q)
+            , requestData(std::move(data)), needsToken(nt)
         { }
         
-        void sendRequest();
+        void sendRequest(bool inBackground);
         const JobTimeoutConfig& getCurrentTimeoutConfig() const;
 
         const ConnectionData* connection = nullptr;
@@ -72,7 +72,8 @@ class BaseJob::Private
 
         QScopedPointer<QNetworkReply, NetworkReplyDeleter> reply;
         Status status = Pending;
-        QByteArray details; //< In case of error, contains the raw response body
+        QByteArray rawResponse;
+        QUrl errorUrl; //< May contain a URL to help with some errors
 
         QTimer timer;
         QTimer retryTimer;
@@ -97,8 +98,6 @@ BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint,
     setExpectedContentTypes({ "application/json" });
     d->timer.setSingleShot(true);
     connect (&d->timer, &QTimer::timeout, this, &BaseJob::timeout);
-    d->retryTimer.setSingleShot(true);
-    connect (&d->retryTimer, &QTimer::timeout, this, &BaseJob::sendRequest);
 }
 
 BaseJob::~BaseJob()
@@ -107,6 +106,17 @@ BaseJob::~BaseJob()
     qCDebug(d->logCat) << this << "destroyed";
 }
 
+QUrl BaseJob::requestUrl() const
+{
+    return d->reply ? d->reply->request().url() : QUrl();
+}
+
+bool BaseJob::isBackground() const
+{
+    return d->reply && d->reply->request().attribute(
+                        QNetworkRequest::BackgroundRequestAttribute).toBool();
+}
+
 const QString& BaseJob::apiEndpoint() const
 {
     return d->apiEndpoint;
@@ -180,7 +190,7 @@ QUrl BaseJob::makeRequestUrl(QUrl baseUrl,
     return baseUrl;
 }
 
-void BaseJob::Private::sendRequest()
+void BaseJob::Private::sendRequest(bool inBackground)
 {
     QNetworkRequest req
         { makeRequestUrl(connection->baseUrl(), apiEndpoint, requestQuery) };
@@ -188,6 +198,7 @@ void BaseJob::Private::sendRequest()
         req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json");
     req.setRawHeader(QByteArray("Authorization"),
                      QByteArray("Bearer ") + connection->accessToken());
+    req.setAttribute(QNetworkRequest::BackgroundRequestAttribute, inBackground);
 #if (QT_VERSION >= QT_VERSION_CHECK(5, 6, 0))
     req.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true);
     req.setMaximumRedirectsAllowed(10);
@@ -220,26 +231,30 @@ void BaseJob::afterStart(const ConnectionData*, QNetworkReply*)
 void BaseJob::beforeAbandon(QNetworkReply*)
 { }
 
-void BaseJob::start(const ConnectionData* connData)
+void BaseJob::start(const ConnectionData* connData, bool inBackground)
 {
     d->connection = connData;
+    d->retryTimer.setSingleShot(true);
+    connect (&d->retryTimer, &QTimer::timeout,
+             this, [this,inBackground] { sendRequest(inBackground); });
+
     beforeStart(connData);
     if (status().good())
-        sendRequest();
+        sendRequest(inBackground);
     if (status().good())
         afterStart(connData, d->reply.data());
     if (!status().good())
         QTimer::singleShot(0, this, &BaseJob::finishJob);
 }
 
-void BaseJob::sendRequest()
+void BaseJob::sendRequest(bool inBackground)
 {
     emit aboutToStart();
     d->retryTimer.stop(); // In case we were counting down at the moment
     qCDebug(d->logCat) << this << "sending request to" << d->apiEndpoint;
     if (!d->requestQuery.isEmpty())
         qCDebug(d->logCat) << "  query:" << d->requestQuery.toString();
-    d->sendRequest();
+    d->sendRequest(inBackground);
     connect( d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply );
     if (d->reply->isRunning())
     {
@@ -269,14 +284,14 @@ void BaseJob::gotReply()
         setStatus(parseReply(d->reply.data()));
     else {
         // FIXME: Factor out to smth like BaseJob::handleError()
-        d->details = d->reply->readAll();
+        d->rawResponse = d->reply->readAll();
         const auto jsonBody =
                 d->reply->rawHeader("Content-Type") == "application/json";
         qCDebug(d->logCat).noquote()
-                << "Error body (truncated if long):" << d->details.left(500);
+                << "Error body (truncated if long):" << d->rawResponse.left(500);
         if (jsonBody)
         {
-            auto json = QJsonDocument::fromJson(d->details).object();
+            auto json = QJsonDocument::fromJson(d->rawResponse).object();
             if (error() == TooManyRequestsError ||
                     json.value("errcode").toString() == "M_LIMIT_EXCEEDED")
             {
@@ -393,8 +408,9 @@ BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const
 
 BaseJob::Status BaseJob::parseReply(QNetworkReply* reply)
 {
+    d->rawResponse = reply->readAll();
     QJsonParseError error;
-    QJsonDocument json = QJsonDocument::fromJson(reply->readAll(), &error);
+    const auto& json = QJsonDocument::fromJson(d->rawResponse, &error);
     if( error.error == QJsonParseError::NoError )
         return parseJson(json);
     else
@@ -493,14 +509,58 @@ int BaseJob::error() const
     return d->status.code;
 }
 
+QString BaseJob::errorCaption() const
+{
+    switch (d->status.code)
+    {
+        case Success:
+            return tr("Success");
+        case Pending:
+            return tr("Request still pending response");
+        case UnexpectedResponseTypeWarning:
+            return tr("Warning: Unexpected response type");
+        case Abandoned:
+            return tr("Request was abandoned");
+        case NetworkError:
+            return tr("Network problems");
+        case JsonParseError:
+            return tr("Response could not be parsed");
+        case TimeoutError:
+            return tr("Request timed out");
+        case ContentAccessError:
+            return tr("Access error");
+        case NotFoundError:
+            return tr("Requested data not found");
+        case IncorrectRequestError:
+            return tr("Invalid request");
+        case IncorrectResponseError:
+            return tr("Response could not be parsed");
+        case TooManyRequestsError:
+            return tr("Too many requests");
+        case RequestNotImplementedError:
+            return tr("Function not implemented by the server");
+        case NetworkAuthRequiredError:
+            return tr("Network authentication required");
+        case UserConsentRequiredError:
+            return tr("User consent required");
+        default:
+            return tr("Request failed");
+    }
+}
+
 QString BaseJob::errorString() const
 {
     return d->status.message;
 }
 
-QByteArray BaseJob::errorDetails() const
+QByteArray BaseJob::errorRawData() const
+{
+    return d->rawResponse;
+}
+
+QUrl BaseJob::errorUrl() const
 {
-    return d->details;
+    return d->errorUrl;
 }
 
 void BaseJob::setStatus(Status s)
diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h
index 6c91a333..25f13472 100644
--- a/lib/jobs/basejob.h
+++ b/lib/jobs/basejob.h
@@ -43,6 +43,7 @@ namespace QMatrixClient
     class BaseJob: public QObject
     {
             Q_OBJECT
+            Q_PROPERTY(QUrl requestUrl READ requestUrl CONSTANT)
             Q_PROPERTY(int maxRetries READ maxRetries WRITE setMaxRetries)
         public:
             /* Just in case, the values are compatible with KJob
@@ -55,7 +56,7 @@ namespace QMatrixClient
                 , Abandoned = 50 //< A very brief period between abandoning and object deletion
                 , ErrorLevel = 100 //< Errors have codes starting from this
                 , NetworkError = 100
-                , JsonParseError
+                , JsonParseError // TODO: Merge into IncorrectResponseError
                 , TimeoutError
                 , ContentAccessError
                 , NotFoundError
@@ -129,10 +130,15 @@ namespace QMatrixClient
                     const Query& query, Data&& data = {},
                     bool needsToken = true);
 
+            QUrl requestUrl() const;
+            bool isBackground() const;
+
             Status status() const;
             int error() const;
+            QString errorCaption() const;
             virtual QString errorString() const;
-            QByteArray errorDetails() const;
+            QByteArray errorRawData() const;
+            QUrl errorUrl() const;
 
             int maxRetries() const;
             void setMaxRetries(int newMaxRetries);
@@ -147,7 +153,8 @@ namespace QMatrixClient
             }
 
         public slots:
-            void start(const ConnectionData* connData);
+            void start(const ConnectionData* connData,
+                       bool inBackground = false);
 
             /**
              * Abandons the result of this job, arrived or unarrived.
@@ -302,7 +309,7 @@ namespace QMatrixClient
             void timeout();
 
         private slots:
-            void sendRequest();
+            void sendRequest(bool inBackground);
             void checkReply();
             void gotReply();