Merge branch 'master' into use-clang-format

2 files changed, 93 insertions, 72 deletions

diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp
index 9c0b431c..f46d2d61 100644
--- a/lib/jobs/basejob.cpp
+++ b/lib/jobs/basejob.cpp
@@ -275,51 +275,23 @@ void BaseJob::gotReply()
     if (status().good())
         setStatus(parseReply(d->reply.data()));
     else {
-        // FIXME: Factor out to smth like BaseJob::handleError()
         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->rawResponse.left(500);
-        if (jsonBody) {
-            auto json = QJsonDocument::fromJson(d->rawResponse).object();
-            const auto errCode = json.value("errcode"_ls).toString();
-            if (error() == TooManyRequestsError
-                || errCode == "M_LIMIT_EXCEEDED") {
-                QString msg = tr("Too many requests");
-                auto retryInterval = json.value("retry_after_ms"_ls).toInt(-1);
-                if (retryInterval != -1)
-                    msg +=
-                        tr(", next retry advised after %1 ms").arg(retryInterval);
-                else // We still have to figure some reasonable interval
-                    retryInterval = getNextRetryInterval();
-
-                setStatus(TooManyRequestsError, msg);
-
-                // Shortcut to retry instead of executing finishJob()
-                stop();
-                qCWarning(d->logCat)
-                    << this << "will retry in" << retryInterval << "ms";
-                d->retryTimer.start(retryInterval);
-                emit retryScheduled(d->retriesTaken, retryInterval);
-                return;
-            }
-            if (errCode == "M_CONSENT_NOT_GIVEN") {
-                d->status.code = UserConsentRequiredError;
-                d->errorUrl = json.value("consent_uri"_ls).toString();
-            } else if (errCode == "M_UNSUPPORTED_ROOM_VERSION"
-                       || errCode == "M_INCOMPATIBLE_ROOM_VERSION") {
-                d->status.code = UnsupportedRoomVersionError;
-                if (json.contains("room_version"))
-                    d->status.message =
-                        tr("Requested room version: %1")
-                            .arg(json.value("room_version").toString());
-            } else if (!json.isEmpty()) // Not localisable on the client side
-                setStatus(d->status.code, json.value("error"_ls).toString());
-        }
+        if (jsonBody)
+            setStatus(
+                parseError(d->reply.data(),
+                           QJsonDocument::fromJson(d->rawResponse).object()));
     }
 
-    finishJob();
+    if (error() != TooManyRequestsError)
+        finishJob();
+    else {
+        stop();
+        emit retryScheduled(d->retriesTaken, d->retryTimer.interval());
+    }
 }
 
 bool checkContentType(const QByteArray& type, const QByteArrayList& patterns)
@@ -348,34 +320,6 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns)
     return false;
 }
 
-BaseJob::Status BaseJob::Status::fromHttpCode(int httpCode, QString msg)
-{
-    // clang-format off
-    return { [httpCode]() -> StatusCode {
-            if (httpCode / 10 == 41) // 41x errors
-                return httpCode == 410 ? IncorrectRequestError : NotFoundError;
-            switch (httpCode) {
-            case 401: case 403: case 407:
-                return ContentAccessError;
-            case 404:
-                return NotFoundError;
-            case 400: case 405: case 406: case 426: case 428: case 505:
-            case 494: // Unofficial nginx "Request header too large"
-            case 497: // Unofficial nginx "HTTP request sent to HTTPS port"
-                return IncorrectRequestError;
-            case 429:
-                return TooManyRequestsError;
-            case 501: case 510:
-                return RequestNotImplementedError;
-            case 511:
-                return NetworkAuthRequiredError;
-            default:
-                return NetworkError;
-            }
-        }(), msg };
-    // clang-format on
-}
-
 BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const
 {
     // QNetworkReply error codes seem to be flawed when it comes to HTTP;
@@ -409,7 +353,38 @@ BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const
 
     qCWarning(d->logCat).noquote().nospace() << this << urlString;
     qCWarning(d->logCat).noquote() << "  " << httpCode << reason << replyState;
-    return Status::fromHttpCode(httpCode, reply->errorString());
+    return { [httpCode]() -> StatusCode {
+                if (httpCode / 10 == 41)
+                    return httpCode == 410 ? IncorrectRequestError
+                                           : NotFoundError;
+                switch (httpCode) {
+                case 401:
+                case 403:
+                case 407:
+                    return ContentAccessError;
+                case 404:
+                    return NotFoundError;
+                case 400:
+                case 405:
+                case 406:
+                case 426:
+                case 428:
+                case 505:
+                case 494: // Unofficial nginx "Request header too large"
+                case 497: // Unofficial nginx "HTTP request sent to HTTPS port"
+                    return IncorrectRequestError;
+                case 429:
+                    return TooManyRequestsError;
+                case 501:
+                case 510:
+                    return RequestNotImplementedError;
+                case 511:
+                    return NetworkAuthRequiredError;
+                default:
+                    return NetworkError;
+                }
+            }(),
+             reply->errorString() };
 }
 
 BaseJob::Status BaseJob::parseReply(QNetworkReply* reply)
@@ -425,8 +400,46 @@ BaseJob::Status BaseJob::parseReply(QNetworkReply* reply)
 
 BaseJob::Status BaseJob::parseJson(const QJsonDocument&) { return Success; }
 
+BaseJob::Status BaseJob::parseError(QNetworkReply* reply,
+                                    const QJsonObject& errorJson)
+{
+    const auto errCode = errorJson.value("errcode"_ls).toString();
+    if (error() == TooManyRequestsError || errCode == "M_LIMIT_EXCEEDED") {
+        QString msg = tr("Too many requests");
+        auto retryInterval = errorJson.value("retry_after_ms"_ls).toInt(-1);
+        if (retryInterval != -1)
+            msg += tr(", next retry advised after %1 ms").arg(retryInterval);
+        else // We still have to figure some reasonable interval
+            retryInterval = getNextRetryInterval();
+
+        qCWarning(d->logCat) << this << "will retry in" << retryInterval << "ms";
+        d->retryTimer.start(retryInterval);
+
+        return { TooManyRequestsError, msg };
+    }
+    if (errCode == "M_CONSENT_NOT_GIVEN") {
+        d->errorUrl = errorJson.value("consent_uri"_ls).toString();
+        return { UserConsentRequiredError };
+    }
+    if (errCode == "M_UNSUPPORTED_ROOM_VERSION"
+        || errCode == "M_INCOMPATIBLE_ROOM_VERSION")
+        return { UnsupportedRoomVersionError,
+                 errorJson.contains("room_version"_ls)
+                     ? tr("Requested room version: %1")
+                           .arg(errorJson.value("room_version"_ls).toString())
+                     : errorJson.value("error"_ls).toString() };
+
+    // Not localisable on the client side
+    if (errorJson.contains("error"_ls))
+        d->status.message = errorJson.value("error"_ls).toString();
+
+    return d->status;
+}
+
 void BaseJob::stop()
 {
+    // This method is used to semi-finalise the job before retrying; so
+    // stop the timeout timer but keep the retry timer running.
     d->timer.stop();
     if (d->reply) {
         d->reply->disconnect(this); // Ignore whatever comes from the reply
diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h
index 4d379f26..d94ab31d 100644
--- a/lib/jobs/basejob.h
+++ b/lib/jobs/basejob.h
@@ -116,8 +116,9 @@ public:
      * along the lines of StatusCode, with additional values
      * starting at UserDefinedError
      */
-    struct Status
+    class Status
     {
+    public:
         Status(StatusCode c)
             : code(c)
         {}
@@ -125,7 +126,6 @@ public:
             : code(c)
             , message(std::move(m))
         {}
-        static Status fromHttpCode(int httpCode, QString msg = {});
 
         bool good() const { return code < ErrorLevel; }
         friend QDebug operator<<(QDebug dbg, const Status& s)
@@ -326,8 +326,7 @@ protected:
      * Processes the reply. By default, parses the reply into
      * a QJsonDocument and calls parseJson() if it's a valid JSON.
      *
-     * @param reply raw contents of a HTTP reply from the server (without
-     * headers)
+     * @param reply raw contents of a HTTP reply from the server
      *
      * @see gotReply, parseJson
      */
@@ -335,7 +334,7 @@ protected:
 
     /**
      * Processes the JSON document received from the Matrix server.
-     * By default returns succesful status without analysing the JSON.
+     * By default returns successful status without analysing the JSON.
      *
      * @param json valid JSON document received from the server
      *
@@ -343,6 +342,15 @@ protected:
      */
     virtual Status parseJson(const QJsonDocument&);
 
+    /**
+     * Processes the reply in case of unsuccessful HTTP code.
+     * The body is already loaded from the reply object to errorJson.
+     * @param reply the HTTP reply from the server
+     * @param errorJson the JSON payload describing the error
+     */
+    virtual Status parseError(QNetworkReply* reply,
+                              const QJsonObject& errorJson);
+
     void setStatus(Status s);
     void setStatus(int code, QString message);