Merge pull request #3442 from uklotzde/webtask

WebTask: Reduce code duplication and simplify timeout handling

7 files changed, 213 insertions, 369 deletions

diff --git a/src/musicbrainz/tagfetcher.cpp b/src/musicbrainz/tagfetcher.cpp
index 37364e564f..d347c8e211 100644
--- a/src/musicbrainz/tagfetcher.cpp
+++ b/src/musicbrainz/tagfetcher.cpp
@@ -10,9 +10,11 @@
 
 namespace {
 
-constexpr int kAcoustIdTimeoutMillis = 5000; // msec
+// Long timeout to cope with occasional server-side unresponsiveness
+constexpr int kAcoustIdTimeoutMillis = 60000; // msec
 
-constexpr int kMusicBrainzTimeoutMillis = 5000; // msec
+// Long timeout to cope with occasional server-side unresponsiveness
+constexpr int kMusicBrainzTimeoutMillis = 60000; // msec
 
 } // anonymous namespace
 
diff --git a/src/musicbrainz/web/musicbrainzrecordingstask.cpp b/src/musicbrainz/web/musicbrainzrecordingstask.cpp
index 864b1a8695..5afec1225e 100644
--- a/src/musicbrainz/web/musicbrainzrecordingstask.cpp
+++ b/src/musicbrainz/web/musicbrainzrecordingstask.cpp
@@ -76,28 +76,18 @@ MusicBrainzRecordingsTask::MusicBrainzRecordingsTask(
     musicbrainz::registerMetaTypesOnce();
 }
 
-MusicBrainzRecordingsTask::~MusicBrainzRecordingsTask() {
-    VERIFY_OR_DEBUG_ASSERT(!m_pendingNetworkReply) {
-        m_pendingNetworkReply->deleteLater();
-    }
-}
-
-bool MusicBrainzRecordingsTask::doStart(
+QNetworkReply* MusicBrainzRecordingsTask::doStartNetworkRequest(
         QNetworkAccessManager* networkAccessManager,
         int parentTimeoutMillis) {
-    m_parentTimeoutMillis = parentTimeoutMillis;
     DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
     DEBUG_ASSERT(networkAccessManager);
-    VERIFY_OR_DEBUG_ASSERT(!m_pendingNetworkReply) {
-        kLogger.warning()
-                << "Task has already been started";
-        return false;
-    }
+
+    m_parentTimeoutMillis = parentTimeoutMillis;
 
     VERIFY_OR_DEBUG_ASSERT(!m_queuedRecordingIds.isEmpty()) {
         kLogger.warning()
                 << "Nothing to do";
-        return false;
+        return nullptr;
     }
     const auto recordingId = m_queuedRecordingIds.takeFirst();
     DEBUG_ASSERT(!recordingId.isNull());
@@ -110,73 +100,24 @@ bool MusicBrainzRecordingsTask::doStart(
                 << "GET"
                 << networkRequest.url();
     }
-    m_pendingNetworkReply =
-            networkAccessManager->get(networkRequest);
-    VERIFY_OR_DEBUG_ASSERT(m_pendingNetworkReply) {
-        kLogger.warning()
-                << "Request not sent";
-        return false;
-    }
-
-    // It is not necessary to connect the QNetworkReply::errorOccurred signal.
-    // Network errors are also received through the QNetworkReply::finished signal.
-    connect(m_pendingNetworkReply,
-            &QNetworkReply::finished,
-            this,
-            &MusicBrainzRecordingsTask::slotNetworkReplyFinished,
-            Qt::UniqueConnection);
-
-    return true;
+    return networkAccessManager->get(networkRequest);
 }
 
-QUrl MusicBrainzRecordingsTask::doAbort() {
-    QUrl requestUrl;
-    if (m_pendingNetworkReply) {
-        requestUrl = abortPendingNetworkReply(m_pendingNetworkReply);
-        if (requestUrl.isValid()) {
-            // Already finished
-            m_pendingNetworkReply->deleteLater();
-            m_pendingNetworkReply = nullptr;
-        }
-    }
-    return requestUrl;
-}
-
-QUrl MusicBrainzRecordingsTask::doTimeOut() {
+void MusicBrainzRecordingsTask::doNetworkReplyFinished(
+        QNetworkReply* finishedNetworkReply,
+        network::HttpStatusCode statusCode) {
     DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    QUrl requestUrl;
-    if (m_pendingNetworkReply) {
-        requestUrl = timeOutPendingNetworkReply(m_pendingNetworkReply);
-        // Don't wait until finished
-        m_pendingNetworkReply->deleteLater();
-        m_pendingNetworkReply = nullptr;
-    }
-    return requestUrl;
-}
-
-void MusicBrainzRecordingsTask::slotNetworkReplyFinished() {
-    DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    const QPair<QNetworkReply*, network::HttpStatusCode>
-            networkReplyWithStatusCode = receiveNetworkReply();
-    auto* const networkReply = networkReplyWithStatusCode.first;
-    if (!networkReply) {
-        // already aborted
-        return;
-    }
-    const auto statusCode = networkReplyWithStatusCode.second;
-    VERIFY_OR_DEBUG_ASSERT(networkReply == m_pendingNetworkReply) {
-        return;
-    }
-    m_pendingNetworkReply = nullptr;
 
-    const QByteArray body = networkReply->readAll();
+    const QByteArray body = finishedNetworkReply->readAll();
     QXmlStreamReader reader(body);
 
     // HTTP status of successful results:
     // 200: Found
     // 301: Found, but UUID moved permanently in database
     // 404: Not found in database, i.e. empty result
-    if (statusCode != 200 && statusCode != 301 && statusCode != 404) {
+    if (statusCode != 200 &&
+            statusCode != 301 &&
+            statusCode != 404) {
         kLogger.info()
                 << "GET reply"
                 << "statusCode:" << statusCode
@@ -184,7 +125,7 @@ void MusicBrainzRecordingsTask::slotNetworkReplyFinished() {
         auto error = musicbrainz::Error(reader);
         emitFailed(
                 network::WebResponse(
-                        networkReply->url(),
+                        finishedNetworkReply->url(),
                         statusCode),
                 error.code,
                 std::move(error.message));
@@ -205,7 +146,7 @@ void MusicBrainzRecordingsTask::slotNetworkReplyFinished() {
                 << "Failed to parse XML response";
         emitFailed(
                 network::WebResponse(
-                        networkReply->url(),
+                        finishedNetworkReply->url(),
                         statusCode),
                 -1,
                 "Failed to parse XML response");
diff --git a/src/musicbrainz/web/musicbrainzrecordingstask.h b/src/musicbrainz/web/musicbrainzrecordingstask.h
index 953298eb4c..39678d65ff 100644
--- a/src/musicbrainz/web/musicbrainzrecordingstask.h
+++ b/src/musicbrainz/web/musicbrainzrecordingstask.h
@@ -19,7 +19,7 @@ class MusicBrainzRecordingsTask : public network::WebTask {
             QNetworkAccessManager* networkAccessManager,
             QList<QUuid>&& recordingIds,
             QObject* parent = nullptr);
-    ~MusicBrainzRecordingsTask() override;
+    ~MusicBrainzRecordingsTask() override = default;
 
   signals:
     void succeeded(
@@ -29,15 +29,13 @@ class MusicBrainzRecordingsTask : public network::WebTask {
             int errorCode,
             const QString& errorMessage);
 
-  private slots:
-    void slotNetworkReplyFinished();
-
   private:
-    bool doStart(
+    QNetworkReply* doStartNetworkRequest(
             QNetworkAccessManager* networkAccessManager,
             int parentTimeoutMillis) override;
-    QUrl doAbort() override;
-    QUrl doTimeOut() override;
+    void doNetworkReplyFinished(
+            QNetworkReply* finishedNetworkReply,
+            network::HttpStatusCode statusCode) override;
 
     void emitSucceeded(
             QList<musicbrainz::TrackRelease>&& trackReleases);
@@ -55,7 +53,6 @@ class MusicBrainzRecordingsTask : public network::WebTask {
 
     QMap<QUuid, musicbrainz::TrackRelease> m_trackReleases;
 
-    QPointer<QNetworkReply> m_pendingNetworkReply;
     int m_parentTimeoutMillis;
 };
 
diff --git a/src/network/jsonwebtask.cpp b/src/network/jsonwebtask.cpp
index bac64db6bc..0e355db369 100644
--- a/src/network/jsonwebtask.cpp
+++ b/src/network/jsonwebtask.cpp
@@ -128,18 +128,11 @@ JsonWebTask::JsonWebTask(
         QObject* parent)
         : WebTask(networkAccessManager, parent),
           m_baseUrl(baseUrl),
-          m_request(std::move(request)),
-          m_pendingNetworkReply(nullptr) {
+          m_request(std::move(request)) {
     std::call_once(registerMetaTypesOnceFlag, registerMetaTypesOnce);
     DEBUG_ASSERT(!m_baseUrl.isEmpty());
 }
 
-JsonWebTask::~JsonWebTask() {
-    if (m_pendingNetworkReply) {
-        m_pendingNetworkReply->deleteLater();
-    }
-}
-
 void JsonWebTask::onFinished(
         JsonWebResponse&& response) {
     kLogger.info()
@@ -244,18 +237,12 @@ QNetworkReply* JsonWebTask::sendNetworkRequest(
     return nullptr;
 }
 
-bool JsonWebTask::doStart(
+QNetworkReply* JsonWebTask::doStartNetworkRequest(
         QNetworkAccessManager* networkAccessManager,
         int parentTimeoutMillis) {
     Q_UNUSED(parentTimeoutMillis);
     DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
     DEBUG_ASSERT(networkAccessManager);
-    VERIFY_OR_DEBUG_ASSERT(!m_pendingNetworkReply) {
-        kLogger.warning()
-                << this
-                << "Task has already been started";
-        return false;
-    }
 
     DEBUG_ASSERT(m_baseUrl.isValid());
     QUrl url = m_baseUrl;
@@ -265,83 +252,29 @@ bool JsonWebTask::doStart(
     }
     DEBUG_ASSERT(url.isValid());
 
-    m_pendingNetworkReply = sendNetworkRequest(
+    return sendNetworkRequest(
             networkAccessManager,
             m_request.method,
             url,
             m_request.content);
-    VERIFY_OR_DEBUG_ASSERT(m_pendingNetworkReply) {
-        kLogger.warning()
-                << this
-                << "Request not sent";
-        return false;
-    }
-
-    // It is not necessary to connect the QNetworkReply::errorOccurred signal.
-    // Network errors are also received through the QNetworkReply::finished signal.
-    connect(m_pendingNetworkReply,
-            &QNetworkReply::finished,
-            this,
-            &JsonWebTask::slotNetworkReplyFinished,
-            Qt::UniqueConnection);
-
-    return true;
 }
 
-QUrl JsonWebTask::doAbort() {
-    DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    QUrl requestUrl;
-    if (m_pendingNetworkReply) {
-        requestUrl = abortPendingNetworkReply(m_pendingNetworkReply);
-        if (requestUrl.isValid()) {
-            // Already finished
-            m_pendingNetworkReply->deleteLater();
-            m_pendingNetworkReply = nullptr;
-        }
-    }
-    return requestUrl;
-}
-
-QUrl JsonWebTask::doTimeOut() {
-    DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    QUrl requestUrl;
-    if (m_pendingNetworkReply) {
-        requestUrl = timeOutPendingNetworkReply(m_pendingNetworkReply);
-        // Don't wait until finished
-        m_pendingNetworkReply->deleteLater();
-        m_pendingNetworkReply = nullptr;
-    }
-    return requestUrl;
-}
-
-void JsonWebTask::slotNetworkReplyFinished() {
-    DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    const QPair<QNetworkReply*, HttpStatusCode> networkReplyWithStatusCode =
-            receiveNetworkReply();
-    auto* const networkReply = networkReplyWithStatusCode.first;
-    if (!networkReply) {
-        // already aborted
-        return;
-    }
-    const auto statusCode = networkReplyWithStatusCode.second;
-    VERIFY_OR_DEBUG_ASSERT(networkReply == m_pendingNetworkReply) {
-        return;
-    }
-    m_pendingNetworkReply = nullptr;
-
+void JsonWebTask::doNetworkReplyFinished(
+        QNetworkReply* finishedNetworkReply,
+        HttpStatusCode statusCode) {
     QJsonDocument content;
     if (statusCode != kHttpStatusCodeInvalid &&
-            networkReply->bytesAvailable() > 0 &&
-            !readJsonContent(networkReply, &content)) {
+            finishedNetworkReply->bytesAvailable() > 0 &&
+            !readJsonContent(finishedNetworkReply, &content)) {
         onFinishedCustom(CustomWebResponse{
                 WebResponse{
-                        networkReply->url(),
+                        finishedNetworkReply->url(),
                         statusCode},
-                networkReply->readAll()});
+                finishedNetworkReply->readAll()});
     } else {
         onFinished(JsonWebResponse{
                 WebResponse{
-                        networkReply->url(),
+                        finishedNetworkReply->url(),
                         statusCode},
                 std::move(content)});
     }
diff --git a/src/network/jsonwebtask.h b/src/network/jsonwebtask.h
index ca83ccea30..544fd97760 100644
--- a/src/network/jsonwebtask.h
+++ b/src/network/jsonwebtask.h
@@ -57,15 +57,12 @@ class JsonWebTask : public WebTask {
             const QUrl& baseUrl,
             JsonWebRequest&& request,
             QObject* parent = nullptr);
-    ~JsonWebTask() override;
+    ~JsonWebTask() override = default;
 
   signals:
     void failed(
             const network::JsonWebResponse& response);
 
-  private slots:
-    void slotNetworkReplyFinished();
-
   protected:
     // Customizable in derived classes
     virtual QNetworkReply* sendNetworkRequest(
@@ -86,18 +83,17 @@ class JsonWebTask : public WebTask {
     virtual void onFinishedCustom(
             CustomWebResponse&& response);
 
-    bool doStart(
+    QNetworkReply* doStartNetworkRequest(
             QNetworkAccessManager* networkAccessManager,
             int parentTimeoutMillis) override;
-    QUrl doAbort() override;
-    QUrl doTimeOut() override;
+    void doNetworkReplyFinished(
+            QNetworkReply* finishedNetworkReply,
+            HttpStatusCode statusCode) override;
 
     // All member variables must only be accessed from
     // the event loop thread!!
     const QUrl m_baseUrl;
     const JsonWebRequest m_request;
-
-    QPointer<QNetworkReply> m_pendingNetworkReply;
 };
 
 } // namespace network
diff --git a/src/network/webtask.cpp b/src/network/webtask.cpp
index b1481df2d6..cc41984431 100644
--- a/src/network/webtask.cpp
+++ b/src/network/webtask.cpp
@@ -30,22 +30,19 @@ void registerMetaTypesOnce() {
     CustomWebResponse::registerMetaType();
 }
 
-bool readStatusCode(
-        const QNetworkReply* reply,
-        int* statusCode) {
-    DEBUG_ASSERT(statusCode);
-    const QVariant statusCodeAttr = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute);
+int readStatusCode(
+        const QNetworkReply* networkReply) {
+    const QVariant statusCodeAttr =
+            networkReply->attribute(QNetworkRequest::HttpStatusCodeAttribute);
     bool statusCodeValid = false;
-    const int statusCodeValue = statusCodeAttr.toInt(&statusCodeValid);
-    VERIFY_OR_DEBUG_ASSERT(statusCodeValid && HttpStatusCode_isValid(statusCodeValue)) {
+    const int statusCode = statusCodeAttr.toInt(&statusCodeValid);
+    VERIFY_OR_DEBUG_ASSERT(statusCodeValid && HttpStatusCode_isValid(statusCode)) {
         kLogger.warning()
                 << "Invalid or missing status code attribute"
                 << statusCodeAttr;
+        return kHttpStatusCodeInvalid;
     }
-    else {
-        *statusCode = statusCodeValue;
-    }
-    return statusCodeValid;
+    return statusCode;
 }
 
 } // anonymous namespace
@@ -56,10 +53,10 @@ bool readStatusCode(
 
 QDebug operator<<(QDebug dbg, const WebResponse& arg) {
     return dbg
-        << "WebResponse{"
-        << arg.replyUrl
-        << arg.statusCode
-        << '}';
+            << "WebResponse{"
+            << arg.replyUrl
+            << arg.statusCode
+            << '}';
 }
 
 /*static*/ void CustomWebResponse::registerMetaType() {
@@ -68,10 +65,10 @@ QDebug operator<<(QDebug dbg, const WebResponse& arg) {
 
 QDebug operator<<(QDebug dbg, const CustomWebResponse& arg) {
     return dbg
-        << "CustomWebResponse{"
-        << static_cast<const WebResponse&>(arg)
-        << arg.content
-        << '}';
+            << "CustomWebResponse{"
+            << static_cast<const WebResponse&>(arg)
+            << arg.content
+            << '}';
 }
 
 WebTask::WebTask(
@@ -80,7 +77,7 @@ WebTask::WebTask(
         : QObject(parent),
           m_networkAccessManager(networkAccessManager),
           m_timeoutTimerId(kInvalidTimerId),
-          m_status(Status::Idle) {
+          m_state(State::Idle) {
     std::call_once(registerMetaTypesOnceFlag, registerMetaTypesOnce);
     DEBUG_ASSERT(m_networkAccessManager);
     s_instanceCounter.increment(1);
@@ -90,56 +87,28 @@ WebTask::~WebTask() {
     s_instanceCounter.increment(-1);
 }
 
-void WebTask::onAborted(
-        QUrl&& requestUrl) {
-    VERIFY_OR_DEBUG_ASSERT(m_status == Status::Aborting) {
-        return;
-    }
-    m_status = Status::Aborted;
-    VERIFY_OR_DEBUG_ASSERT(
-            isSignalFuncConnected(&WebTask::aborted)) {
-        kLogger.warning()
-                << this
-                << "Unhandled abort signal"
-                << requestUrl;
-        deleteLater();
-        return;
-    }
-    emit aborted(
-            std::move(requestUrl));
-}
-
-void WebTask::onTimedOut(
-        QUrl&& requestUrl) {
-    VERIFY_OR_DEBUG_ASSERT(m_status == Status::Pending) {
-        return;
-    }
-    if (m_timeoutTimerId != kInvalidTimerId) {
-        killTimer(m_timeoutTimerId);
-        m_timeoutTimerId = kInvalidTimerId;
-    }
-    onNetworkError(
-            std::move(requestUrl),
-            QNetworkReply::TimeoutError,
-            tr("Client-side network timeout"),
-            QByteArray());
-}
-
 void WebTask::onNetworkError(
         QUrl&& requestUrl,
         QNetworkReply::NetworkError errorCode,
         QString&& errorString,
         QByteArray&& errorContent) {
     DEBUG_ASSERT(m_timeoutTimerId == kInvalidTimerId);
-    VERIFY_OR_DEBUG_ASSERT(m_status == Status::Pending) {
+    VERIFY_OR_DEBUG_ASSERT(m_state == State::Pending) {
         return;
     }
+
     DEBUG_ASSERT(errorCode != QNetworkReply::NoError);
-    if (errorCode == QNetworkReply::TimeoutError) {
-        m_status = Status::TimedOut;
-    } else {
-        m_status = Status::Failed;
+    switch (errorCode) {
+    case QNetworkReply::OperationCanceledError:
+        m_state = State::Aborted;
+        break;
+    case QNetworkReply::TimeoutError:
+        m_state = State::TimedOut;
+        break;
+    default:
+        m_state = State::Failed;
     }
+
     VERIFY_OR_DEBUG_ASSERT(
             isSignalFuncConnected(&WebTask::networkError)) {
         kLogger.warning()
@@ -189,13 +158,14 @@ void WebTask::invokeAbort() {
 
 void WebTask::slotStart(int timeoutMillis) {
     DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    VERIFY_OR_DEBUG_ASSERT(m_status != Status::Pending) {
+    VERIFY_OR_DEBUG_ASSERT(m_state != State::Pending) {
         return;
     }
-    m_status = Status::Idle;
+    DEBUG_ASSERT(!m_pendingNetworkReply);
+    m_state = State::Idle;
 
     VERIFY_OR_DEBUG_ASSERT(m_networkAccessManager) {
-        m_status = Status::Pending;
+        m_state = State::Pending;
         onNetworkError(
                 QUrl(),
                 QNetworkReply::NetworkSessionFailedError,
@@ -207,73 +177,74 @@ void WebTask::slotStart(int timeoutMillis) {
     kLogger.debug()
             << this
             << "Starting...";
-    if (!doStart(m_networkAccessManager, timeoutMillis)) {
-        // Still idle, because we are in the same thread.
-        // The callee is not supposed to abort a request
-        // before it has beeen started successfully.
-        DEBUG_ASSERT(m_status == Status::Idle);
-        m_status = Status::Pending;
-        onNetworkError(
-                QUrl(),
-                QNetworkReply::OperationCanceledError,
-                tr("Start of network task has been aborted"),
-                QByteArray());
+
+    m_pendingNetworkReply = doStartNetworkRequest(
+            m_networkAccessManager,
+            timeoutMillis);
+    // Still idle, because we are in the same thread.
+    // The callee is not supposed to abort a request
+    // before it has beeen started successfully.
+    DEBUG_ASSERT(m_state == State::Idle);
+    if (!m_pendingNetworkReply) {
+        kLogger.debug()
+                << "Network task has not been started";
         return;
     }
-    // Still idle after the request has been started
-    // successfully, i.e. nothing happened yet in this
-    // thread.
-    DEBUG_ASSERT(m_status == Status::Idle);
-    m_status = Status::Pending;
+    m_state = State::Pending;
 
     DEBUG_ASSERT(m_timeoutTimerId == kInvalidTimerId);
     if (timeoutMillis > 0) {
         m_timeoutTimerId = startTimer(timeoutMillis);
         DEBUG_ASSERT(m_timeoutTimerId != kInvalidTimerId);
     }
-}
 
-QUrl WebTask::abortPendingNetworkReply(
-        QNetworkReply* pendingNetworkReply) {
-    DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    DEBUG_ASSERT(pendingNetworkReply);
-    if (pendingNetworkReply->isRunning()) {
-        pendingNetworkReply->abort();
-        // Suspend until finished
-        return QUrl();
-    }
-    return pendingNetworkReply->request().url();
+    // It is not necessary to connect the QNetworkReply::errorOccurred signal.
+    // Network errors are also received through the QNetworkReply::finished signal.
+    connect(m_pendingNetworkReply,
+            &QNetworkReply::finished,
+            this,
+            &WebTask::slotNetworkReplyFinished,
+            Qt::UniqueConnection);
 }
 
-QUrl WebTask::timeOutPendingNetworkReply(
-        QNetworkReply* pendingNetworkReply) {
+void WebTask::abort() {
     DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    DEBUG_ASSERT(pendingNetworkReply);
-    if (pendingNetworkReply->isRunning()) {
-        //pendingNetworkReply->abort();
-        // Don't suspend until finished, i.e. abort and then
-        // delete the pending network request instantly
+    if (m_state != State::Pending) {
+        DEBUG_ASSERT(m_timeoutTimerId == kInvalidTimerId);
+        return;
     }
-    return pendingNetworkReply->request().url();
-}
-
-QUrl WebTask::abort() {
-    DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    if (m_status != Status::Pending) {
+    VERIFY_OR_DEBUG_ASSERT(m_pendingNetworkReply) {
+        return;
         DEBUG_ASSERT(m_timeoutTimerId == kInvalidTimerId);
-        return QUrl();
     }
     if (m_timeoutTimerId != kInvalidTimerId) {
         killTimer(m_timeoutTimerId);
         m_timeoutTimerId = kInvalidTimerId;
     }
-    m_status = Status::Aborting;
+    m_state = State::Aborting;
     kLogger.debug()
             << this
             << "Aborting...";
-    QUrl url = doAbort();
-    onAborted(QUrl(url));
-    return url;
+    if (m_pendingNetworkReply->isRunning()) {
+        m_pendingNetworkReply->abort();
+        doNetworkReplyAborted(m_pendingNetworkReply);
+        // Suspend and await finished signal
+        return;
+    }
+    doNetworkReplyAborted(m_pendingNetworkReply);
+    m_state = State::Aborted;
+    const auto requestUrl = m_pendingNetworkReply->request().url();
+    VERIFY_OR_DEBUG_ASSERT(
+            isSignalFuncConnected(&WebTask::aborted)) {
+        kLogger.warning()
+                << this
+                << "Unhandled abort signal"
+                << requestUrl;
+        deleteLater();
+        return;
+    }
+    emit aborted(
+            std::move(requestUrl));
 }
 
 void WebTask::slotAbort() {
@@ -289,54 +260,67 @@ void WebTask::timerEvent(QTimerEvent* event) {
     }
     killTimer(m_timeoutTimerId);
     m_timeoutTimerId = kInvalidTimerId;
-    VERIFY_OR_DEBUG_ASSERT(m_status == Status::Pending) {
+    VERIFY_OR_DEBUG_ASSERT(m_state == State::Pending) {
+        return;
+    }
+    VERIFY_OR_DEBUG_ASSERT(m_pendingNetworkReply) {
         return;
     }
+    if (m_pendingNetworkReply->isFinished()) {
+        // Nothing to do
+    }
     kLogger.info()
             << this
-            << "Timed out";
-    onTimedOut(doTimeOut());
+            << "Aborting after timed out";
+    DEBUG_ASSERT(m_pendingNetworkReply->isRunning());
+    m_pendingNetworkReply->abort();
+    // Aborting the network reply might finish it
+    // immediately. It will be destroyed when handling
+    // the finished signal, i.e. m_pendingNetworkReply
+    // could be nullptr here!
 }
 
-QPair<QNetworkReply*, HttpStatusCode> WebTask::receiveNetworkReply() {
+void WebTask::slotNetworkReplyFinished() {
     DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
-    DEBUG_ASSERT(m_status != Status::Idle);
-    auto* const networkReply = qobject_cast<QNetworkReply*>(sender());
-    HttpStatusCode statusCode = kHttpStatusCodeInvalid;
-    VERIFY_OR_DEBUG_ASSERT(networkReply) {
-        return qMakePair(nullptr, statusCode);
-    }
-    networkReply->deleteLater();
 
+    auto* const finishedNetworkReply = qobject_cast<QNetworkReply*>(sender());
+    VERIFY_OR_DEBUG_ASSERT(finishedNetworkReply) {
+        return;
+    }
+    finishedNetworkReply->deleteLater();
     if (kLogger.debugEnabled()) {
-        if (networkReply->url() == networkReply->request().url()) {
+        if (finishedNetworkReply->url() == finishedNetworkReply->request().url()) {
             kLogger.debug()
                     << this
                     << "Received reply for request"
-                    << networkReply->url();
+                    << finishedNetworkReply->url();
         } else {
             // Redirected
             kLogger.debug()
                     << this
                     << "Received reply for redirected request"
-                    << networkReply->request().url()
+                    << finishedNetworkReply->request().url()
                     << "->"
-                    << networkReply->url();
+                    << finishedNetworkReply->url();
         }
     }
 
-    if (m_status == Status::Aborted ||
-            m_status == Status::TimedOut) {
-        // Already aborted or timed out by the client
+    if (!m_pendingNetworkReply) {
+        DEBUG_ASSERT(m_state == State::Aborted || m_state == State::TimedOut);
         DEBUG_ASSERT(m_timeoutTimerId == kInvalidTimerId);
         kLogger.debug()
                 << this
                 << "Ignoring obsolete network reply";
-        return qMakePair(nullptr, statusCode);
+        return;
     }
-    VERIFY_OR_DEBUG_ASSERT(m_status == Status::Pending) {
+    VERIFY_OR_DEBUG_ASSERT(m_pendingNetworkReply == finishedNetworkReply) {
+        return;
+    }
+    m_pendingNetworkReply = nullptr;
+
+    VERIFY_OR_DEBUG_ASSERT(m_state == State::Pending) {
         DEBUG_ASSERT(m_timeoutTimerId == kInvalidTimerId);
-        return qMakePair(nullptr, statusCode);
+        return;
     }
 
     if (m_timeoutTimerId != kInvalidTimerId) {
@@ -344,24 +328,18 @@ QPair<QNetworkReply*, HttpStatusCode> WebTask::receiveNetworkReply() {
         m_timeoutTimerId = kInvalidTimerId;
     }
 
-    if (networkReply->error() != QNetworkReply::NetworkError::NoError) {
+    if (finishedNetworkReply->error() != QNetworkReply::NetworkError::NoError) {
         onNetworkError(
-                networkReply->request().url(),
-                networkReply->error(),
-                networkReply->errorString(),
-                networkReply->readAll());
-        return qMakePair(nullptr, statusCode);
-    }
-    m_status = Status::Finished;
-
-    DEBUG_ASSERT(statusCode == kHttpStatusCodeInvalid);
-    VERIFY_OR_DEBUG_ASSERT(readStatusCode(networkReply, &statusCode)) {
-        kLogger.warning()
-                << this
-                << "Failed to read HTTP status code";
+                finishedNetworkReply->request().url(),
+                finishedNetworkReply->error(),
+                finishedNetworkReply->errorString(),
+                finishedNetworkReply->readAll());
+        return;
     }
 
-    return qMakePair(networkReply, statusCode);
+    m_state = State::Finished;
+    const auto statusCode = readStatusCode(finishedNetworkReply);
+    doNetworkReplyFinished(finishedNetworkReply, statusCode);
 }
 
 } // namespace network
diff --git a/src/network/webtask.h b/src/network/webtask.h
index 98f60880ac..48a3942ebc 100644
--- a/src/network/webtask.h
+++ b/src/network/webtask.h
@@ -3,11 +3,11 @@
 #include <QMetaMethod>
 #include <QNetworkAccessManager>
 #include <QNetworkReply>
-#include <QPair>
 #include <QPointer>
 #include <QUrl>
 
 #include "network/httpstatuscode.h"
+#include "util/optional.h"
 
 namespace mixxx {
 
@@ -84,18 +84,17 @@ struct CustomWebResponse : public WebResponse {
 
 QDebug operator<<(QDebug dbg, const CustomWebResponse& arg);
 
-// A transient task for performing a single HTTP network request
-// asynchronously.
-//
-// The results are transmitted by emitting signals. Only a single
-// receiver can be connected to each signal by using Qt::UniqueConnection.
-// The receiver of the signal is responsible for destroying the task
-// by invoking QObject::deleteLater(). If no receiver is connected to
-// a signal the task will destroy itself.
-//
-// Instances of this class must not be parented due to their built-in
-// self-destruction mechanism. All pointers to tasks should be wrapped
-// into QPointer. Otherwise plain pointers might become dangling!
+/// A transient task for performing a single HTTP network request
+/// asynchronously.
+///
+/// The results are transmitted by emitting signals. At least one
+/// of the signal receivers is responsible for destroying the task
+/// by invoking QObject::deleteLater(). If no receiver is connected
+/// at the time the finalization signal is emitted then the task
+/// will destroy itself.
+///
+/// All pointers to tasks should be wrapped into QPointer. Otherwise
+/// plain pointers might become dangling upon deletion!
 class WebTask : public QObject {
     Q_OBJECT
 
@@ -105,28 +104,31 @@ class WebTask : public QObject {
             QObject* parent = nullptr);
     ~WebTask() override;
 
-    // timeoutMillis <= 0: No timeout (unlimited)
-    // timeoutMillis > 0: Implicitly aborted after timeout expired
+    /// timeoutMillis <= 0: No timeout (unlimited)
+    /// timeoutMillis > 0: Implicitly aborted after timeout expired
     void invokeStart(
             int timeoutMillis = 0);
 
-    // Cancel a pending request.
+    /// Cancel a pending request.
     void invokeAbort();
 
-    // Cancel a pending request from the event loop thread.
-    QUrl abort();
+    /// Cancel a pending request from the event loop thread.
+    void abort();
 
   public slots:
     void slotStart(
             int timeoutMillis);
     void slotAbort();
 
+  private slots:
+    void slotNetworkReplyFinished();
+
   signals:
-    // The receiver is responsible for deleting the task in the
-    // corresponding slot handler!! Otherwise the task will remain
-    // in memory as a dysfunctional zombie until its parent object
-    // is finally deleted. If no receiver is connected the task
-    // will be deleted implicitly.
+    /// The receiver is responsible for deleting the task in the
+    /// corresponding slot handler!! Otherwise the task will remain
+    /// in memory as a dysfunctional zombie until its parent object
+    /// is finally deleted. If no receiver is connected the task
+    /// will be deleted implicitly.
     void aborted(
             const QUrl& requestUrl);
     void networkError(
@@ -145,7 +147,7 @@ class WebTask : public QObject {
 
     void timerEvent(QTimerEvent* event) final;
 
-    enum class Status {
+    enum class State {
         Idle,
         Pending,
         Aborting,
@@ -154,25 +156,13 @@ class WebTask : public QObject {
         Failed,
         Finished,
     };
+    State state() const {
+        return m_state;
+    }
 
-    QUrl abortPendingNetworkReply(
-            QNetworkReply* pendingNetworkReply);
-    QUrl timeOutPendingNetworkReply(
-            QNetworkReply* pendingNetworkReply);
-
-    QPair<QNetworkReply*, HttpStatusCode> receiveNetworkReply();
-
-    // Handle status changes and ensure that the task eventually
-    // gets deleted. The default implementations emit a signal
-    // if connected or otherwise implicitly delete the task.
-    virtual void onAborted(
-            QUrl&& requestUrl);
-    virtual void onTimedOut(
-            QUrl&& requestUrl);
-
-    // Handle the abort and ensure that the task eventually
-    // gets deleted. The default implementation logs a warning
-    // and deletes the task.
+    /// Handle the abort and ensure that the task eventually
+    /// gets deleted. The default implementation logs a warning
+    /// and deletes the task.
     virtual void onNetworkError(
             QUrl&& requestUrl,
             QNetworkReply::NetworkError errorCode,
@@ -180,26 +170,33 @@ class WebTask : public QObject {
             QByteArray&& errorContent);
 
   private:
-    // Try to compose and send the actual network request. If
-    // true is returned than the network request is running
-    // and a reply is pending.
-    virtual bool doStart(
+    QUrl abortPendingNetworkReply();
+