diff options
author | Uwe Klotz <uklotz@mixxx.org> | 2021-01-08 19:51:24 +0100 |
---|---|---|
committer | Uwe Klotz <uklotz@mixxx.org> | 2021-01-08 23:26:45 +0100 |
commit | 39d6dd0968cd6d792b371cbcd64df9d0586f92d2 (patch) | |
tree | 3a54e657882ad4fdbe336e34fc3c11dfc45261d0 /src/network | |
parent | b98d3aad34ab3bae815264498f67daa98b5f3234 (diff) |
Move network error handling from NetworkTask into WebTask
...and provide the response content if available.
Diffstat (limited to 'src/network')
-rw-r--r-- | src/network/jsonwebtask.cpp | 58 | ||||
-rw-r--r-- | src/network/jsonwebtask.h | 2 | ||||
-rw-r--r-- | src/network/networktask.cpp | 23 | ||||
-rw-r--r-- | src/network/networktask.h | 14 | ||||
-rw-r--r-- | src/network/webtask.cpp | 93 | ||||
-rw-r--r-- | src/network/webtask.h | 69 |
6 files changed, 150 insertions, 109 deletions
diff --git a/src/network/jsonwebtask.cpp b/src/network/jsonwebtask.cpp index 690d31ef36..f33a7cea0f 100644 --- a/src/network/jsonwebtask.cpp +++ b/src/network/jsonwebtask.cpp @@ -37,26 +37,6 @@ const QString JSON_CONTENT_TYPE = "application/json"; const QMimeType JSON_MIME_TYPE = QMimeDatabase().mimeTypeForName(JSON_CONTENT_TYPE); -QMimeType readContentType( - const QNetworkReply* reply) { - DEBUG_ASSERT(reply); - const QVariant contentTypeHeader = reply->header(QNetworkRequest::ContentTypeHeader); - if (!contentTypeHeader.isValid() || contentTypeHeader.isNull()) { - kLogger.warning() - << "Missing content type header"; - return QMimeType(); - } - const QString contentTypeString = contentTypeHeader.toString(); - const QString contentTypeWithoutParams = contentTypeString.left(contentTypeString.indexOf(';')); - const QMimeType contentType = QMimeDatabase().mimeTypeForName(contentTypeWithoutParams); - if (!contentType.isValid()) { - kLogger.warning() - << "Unknown content type" - << contentTypeWithoutParams; - } - return contentType; -} - /// If parsing fails the functions returns std::nullopt and optionally /// the response content in pInvalidResponseContent for further processing. std::optional<QJsonDocument> readJsonContent( @@ -64,24 +44,28 @@ std::optional<QJsonDocument> readJsonContent( std::pair<QMimeType, QByteArray>* pInvalidResponseContent = nullptr) { DEBUG_ASSERT(reply); DEBUG_ASSERT(JSON_MIME_TYPE.isValid()); - QByteArray contentBytes = reply->readAll(); - const auto contentType = readContentType(reply); + auto contentType = WebTask::readContentType(*reply); + auto optContentData = WebTask::readContentData(reply); if (contentType != JSON_MIME_TYPE) { - kLogger.warning() - << "Unexpected content type" - << contentType; + kLogger.debug() + << "Received content type" + << contentType + << "instead of" + << JSON_MIME_TYPE; if (pInvalidResponseContent) { - *pInvalidResponseContent = std::make_pair(contentType, contentBytes); + *pInvalidResponseContent = std::make_pair( + std::move(contentType), + optContentData.value_or(QByteArray{})); } return std::nullopt; } - if (contentBytes.isEmpty()) { - kLogger.info() << "Empty JSON network reply"; + if (!optContentData || optContentData->isEmpty()) { + kLogger.debug() << "Empty JSON network reply"; return QJsonDocument{}; } QJsonParseError parseError; - const auto jsonDocument = QJsonDocument::fromJson( - contentBytes, + auto jsonDocument = QJsonDocument::fromJson( + *optContentData, &parseError); // QJsonDocument::fromJson() returns a non-null document // if parsing succeeds and otherwise null on error. The @@ -95,7 +79,9 @@ std::optional<QJsonDocument> readJsonContent( << "at offset" << parseError.offset; if (pInvalidResponseContent) { - *pInvalidResponseContent = std::make_pair(contentType, contentBytes); + *pInvalidResponseContent = std::make_pair( + std::move(contentType), + std::move(*optContentData)); } return std::nullopt; } @@ -120,7 +106,7 @@ QNetworkRequest newRequest( QDebug operator<<(QDebug dbg, const JsonWebResponse& arg) { return dbg - << "CustomWebResponse{" + << "WebResponseWithContent{" << arg.m_response << arg.m_content << '}'; @@ -148,7 +134,7 @@ void JsonWebTask::onFinished( } void JsonWebTask::onFinishedCustom( - CustomWebResponse&& customResponse) { + WebResponseWithContent&& customResponse) { kLogger.info() << this << "Received custom response" @@ -279,14 +265,14 @@ void JsonWebTask::doNetworkReplyFinished( std::optional<QJsonDocument> optJsonContent; auto webResponse = WebResponse{ finishedNetworkReply->url(), + finishedNetworkReply->request().url(), statusCode}; - if (statusCode != kHttpStatusCodeInvalid && - finishedNetworkReply->bytesAvailable() > 0) { + if (statusCode != kHttpStatusCodeInvalid) { std::pair<QMimeType, QByteArray> contentTypeAndBytes; optJsonContent = readJsonContent(finishedNetworkReply, &contentTypeAndBytes); if (!optJsonContent) { // Failed to read JSON content - onFinishedCustom(CustomWebResponse{ + onFinishedCustom(WebResponseWithContent{ std::move(webResponse), std::move(contentTypeAndBytes.first), std::move(contentTypeAndBytes.second)}); diff --git a/src/network/jsonwebtask.h b/src/network/jsonwebtask.h index 1bd22b2d74..cd3f89f137 100644 --- a/src/network/jsonwebtask.h +++ b/src/network/jsonwebtask.h @@ -98,7 +98,7 @@ class JsonWebTask : public WebTask { virtual void onFinished( JsonWebResponse&& response); virtual void onFinishedCustom( - CustomWebResponse&& response); + WebResponseWithContent&& response); QNetworkReply* doStartNetworkRequest( QNetworkAccessManager* networkAccessManager, diff --git a/src/network/networktask.cpp b/src/network/networktask.cpp index 96ec7b9b96..4aac5191bc 100644 --- a/src/network/networktask.cpp +++ b/src/network/networktask.cpp @@ -65,7 +65,7 @@ void NetworkTask::abort() { } void NetworkTask::emitAborted( - QUrl&& requestUrl) { + const QUrl& requestUrl) { VERIFY_OR_DEBUG_ASSERT( isSignalFuncConnected(&NetworkTask::aborted)) { kLogger.warning() @@ -78,27 +78,6 @@ void NetworkTask::emitAborted( emit aborted(requestUrl); } -void NetworkTask::emitNetworkError( - QUrl&& requestUrl, - QNetworkReply::NetworkError errorCode, - QString&& errorString) { - VERIFY_OR_DEBUG_ASSERT( - isSignalFuncConnected(&NetworkTask::networkError)) { - kLogger.warning() - << this - << "Unhandled network error signal" - << requestUrl - << errorCode - << errorString; - deleteLater(); - return; - } - emit networkError( - std::move(requestUrl), - errorCode, - std::move(errorString)); -} - } // namespace network } // namespace mixxx diff --git a/src/network/networktask.h b/src/network/networktask.h index 70e74953fc..410b977977 100644 --- a/src/network/networktask.h +++ b/src/network/networktask.h @@ -51,13 +51,7 @@ class NetworkTask : public QObject { /// Send an aborted signal with the optional request URL if available. void emitAborted( - QUrl&& requestUrl = QUrl{}); - - /// Send a signal after an aborted/timed out/failed network response. - void emitNetworkError( - QUrl&& requestUrl, - QNetworkReply::NetworkError errorCode, - QString&& errorString); + const QUrl& requestUrl = QUrl{}); /// All member variables must only be accessed from /// the event loop thread!! @@ -95,12 +89,6 @@ class NetworkTask : public QObject { /// Client-side abort void aborted( const QUrl& requestUrl); - - /// Network or server-side abort/timeout/failure - void networkError( - const QUrl& requestUrl, - QNetworkReply::NetworkError errorCode, - const QString& errorString); }; } // namespace network diff --git a/src/network/webtask.cpp b/src/network/webtask.cpp index 6de7dcc9b1..8ddcf4b866 100644 --- a/src/network/webtask.cpp +++ b/src/network/webtask.cpp @@ -1,5 +1,6 @@ #include "network/webtask.h" +#include <QMimeDatabase> #include <QTimerEvent> #include <mutex> // std::once_flag @@ -21,7 +22,7 @@ std::once_flag registerMetaTypesOnceFlag; void registerMetaTypesOnce() { WebResponse::registerMetaType(); - CustomWebResponse::registerMetaType(); + WebResponseWithContent::registerMetaType(); } int readStatusCode( @@ -48,24 +49,54 @@ int readStatusCode( QDebug operator<<(QDebug dbg, const WebResponse& arg) { return dbg << "WebResponse{" + << arg.m_requestUrl << arg.m_replyUrl << arg.m_statusCode << '}'; } -/*static*/ void CustomWebResponse::registerMetaType() { - qRegisterMetaType<CustomWebResponse>("mixxx::network::CustomWebResponse"); +/*static*/ void WebResponseWithContent::registerMetaType() { + qRegisterMetaType<WebResponseWithContent>("mixxx::network::WebResponseWithContent"); } -QDebug operator<<(QDebug dbg, const CustomWebResponse& arg) { +QDebug operator<<(QDebug dbg, const WebResponseWithContent& arg) { return dbg - << "CustomWebResponse{" + << "WebResponseWithContent{" << arg.m_response << arg.m_contentType - << arg.m_contentBytes + << arg.m_contentData << '}'; } +//static +QMimeType WebTask::readContentType( + const QNetworkReply& reply) { + const QVariant contentTypeHeader = reply.header(QNetworkRequest::ContentTypeHeader); + if (!contentTypeHeader.isValid() || contentTypeHeader.isNull()) { + kLogger.warning() + << "Missing content type header"; + return QMimeType(); + } + const QString contentTypeString = contentTypeHeader.toString(); + const QString contentTypeWithoutParams = contentTypeString.left(contentTypeString.indexOf(';')); + const QMimeType contentType = QMimeDatabase().mimeTypeForName(contentTypeWithoutParams); + if (!contentType.isValid()) { + kLogger.warning() + << "Unknown content type" + << contentTypeWithoutParams; + } + return contentType; +} + +//static +std::optional<QByteArray> WebTask::readContentData( + QNetworkReply* reply) { + if (!reply->isReadable()) { + return std::nullopt; + } + return reply->readAll(); +} + WebTask::WebTask( QNetworkAccessManager* networkAccessManager, QObject* parent) @@ -76,9 +107,9 @@ WebTask::WebTask( } void WebTask::onNetworkError( - QUrl&& requestUrl, QNetworkReply::NetworkError errorCode, - QString&& errorString) { + const QString& errorString, + const WebResponseWithContent& responseWithContent) { DEBUG_ASSERT(m_state == State::Pending); DEBUG_ASSERT(m_timeoutTimerId == kInvalidTimerId); @@ -98,15 +129,36 @@ void WebTask::onNetworkError( DEBUG_ASSERT(hasTerminated()); if (m_state == State::Aborted) { - emitAborted(std::move(requestUrl)); + emitAborted(responseWithContent.replyUrl()); } else { emitNetworkError( - std::move(requestUrl), errorCode, - std::move(errorString)); + errorString, + responseWithContent); } } +void WebTask::emitNetworkError( + QNetworkReply::NetworkError errorCode, + const QString& errorString, + const WebResponseWithContent& responseWithContent) { + VERIFY_OR_DEBUG_ASSERT( + isSignalFuncConnected(&WebTask::networkError)) { + kLogger.warning() + << this + << "Unhandled network error signal" + << errorCode + << errorString + << responseWithContent; + deleteLater(); + return; + } + emit networkError( + errorCode, + errorString, + responseWithContent); +} + void WebTask::slotStart(int timeoutMillis) { DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); if (m_state == State::Pending) { @@ -124,9 +176,9 @@ void WebTask::slotStart(int timeoutMillis) { VERIFY_OR_DEBUG_ASSERT(pNetworkAccessManager) { m_state = State::Pending; onNetworkError( - QUrl(), QNetworkReply::NetworkSessionFailedError, - tr("No network access")); + tr("No network access"), + WebResponseWithContent{}); return; } DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(pNetworkAccessManager); @@ -230,7 +282,7 @@ void WebTask::slotAbort() { } m_state = State::Aborted; - emitAborted(std::move(requestUrl)); + emitAborted(requestUrl); } void WebTask::timerEvent(QTimerEvent* event) { @@ -310,17 +362,24 @@ void WebTask::slotNetworkReplyFinished() { m_timeoutTimerId = kInvalidTimerId; } + const auto statusCode = readStatusCode(pFinishedNetworkReply); if (pFinishedNetworkReply->error() != QNetworkReply::NetworkError::NoError) { onNetworkError( - pFinishedNetworkReply->request().url(), pFinishedNetworkReply->error(), - pFinishedNetworkReply->errorString()); + pFinishedNetworkReply->errorString(), + WebResponseWithContent{ + WebResponse{ + pFinishedNetworkReply->url(), + pFinishedNetworkReply->request().url(), + statusCode}, + readContentType(*pFinishedNetworkReply), + readContentData(pFinishedNetworkReply).value_or(QByteArray{}), + }); DEBUG_ASSERT(hasTerminated()); return; } m_state = State::Finished; - const auto statusCode = readStatusCode(pFinishedNetworkReply); doNetworkReplyFinished(pFinishedNetworkReply, statusCode); } diff --git a/src/network/webtask.h b/src/network/webtask.h index 7d364c1afb..ae2e8ed654 100644 --- a/src/network/webtask.h +++ b/src/network/webtask.h @@ -23,9 +23,11 @@ class WebResponse final { : m_statusCode(kHttpStatusCodeInvalid) { } explicit WebResponse( - QUrl replyUrl, + QUrl requestUrl, + QUrl replyUrl = QUrl(), HttpStatusCode statusCode = kHttpStatusCodeInvalid) - : m_replyUrl(std::move(replyUrl)), + : m_requestUrl(std::move(requestUrl)), + m_replyUrl(std::move(replyUrl)), m_statusCode(statusCode) { } WebResponse(const WebResponse&) = default; @@ -38,39 +40,44 @@ class WebResponse final { return HttpStatusCode_isSuccess(m_statusCode); } - HttpStatusCode statusCode() const { - return m_statusCode; + const QUrl& requestUrl() const { + return m_requestUrl; } const QUrl& replyUrl() const { return m_replyUrl; } + HttpStatusCode statusCode() const { + return m_statusCode; + } + friend QDebug operator<<(QDebug dbg, const WebResponse& arg); private: + QUrl m_requestUrl; QUrl m_replyUrl; HttpStatusCode m_statusCode; }; -class CustomWebResponse final { +class WebResponseWithContent final { public: static void registerMetaType(); - CustomWebResponse() = default; - CustomWebResponse( + WebResponseWithContent() = default; + WebResponseWithContent( WebResponse&& response, QMimeType&& contentType, - QByteArray&& contentBytes) + QByteArray&& contentData) : m_response(std::move(response)), m_contentType(std::move(contentType)), - m_contentBytes(std::move(contentBytes)) { + m_contentData(std::move(contentData)) { } - CustomWebResponse(const CustomWebResponse&) = default; - CustomWebResponse(CustomWebResponse&&) = default; + WebResponseWithContent(const WebResponseWithContent&) = default; + WebResponseWithContent(WebResponseWithContent&&) = default; - CustomWebResponse& operator=(const CustomWebResponse&) = default; - CustomWebResponse& operator=(CustomWebResponse&&) = default; + WebResponseWithContent& operator=(const WebResponseWithContent&) = default; + WebResponseWithContent& operator=(WebResponseWithContent&&) = default; bool isStatusCodeSuccess() const { return m_response.isStatusCodeSuccess(); @@ -80,6 +87,10 @@ class CustomWebResponse final { return m_response.statusCode(); } + const QUrl& requestUrl() const { + return m_response.requestUrl(); + } + const QUrl& replyUrl() const { return m_response.replyUrl(); } @@ -88,16 +99,16 @@ class CustomWebResponse final { return m_contentType; } - const QByteArray& contentBytes() const { - return m_contentBytes; + const QByteArray& contentData() const { + return m_contentData; } - friend QDebug operator<<(QDebug dbg, const CustomWebResponse& arg); + friend QDebug operator<<(QDebug dbg, const WebResponseWithContent& arg); private: WebResponse m_response; QMimeType m_contentType; - QByteArray m_contentBytes; + QByteArray m_contentData; }; /// A transient task for performing a single HTTP network request @@ -106,11 +117,23 @@ class WebTask : public NetworkTask { Q_OBJECT public: + static QMimeType readContentType( + const QNetworkReply& reply); + static std::optional<QByteArray> readContentData( + QNetworkReply* reply); + explicit WebTask( QNetworkAccessManager* networkAccessManager, QObject* parent = nullptr); ~WebTask() override = default; + signals: + /// Network or server-side abort/timeout/failure + void networkError( + QNetworkReply::NetworkError errorCode, + const QString& errorString, + const mixxx::network::WebResponseWithContent& responseWithContent); + public slots: void slotStart( int timeoutMillis) override; @@ -145,6 +168,12 @@ class WebTask : public NetworkTask { state() == State::Finished; } + /// Send a signal after a failed network response. + void emitNetworkError( + QNetworkReply::NetworkError errorCode, + const QString& errorString, + const WebResponseWithContent& responseWithContent); + private: QUrl abortPendingNetworkReply(); @@ -169,9 +198,9 @@ class WebTask : public NetworkTask { /// gets deleted. The default implementation logs a warning /// and deletes the task. void onNetworkError( - QUrl&& requestUrl, QNetworkReply::NetworkError errorCode, - QString&& errorString); + const QString& errorString, + const WebResponseWithContent& responseWithContent); /// All member variables must only be accessed from /// the event loop thread!! @@ -191,4 +220,4 @@ class WebTask : public NetworkTask { Q_DECLARE_METATYPE(mixxx::network::WebResponse); -Q_DECLARE_METATYPE(mixxx::network::CustomWebResponse); +Q_DECLARE_METATYPE(mixxx::network::WebResponseWithContent); |