Handle M_UNKNOWN as The Spec says; factor out BaseJob::parseError()

2 files changed, 60 insertions, 50 deletions

diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp
index 97b5a904..dbf197ec 100644
--- a/lib/jobs/basejob.cpp
+++ b/lib/jobs/basejob.cpp
@@ -289,59 +289,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";
+            d->reply->rawHeader("Content-Type") == "application/json";
         qCDebug(d->logCat).noquote()
-                << "Error body (truncated if long):" << d->rawResponse.left(500);
+            << "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_UNKNOWN") {
-                d->status.code = UnknownObjectError;
-            }
-            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());
-        }
+            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)
@@ -445,8 +409,46 @@ 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)
     {
diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h
index c1747cca..30ecceb3 100644
--- a/lib/jobs/basejob.h
+++ b/lib/jobs/basejob.h
@@ -67,7 +67,6 @@ namespace QMatrixClient
                 , UnsupportedRoomVersionError
                 , NetworkAuthRequiredError
                 , UserConsentRequiredError
-                , UnknownObjectError // Unknown room or other item (M_UNKNOWN)
                 , UserDefinedError = 200
             };
 
@@ -303,7 +302,7 @@ namespace QMatrixClient
              * 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
              */
@@ -311,7 +310,7 @@ namespace QMatrixClient
 
             /**
              * 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
              *
@@ -319,6 +318,15 @@ namespace QMatrixClient
              */
             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);