From 26c51455901a6c0112531d86d61d65d31a70dfaa Mon Sep 17 00:00:00 2001
From: Ville Ranki <ville.ranki@iki.fi>
Date: Thu, 4 Apr 2019 11:45:41 +0300
Subject: Ignore some errors on leaving rooms, add new error enum. Fixes #307
---
lib/connection.cpp | 56 ++++++++++++++++++++++++++++++++++++----------------
lib/jobs/basejob.cpp | 3 +++
lib/jobs/basejob.h | 1 +
3 files changed, 43 insertions(+), 17 deletions(-)
(limited to 'lib')
diff --git a/lib/connection.cpp b/lib/connection.cpp
index a7eae30f..5bf89815 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -110,6 +110,7 @@ class Connection::Private
void connectWithToken(const QString& user, const QString& accessToken,
const QString& deviceId);
+ void removeRoom(const QString& roomId);
template <typename EventT>
EventT* unpackAccountData() const
@@ -790,29 +791,36 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id)
if (room && room->joinState() != JoinState::Leave)
{
auto leaveJob = room->leaveRoom();
- connect(leaveJob, &BaseJob::success, this, [this, forgetJob, room] {
- forgetJob->start(connectionData());
- // If the matching /sync response hasn't arrived yet, mark the room
- // for explicit deletion
- if (room->joinState() != JoinState::Leave)
- d->roomIdsToForget.push_back(room->id());
+ connect(leaveJob, &BaseJob::result, this, [this, leaveJob, forgetJob, room] {
+ // After leave, continue if there is no error or the room id is not found (IncorrectRequestError)
+ if(!leaveJob->error() || leaveJob->error() == BaseJob::StatusCode::UnknownObjectError) {
+ forgetJob->start(connectionData());
+ // If the matching /sync response hasn't arrived yet, mark the room
+ // for explicit deletion
+ if (room->joinState() != JoinState::Leave)
+ d->roomIdsToForget.push_back(room->id());
+ } else {
+ qCWarning(MAIN) << "Error leaving room "
+ << room->name() << ":"
+ << leaveJob->errorString();
+ forgetJob->abandon();
+ }
});
connect(leaveJob, &BaseJob::failure, forgetJob, &BaseJob::abandon);
}
else
forgetJob->start(connectionData());
- connect(forgetJob, &BaseJob::success, this, [this, id]
+ connect(forgetJob, &BaseJob::result, this, [this, id, forgetJob]
{
- // Delete whatever instances of the room are still in the map.
- for (auto f: {false, true})
- if (auto r = d->roomMap.take({ id, f }))
- {
- qCDebug(MAIN) << "Room" << r->objectName()
- << "in state" << toCString(r->joinState())
- << "will be deleted";
- emit r->beforeDestruction(r);
- r->deleteLater();
- }
+ // Leave room in case of success, or room not known by server
+ if(!forgetJob->error() || forgetJob->error() == BaseJob::StatusCode::IncorrectRequestError || forgetJob->error() == BaseJob::StatusCode::UnknownObjectError) {
+ // Delete the room from roomMap
+ d->removeRoom(id);
+ } else {
+ qCWarning(MAIN) << "Error forgetting room "
+ << id << ":"
+ << forgetJob->errorString();
+ }
});
return forgetJob;
}
@@ -1056,6 +1064,20 @@ Connection::DirectChatsMap Connection::directChats() const
return d->directChats;
}
+// Removes room with given id from roomMap
+void Connection::Private::removeRoom(const QString& roomId)
+{
+ for (auto f: {false, true})
+ if (auto r = roomMap.take({ roomId, f }))
+ {
+ qCDebug(MAIN) << "Room" << r->objectName()
+ << "in state" << toCString(r->joinState())
+ << "will be deleted";
+ emit r->beforeDestruction(r);
+ r->deleteLater();
+ }
+}
+
void Connection::addToDirectChats(const Room* room, User* user)
{
Q_ASSERT(room != nullptr && user != nullptr);
diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp
index 0d9b9f10..97b5a904 100644
--- a/lib/jobs/basejob.cpp
+++ b/lib/jobs/basejob.cpp
@@ -325,6 +325,9 @@ void BaseJob::gotReply()
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")
{
diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h
index 4c1c7706..c1747cca 100644
--- a/lib/jobs/basejob.h
+++ b/lib/jobs/basejob.h
@@ -67,6 +67,7 @@ namespace QMatrixClient
, UnsupportedRoomVersionError
, NetworkAuthRequiredError
, UserConsentRequiredError
+ , UnknownObjectError // Unknown room or other item (M_UNKNOWN)
, UserDefinedError = 200
};
--
cgit v1.2.3
From 7591ec34cee15a58611408a996bdb1b92b6ffb98 Mon Sep 17 00:00:00 2001
From: Ville Ranki <ville.ranki@iki.fi>
Date: Tue, 11 Jun 2019 11:58:57 +0300
Subject: Remove unnecessary error checks in lib/connection.cpp
Co-Authored-By: Kitsune Ral <Kitsune-Ral@users.sf.net>
---
lib/connection.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
(limited to 'lib')
diff --git a/lib/connection.cpp b/lib/connection.cpp
index 5bf89815..27211a77 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -813,7 +813,7 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id)
connect(forgetJob, &BaseJob::result, this, [this, id, forgetJob]
{
// Leave room in case of success, or room not known by server
- if(!forgetJob->error() || forgetJob->error() == BaseJob::StatusCode::IncorrectRequestError || forgetJob->error() == BaseJob::StatusCode::UnknownObjectError) {
+ if(!forgetJob->error() || forgetJob->error() == BaseJob::UnknownObjectError) {
// Delete the room from roomMap
d->removeRoom(id);
} else {
--
cgit v1.2.3
From a19e12544d174588bb99d1d9d5b2576f0ea1e037 Mon Sep 17 00:00:00 2001
From: Ville Ranki <ville.ranki@iki.fi>
Date: Tue, 11 Jun 2019 11:59:40 +0300
Subject: Comment change as requested
Co-Authored-By: Kitsune Ral <Kitsune-Ral@users.sf.net>
---
lib/connection.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
(limited to 'lib')
diff --git a/lib/connection.cpp b/lib/connection.cpp
index 27211a77..af2aa2ab 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -792,7 +792,7 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id)
{
auto leaveJob = room->leaveRoom();
connect(leaveJob, &BaseJob::result, this, [this, leaveJob, forgetJob, room] {
- // After leave, continue if there is no error or the room id is not found (IncorrectRequestError)
+ // After leave, continue if there is no error or the room id is not found
if(!leaveJob->error() || leaveJob->error() == BaseJob::StatusCode::UnknownObjectError) {
forgetJob->start(connectionData());
// If the matching /sync response hasn't arrived yet, mark the room
--
cgit v1.2.3
From 12478cf7330727083103d22f76de92c4aa476f5b Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Mon, 1 Jul 2019 16:10:35 +0900
Subject: Handle M_UNKNOWN as The Spec says; factor out BaseJob::parseError()
---
lib/connection.cpp | 46 ++++++++++++-------------
lib/jobs/basejob.cpp | 96 +++++++++++++++++++++++++++-------------------------
lib/jobs/basejob.h | 14 ++++++--
3 files changed, 83 insertions(+), 73 deletions(-)
(limited to 'lib')
diff --git a/lib/connection.cpp b/lib/connection.cpp
index af2aa2ab..4c068b8f 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -791,21 +791,23 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id)
if (room && room->joinState() != JoinState::Leave)
{
auto leaveJob = room->leaveRoom();
- connect(leaveJob, &BaseJob::result, this, [this, leaveJob, forgetJob, room] {
- // After leave, continue if there is no error or the room id is not found
- if(!leaveJob->error() || leaveJob->error() == BaseJob::StatusCode::UnknownObjectError) {
- forgetJob->start(connectionData());
- // If the matching /sync response hasn't arrived yet, mark the room
- // for explicit deletion
- if (room->joinState() != JoinState::Leave)
- d->roomIdsToForget.push_back(room->id());
- } else {
- qCWarning(MAIN) << "Error leaving room "
- << room->name() << ":"
- << leaveJob->errorString();
- forgetJob->abandon();
- }
- });
+ connect(leaveJob, &BaseJob::result, this,
+ [this, leaveJob, forgetJob, room] {
+ if (leaveJob->error() == BaseJob::Success
+ || leaveJob->error() == BaseJob::NotFoundError)
+ {
+ forgetJob->start(connectionData());
+ // If the matching /sync response hasn't arrived yet,
+ // mark the room for explicit deletion
+ if (room->joinState() != JoinState::Leave)
+ d->roomIdsToForget.push_back(room->id());
+ } else {
+ qCWarning(MAIN).nospace()
+ << "Error leaving room " << room->objectName()
+ << ": " << leaveJob->errorString();
+ forgetJob->abandon();
+ }
+ });
connect(leaveJob, &BaseJob::failure, forgetJob, &BaseJob::abandon);
}
else
@@ -813,14 +815,12 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id)
connect(forgetJob, &BaseJob::result, this, [this, id, forgetJob]
{
// Leave room in case of success, or room not known by server
- if(!forgetJob->error() || forgetJob->error() == BaseJob::UnknownObjectError) {
- // Delete the room from roomMap
- d->removeRoom(id);
- } else {
- qCWarning(MAIN) << "Error forgetting room "
- << id << ":"
- << forgetJob->errorString();
- }
+ if(forgetJob->error() == BaseJob::Success
+ || forgetJob->error() == BaseJob::NotFoundError)
+ d->removeRoom(id); // Delete the room from roomMap
+ else
+ qCWarning(MAIN).nospace() << "Error forgetting room " << id << ": "
+ << forgetJob->errorString();
});
return forgetJob;
}
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);
--
cgit v1.2.3