From 07485711f867813098180afcfe15e4af393b66ec Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 28 Feb 2018 14:46:36 +0900 Subject: =?UTF-8?q?Profiling=20logs:=20added=20=C2=B5s,=20less=20empty=20p?= =?UTF-8?q?rofiling=20log=20lines?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #177. --- jobs/syncjob.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'jobs') diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index 7b066f4f..fd8bfc1a 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -83,7 +83,8 @@ BaseJob::Status SyncData::parseJson(const QJsonDocument &data) roomData.emplace_back(roomIt.key(), JoinState(i), roomIt.value().toObject()); } - qCDebug(PROFILER) << "*** SyncData::parseJson():" << et.elapsed() << "ms"; + qCDebug(PROFILER) << "*** SyncData::parseJson(): batch with" + << rooms.size() << "room(s) in" << et; return BaseJob::Success; } -- cgit v1.2.3 From 9ff04e98d62f93a7a6003fc80d189e96c6835f84 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 28 Feb 2018 17:11:10 +0900 Subject: Skip retry interval if the last job attempt timed out Closes #175. --- jobs/basejob.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index c3b110f0..7669d1d4 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -373,7 +373,8 @@ void BaseJob::finishJob() // TODO: The whole retrying thing should be put to ConnectionManager // otherwise independently retrying jobs make a bit of notification // storm towards the UI. - const auto retryInterval = getNextRetryInterval(); + const auto retryInterval = + error() == TimeoutError ? 0 : getNextRetryInterval(); ++d->retriesTaken; qCWarning(d->logCat) << this << "will take retry" << d->retriesTaken << "in" << retryInterval/1000 << "s"; -- cgit v1.2.3 From 4916c0b65f8415db1e189e7a9963fce71d3b8b71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Pl=C3=A1=C5=A1il?= Date: Thu, 1 Mar 2018 23:46:35 +0800 Subject: Improve compatibility with gcc 4.9 to be able to build for Android with QtCreator --- jobs/syncjob.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'jobs') diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index fd8bfc1a..6b3f3acf 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -68,7 +68,7 @@ BaseJob::Status SyncData::parseJson(const QJsonDocument &data) { QElapsedTimer et; et.start(); - auto json { data.object() }; + auto json = data.object(); nextBatch_ = json.value("next_batch").toString(); // TODO: presence accountData.fromJson(json); -- cgit v1.2.3 From 1edfe9d82ea9d9a50645d419c736db45bf940978 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 27 Feb 2018 20:14:47 +0900 Subject: jobs/generated: SetAccountDataJob, SetAccountDataPerRoomJob --- jobs/generated/account-data.cpp | 28 ++++++++++++++++++++++++++++ jobs/generated/account-data.h | 27 +++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 jobs/generated/account-data.cpp create mode 100644 jobs/generated/account-data.h (limited to 'jobs') diff --git a/jobs/generated/account-data.cpp b/jobs/generated/account-data.cpp new file mode 100644 index 00000000..35ee94c0 --- /dev/null +++ b/jobs/generated/account-data.cpp @@ -0,0 +1,28 @@ +/****************************************************************************** + * THIS FILE IS GENERATED - ANY EDITS WILL BE OVERWRITTEN + */ + +#include "account-data.h" + +#include "converters.h" + +#include + +using namespace QMatrixClient; + +static const auto basePath = QStringLiteral("/_matrix/client/r0"); + +SetAccountDataJob::SetAccountDataJob(const QString& userId, const QString& type, const QJsonObject& content) + : BaseJob(HttpVerb::Put, "SetAccountDataJob", + basePath % "/user/" % userId % "/account_data/" % type) +{ + setRequestData(Data(content)); +} + +SetAccountDataPerRoomJob::SetAccountDataPerRoomJob(const QString& userId, const QString& roomId, const QString& type, const QJsonObject& content) + : BaseJob(HttpVerb::Put, "SetAccountDataPerRoomJob", + basePath % "/user/" % userId % "/rooms/" % roomId % "/account_data/" % type) +{ + setRequestData(Data(content)); +} + diff --git a/jobs/generated/account-data.h b/jobs/generated/account-data.h new file mode 100644 index 00000000..69ad9fb4 --- /dev/null +++ b/jobs/generated/account-data.h @@ -0,0 +1,27 @@ +/****************************************************************************** + * THIS FILE IS GENERATED - ANY EDITS WILL BE OVERWRITTEN + */ + +#pragma once + +#include "../basejob.h" + +#include + + +namespace QMatrixClient +{ + // Operations + + class SetAccountDataJob : public BaseJob + { + public: + explicit SetAccountDataJob(const QString& userId, const QString& type, const QJsonObject& content = {}); + }; + + class SetAccountDataPerRoomJob : public BaseJob + { + public: + explicit SetAccountDataPerRoomJob(const QString& userId, const QString& roomId, const QString& type, const QJsonObject& content = {}); + }; +} // namespace QMatrixClient -- cgit v1.2.3 From 63efbe26f37819048bb236d4839cc5f25c102785 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 5 Mar 2018 21:06:16 +0900 Subject: Support server-side read marker (m.full_read) Closes #183. There's also the m.read part but it can be done sometime later, as it's pure optimisation. --- jobs/postreadmarkersjob.h | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 jobs/postreadmarkersjob.h (limited to 'jobs') diff --git a/jobs/postreadmarkersjob.h b/jobs/postreadmarkersjob.h new file mode 100644 index 00000000..d0198821 --- /dev/null +++ b/jobs/postreadmarkersjob.h @@ -0,0 +1,37 @@ +/****************************************************************************** + * Copyright (C) 2017 Kitsune Ral + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#pragma once + +#include "basejob.h" + +using namespace QMatrixClient; + +class PostReadMarkersJob : public BaseJob +{ + public: + explicit PostReadMarkersJob(const QString& roomId, + const QString& readUpToEventId) + : BaseJob(HttpVerb::Post, "PostReadMarkersJob", + QStringLiteral("_matrix/client/r0/rooms/%1/read_markers") + .arg(roomId)) + { + setRequestData(QJsonObject {{ + QStringLiteral("m.fully_read"), readUpToEventId }}); + } +}; -- cgit v1.2.3 From 4e78035d03817c29ceac46bdf9cb045e4ba3a101 Mon Sep 17 00:00:00 2001 From: Krombel Date: Tue, 20 Mar 2018 23:33:42 +0100 Subject: ignore possible appendixes from content type Currently libqmatrixclient fails checking the `Content-Type` header when there is an appendix like "charset". That is allowed e.g. in [rfc7231](https://tools.ietf.org/html/rfc7231#section-3.1.1.5)) One example is a Content-Type `application/json` vs `application/json;charset=UTF-8` Setting of the charset appendis is currently not supported. It fails with libqmatrixclient.jobs: "LoginJob" status 106 : "Incorrect content type of the response" This PR aims to just drop that appendix as it is currently not handled somewhere else. --- jobs/basejob.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 7669d1d4..981b75b1 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -282,9 +282,12 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) if (patterns.isEmpty()) return true; + // ignore possible appendixes of the content type + const auto ctype = type.split(';').front(); + for (const auto& pattern: patterns) { - if (pattern.startsWith('*') || type == pattern) // Fast lane + if (pattern.startsWith('*') || ctype == pattern) // Fast lane return true; auto patternParts = pattern.split('/'); @@ -292,7 +295,7 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) "BaseJob: Expected content type should have up to two" " /-separated parts; violating pattern: " + pattern); - if (type.split('/').front() == patternParts.front() && + if (ctype.split('/').front() == patternParts.front() && patternParts.back() == "*") return true; // Exact match already went on fast lane } -- cgit v1.2.3 From bac178f488299ec37ebe86aa91053cf1133d1c12 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Mar 2018 10:43:39 +0900 Subject: BaseJob::start(): self-destruct if not succesfully started Closes #193. --- jobs/basejob.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 981b75b1..eb250723 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -235,8 +235,12 @@ void BaseJob::start(const ConnectionData* connData) { d->connection = connData; beforeStart(connData); - sendRequest(); - afterStart(connData, d->reply.data()); + if (status().good()) + sendRequest(); + if (status().good()) + afterStart(connData, d->reply.data()); + if (!status().good()) + QTimer::singleShot(0, this, &BaseJob::finishJob); } void BaseJob::sendRequest() -- cgit v1.2.3 From 403dde950b4a7398c856d495ed2d66157b175bf1 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Mar 2018 12:01:46 +0900 Subject: DownloadFileJob::beforeStart(): make sure to immediately return in case of error --- jobs/downloadfilejob.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'jobs') diff --git a/jobs/downloadfilejob.cpp b/jobs/downloadfilejob.cpp index 07d14197..6a3d8483 100644 --- a/jobs/downloadfilejob.cpp +++ b/jobs/downloadfilejob.cpp @@ -54,6 +54,7 @@ void DownloadFileJob::beforeStart(const ConnectionData*) qCWarning(JOBS) << "Couldn't open the temporary file" << d->tempFile->fileName() << "for writing"; setStatus(FileError, "Could not open the temporary download file"); + return; } qCDebug(JOBS) << "Downloading to" << d->tempFile->fileName(); } -- cgit v1.2.3 From e7e9330d665c1d8d2391707d27019a7f454cbcdf Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Mar 2018 11:43:00 +0900 Subject: Introduce JoinStates (QFlags) This required to change numeric values for JoinState enum; so anybody who relied on them being 0-based and/or contiguous, beware. --- jobs/syncjob.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'jobs') diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index 6b3f3acf..ed579f12 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -74,13 +74,14 @@ BaseJob::Status SyncData::parseJson(const QJsonDocument &data) accountData.fromJson(json); QJsonObject rooms = json.value("rooms").toObject(); - for (size_t i = 0; i < JoinStateStrings.size(); ++i) + JoinStates::Int ii = 1; // ii is used to make a JoinState value + for (size_t i = 0; i < JoinStateStrings.size(); ++i, ii <<= 1) { const auto rs = rooms.value(JoinStateStrings[i]).toObject(); // We have a Qt container on the right and an STL one on the left roomData.reserve(static_cast(rs.size())); for(auto roomIt = rs.begin(); roomIt != rs.end(); ++roomIt) - roomData.emplace_back(roomIt.key(), JoinState(i), + roomData.emplace_back(roomIt.key(), JoinState(ii), roomIt.value().toObject()); } qCDebug(PROFILER) << "*** SyncData::parseJson(): batch with" -- cgit v1.2.3 From f213e02daa6b9e83e8e76d1576e446357c6c3bc7 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 27 Mar 2018 20:34:46 +0900 Subject: Rework unread messages counting logic The previous one didn't cover all the cases; the current one seems to do. Closes #192. Accompanied by the developer's notes at: https://github.com/QMatrixClient/libqmatrixclient/wiki/unread_count --- jobs/syncjob.cpp | 22 ++++++++++++++-------- jobs/syncjob.h | 3 +++ 2 files changed, 17 insertions(+), 8 deletions(-) (limited to 'jobs') diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index ed579f12..435dfd0e 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -89,6 +89,9 @@ BaseJob::Status SyncData::parseJson(const QJsonDocument &data) return BaseJob::Success; } +const QString SyncRoomData::UnreadCountKey = + QStringLiteral("x-qmatrixclient.unread_count"); + SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, const QJsonObject& room_) : roomId(roomId_) @@ -116,12 +119,15 @@ SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, qCWarning(SYNCJOB) << "SyncRoomData: Unknown JoinState value, ignoring:" << int(joinState); } - QJsonObject timeline = room_.value("timeline").toObject(); - timelineLimited = timeline.value("limited").toBool(); - timelinePrevBatch = timeline.value("prev_batch").toString(); - - QJsonObject unread = room_.value("unread_notifications").toObject(); - highlightCount = unread.value("highlight_count").toInt(); - notificationCount = unread.value("notification_count").toInt(); - qCDebug(SYNCJOB) << "Highlights: " << highlightCount << " Notifications:" << notificationCount; + auto timelineJson = room_.value("timeline").toObject(); + timelineLimited = timelineJson.value("limited").toBool(); + timelinePrevBatch = timelineJson.value("prev_batch").toString(); + + auto unreadJson = room_.value("unread_notifications").toObject(); + unreadCount = unreadJson.value(UnreadCountKey).toInt(-2); + highlightCount = unreadJson.value("highlight_count").toInt(); + notificationCount = unreadJson.value("notification_count").toInt(); + if (highlightCount > 0 || notificationCount > 0) + qCDebug(SYNCJOB) << "Highlights: " << highlightCount + << " Notifications:" << notificationCount; } diff --git a/jobs/syncjob.h b/jobs/syncjob.h index 5956e73b..919060be 100644 --- a/jobs/syncjob.h +++ b/jobs/syncjob.h @@ -53,6 +53,7 @@ namespace QMatrixClient bool timelineLimited; QString timelinePrevBatch; + int unreadCount; int highlightCount; int notificationCount; @@ -60,6 +61,8 @@ namespace QMatrixClient const QJsonObject& room_); SyncRoomData(SyncRoomData&&) = default; SyncRoomData& operator=(SyncRoomData&&) = default; + + static const QString UnreadCountKey; }; // QVector cannot work with non-copiable objects, std::vector can. using SyncDataList = std::vector; -- cgit v1.2.3 From 503957c86a84f1be190719a17984df1bb1267658 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 28 Mar 2018 11:29:41 +0900 Subject: BaseJob: small refactoring and cleanup in logging code --- jobs/basejob.cpp | 17 +++-------------- jobs/basejob.h | 12 +++++++++++- 2 files changed, 14 insertions(+), 15 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index eb250723..5198c45c 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -84,18 +84,6 @@ class BaseJob::Private LoggingCategory logCat = JOBS; }; -inline QDebug operator<<(QDebug dbg, const BaseJob* j) -{ - return dbg << j->objectName(); -} - -QDebug QMatrixClient::operator<<(QDebug dbg, const BaseJob::Status& s) -{ - QRegularExpression filter { "(access_token)(=|: )[-_A-Za-z0-9]+" }; - return dbg << s.code << ':' - << QString(s.message).replace(filter, "\\1 HIDDEN"); -} - BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, bool needsToken) : BaseJob(verb, name, endpoint, Query { }, Data { }, needsToken) { } @@ -319,7 +307,7 @@ BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const if (checkContentType(reply->rawHeader("Content-Type"), d->expectedContentTypes)) return NoError; - else + else // A warning in the logs might be more proper instead return { IncorrectResponseError, "Incorrect content type of the response" }; @@ -383,7 +371,7 @@ void BaseJob::finishJob() const auto retryInterval = error() == TimeoutError ? 0 : getNextRetryInterval(); ++d->retriesTaken; - qCWarning(d->logCat) << this << "will take retry" << d->retriesTaken + qCWarning(d->logCat) << this << "will retry" << d->retriesTaken << "in" << retryInterval/1000 << "s"; d->retryTimer.start(retryInterval); emit retryScheduled(d->retriesTaken, retryInterval); @@ -456,6 +444,7 @@ void BaseJob::setStatus(Status s) void BaseJob::setStatus(int code, QString message) { + message.replace(d->connection->accessToken(), "(REDACTED)"); setStatus({ code, message }); } diff --git a/jobs/basejob.h b/jobs/basejob.h index a5b457c5..fa253d96 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -98,7 +98,12 @@ namespace QMatrixClient Status(int c, QString m) : code(c), message(std::move(m)) { } bool good() const { return code < ErrorLevel; } - friend QDebug operator<<(QDebug dbg, const Status& s); + friend QDebug operator<<(QDebug dbg, const Status& s) + { + QDebug(dbg).noquote().nospace() + << s.code << ": " << s.message; + return dbg; + } int code; QString message; @@ -124,6 +129,11 @@ namespace QMatrixClient Q_INVOKABLE duration_t getNextRetryInterval() const; Q_INVOKABLE duration_t millisToRetry() const; + friend QDebug operator<<(QDebug dbg, const BaseJob* j) + { + return dbg << j->objectName(); + } + public slots: void start(const ConnectionData* connData); -- cgit v1.2.3 From d602de60433da80fc66a7152881d3dfe934eca62 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 28 Mar 2018 11:33:06 +0900 Subject: BaseJob: Process error 429 (Too Many Requests) The job will retry after the period either advised by the server or the default retry period. Closes #186. --- jobs/basejob.cpp | 24 ++++++++++++++++++++++++ jobs/basejob.h | 1 + 2 files changed, 25 insertions(+) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 5198c45c..37345338 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -263,7 +263,28 @@ void BaseJob::gotReply() { auto json = QJsonDocument::fromJson(d->reply->readAll()).object(); if (!json.isEmpty()) + { + if (error() == TooManyRequestsError) + { + QString msg = tr("Too many requests"); + auto retryInterval = json.value("retry_after_ms").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 the whole finishJob() + stop(); + qCWarning(d->logCat) << this << "will retry in" << retryInterval; + d->retryTimer.start(retryInterval); + emit retryScheduled(d->retriesTaken, retryInterval); + return; + } setStatus(IncorrectRequestError, json.value("error").toString()); + } } finishJob(); @@ -324,6 +345,9 @@ BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const return { NotFoundError, reply->errorString() }; default: + if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute) + .toInt() == 429) // Qt doesn't know about it yet + return { TooManyRequestsError, tr("Too many requests") }; return { NetworkError, reply->errorString() }; } } diff --git a/jobs/basejob.h b/jobs/basejob.h index fa253d96..ed630a67 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -62,6 +62,7 @@ namespace QMatrixClient , NotFoundError , IncorrectRequestError , IncorrectResponseError + , TooManyRequestsError , UserDefinedError = 200 }; -- cgit v1.2.3 From b2d22d7ce9d7235cf9ab61268e48102ff06cb727 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 28 Mar 2018 12:26:31 +0900 Subject: BaseJob: more improvements in logging and errors detection --- jobs/basejob.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 37345338..79cfb9e0 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -264,7 +264,8 @@ void BaseJob::gotReply() auto json = QJsonDocument::fromJson(d->reply->readAll()).object(); if (!json.isEmpty()) { - if (error() == TooManyRequestsError) + if (error() == TooManyRequestsError || + json.value("errcode").toString() == "M_LIMIT_EXCEEDED") { QString msg = tr("Too many requests"); auto retryInterval = json.value("retry_after_ms").toInt(-1); @@ -318,10 +319,16 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const { - qCDebug(d->logCat) << this << "returned" + const auto httpCode = + reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); + qCDebug(d->logCat).nospace().noquote() << this << " returned HTTP code " + << httpCode << ": " << (reply->error() == QNetworkReply::NoError ? "Success" : reply->errorString()) - << "from" << reply->url().toDisplayString(); + << " (URL: " << reply->url().toDisplayString() << ")"; + + // Should we check httpCode instead? Maybe even use it in BaseJob::Status? + // That would make codes logs slightly more readable. switch( reply->error() ) { case QNetworkReply::NoError: @@ -345,8 +352,7 @@ BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const return { NotFoundError, reply->errorString() }; default: - if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute) - .toInt() == 429) // Qt doesn't know about it yet + if (httpCode == 429) // Qt doesn't know about it yet return { TooManyRequestsError, tr("Too many requests") }; return { NetworkError, reply->errorString() }; } -- cgit v1.2.3 From 244fa53c10ee8697ff30e4135f6ac4cb4abb4506 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 28 Mar 2018 13:19:21 +0900 Subject: BaseJob: Dump error body (if there's any) to logs; detect error 429 more reliably --- jobs/basejob.cpp | 58 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 25 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 79cfb9e0..3cde7c50 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -261,30 +261,37 @@ void BaseJob::gotReply() setStatus(parseReply(d->reply.data())); else { - auto json = QJsonDocument::fromJson(d->reply->readAll()).object(); - if (!json.isEmpty()) + const auto body = d->reply->readAll(); + if (!body.isEmpty()) { - if (error() == TooManyRequestsError || - json.value("errcode").toString() == "M_LIMIT_EXCEEDED") - { - QString msg = tr("Too many requests"); - auto retryInterval = json.value("retry_after_ms").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 the whole finishJob() - stop(); - qCWarning(d->logCat) << this << "will retry in" << retryInterval; - d->retryTimer.start(retryInterval); - emit retryScheduled(d->retriesTaken, retryInterval); - return; + qCDebug(d->logCat).noquote() << "Error body:" << body; + auto json = QJsonDocument::fromJson(body).object(); + if (json.isEmpty()) + setStatus(IncorrectRequestError, body); + else { + if (error() == TooManyRequestsError || + json.value("errcode").toString() == "M_LIMIT_EXCEEDED") + { + QString msg = tr("Too many requests"); + auto retryInterval = json.value("retry_after_ms").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; + d->retryTimer.start(retryInterval); + emit retryScheduled(d->retriesTaken, retryInterval); + return; + } + setStatus(IncorrectRequestError, json.value("error").toString()); } - setStatus(IncorrectRequestError, json.value("error").toString()); } } @@ -327,8 +334,11 @@ BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const "Success" : reply->errorString()) << " (URL: " << reply->url().toDisplayString() << ")"; + if (httpCode == 429) // Qt doesn't know about it yet + return { TooManyRequestsError, tr("Too many requests") }; + // Should we check httpCode instead? Maybe even use it in BaseJob::Status? - // That would make codes logs slightly more readable. + // That would make codes in logs slightly more readable. switch( reply->error() ) { case QNetworkReply::NoError: @@ -352,8 +362,6 @@ BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const return { NotFoundError, reply->errorString() }; default: - if (httpCode == 429) // Qt doesn't know about it yet - return { TooManyRequestsError, tr("Too many requests") }; return { NetworkError, reply->errorString() }; } } -- cgit v1.2.3