Handle client-side timeouts

4 files changed, 116 insertions, 24 deletions

diff --git a/src/network/jsonwebtask.cpp b/src/network/jsonwebtask.cpp
index af9b084c0a..8ccdcffddc 100644
--- a/src/network/jsonwebtask.cpp
+++ b/src/network/jsonwebtask.cpp
@@ -252,10 +252,30 @@ bool JsonWebTask::doStart(
     return true;
 }
 
-void JsonWebTask::doAbort() {
+QUrl JsonWebTask::doAbort() {
+    DEBUG_ASSERT(thread() == QThread::currentThread());
+    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(thread() == QThread::currentThread());
+    QUrl requestUrl;
     if (m_pendingNetworkReply) {
-        m_pendingNetworkReply->abort();
+        requestUrl = timeOutPendingNetworkReply(m_pendingNetworkReply);
+        // Don't wait until finished
+        m_pendingNetworkReply->deleteLater();
+        m_pendingNetworkReply = nullptr;
     }
+    return requestUrl;
 }
 
 void JsonWebTask::slotNetworkReplyFinished() {
diff --git a/src/network/jsonwebtask.h b/src/network/jsonwebtask.h
index b7d8a79138..879feb1cf6 100644
--- a/src/network/jsonwebtask.h
+++ b/src/network/jsonwebtask.h
@@ -86,7 +86,8 @@ class JsonWebTask : public WebTask {
     bool doStart(
             QNetworkAccessManager* networkAccessManager,
             int parentTimeoutMillis) override;
-    void doAbort() override;
+    QUrl doAbort() override;
+    QUrl doTimeOut() override;
 
     // All member variables must only be accessed from
     // the event loop thread!!
diff --git a/src/network/webtask.cpp b/src/network/webtask.cpp
index 0cbf129c46..eb2e43cf6a 100644
--- a/src/network/webtask.cpp
+++ b/src/network/webtask.cpp
@@ -62,7 +62,7 @@ WebTask::WebTask(
         QNetworkAccessManager* networkAccessManager)
         : m_networkAccessManager(networkAccessManager),
           m_timeoutTimerId(kInvalidTimerId),
-          m_abort(false) {
+          m_status(Status::Idle) {
     std::call_once(registerMetaTypesOnceFlag, registerMetaTypesOnce);
     DEBUG_ASSERT(m_networkAccessManager);
     s_instanceCounter.increment(1);
@@ -72,15 +72,38 @@ WebTask::~WebTask() {
     s_instanceCounter.increment(-1);
 }
 
-void WebTask::onAborted() {
-    DEBUG_ASSERT(m_abort);
+void WebTask::onAborted(
+        QUrl requestUrl) {
+    DEBUG_ASSERT(m_status == Status::Aborted);
     const auto signal = QMetaMethod::fromSignal(
             &WebTask::aborted);
     if (isSignalConnected(signal)) {
         emit aborted();
     } else {
         kLogger.info()
-                << "Request aborted";
+                << "Request aborted"
+                << requestUrl;
+        deleteAfterFinished();
+    }
+}
+
+void WebTask::onTimedOut(
+        QUrl requestUrl) {
+    DEBUG_ASSERT(m_status == Status::TimedOut);
+    const auto signal = QMetaMethod::fromSignal(
+            &WebTask::networkError);
+    if (isSignalConnected(signal)) {
+        emit networkError(
+                std::move(requestUrl),
+                QNetworkReply::TimeoutError,
+                QStringLiteral("Client-side timeout"),
+                QByteArray());
+    } else {
+        kLogger.warning()
+                << "Aborting request after client-side timeout"
+                << requestUrl;
+        // Do not abort the request before deleting it,
+        // i.e. pretend it already has been finished.
         deleteAfterFinished();
     }
 }
@@ -156,14 +179,12 @@ void WebTask::deleteAfterFinished() {
 
 void WebTask::slotStart(int timeoutMillis) {
     DEBUG_ASSERT(thread() == QThread::currentThread());
+    DEBUG_ASSERT(m_status == Status::Idle);
     VERIFY_OR_DEBUG_ASSERT(m_networkAccessManager) {
         kLogger.warning()
                 << "No network access";
         return;
     }
-    // The task might be restarted after it has been aborted
-    // or finished.
-    m_abort = false;
 
     kLogger.debug()
             << "Starting...";
@@ -174,10 +195,11 @@ void WebTask::slotStart(int timeoutMillis) {
     }
 
     // The task could be aborted immediately while being started.
-    if (m_abort) {
-        onAborted();
+    if (m_status == Status::Aborted) {
+        onAborted(doAbort());
         return;
     }
+    m_status = Status::Pending;
 
     DEBUG_ASSERT(m_timeoutTimerId == kInvalidTimerId);
     if (timeoutMillis > 0) {
@@ -186,9 +208,33 @@ void WebTask::slotStart(int timeoutMillis) {
     }
 }
 
+QUrl WebTask::abortPendingNetworkReply(
+        QNetworkReply* pendingNetworkReply) {
+    DEBUG_ASSERT(thread() == QThread::currentThread());
+    DEBUG_ASSERT(pendingNetworkReply);
+    if (pendingNetworkReply->isRunning()) {
+        pendingNetworkReply->abort();
+        // Suspend until finished
+        return QUrl();
+    }
+    return pendingNetworkReply->request().url();
+}
+
+QUrl WebTask::timeOutPendingNetworkReply(
+        QNetworkReply* pendingNetworkReply) {
+    DEBUG_ASSERT(thread() == QThread::currentThread());
+    DEBUG_ASSERT(pendingNetworkReply);
+    if (pendingNetworkReply->isRunning()) {
+        //pendingNetworkReply->abort();
+        // Don't suspend until finished, i.e. abort and then
+        // delete the pending network request instantly
+    }
+    return pendingNetworkReply->request().url();
+}
+
 void WebTask::slotAbort() {
     DEBUG_ASSERT(thread() == QThread::currentThread());
-    if (m_abort) {
+    if (m_status != Status::Pending) {
         DEBUG_ASSERT(m_timeoutTimerId == kInvalidTimerId);
         return;
     }
@@ -196,10 +242,10 @@ void WebTask::slotAbort() {
         killTimer(m_timeoutTimerId);
         m_timeoutTimerId = kInvalidTimerId;
     }
-    m_abort = true;
+    m_status = Status::Aborted;
     kLogger.debug()
             << "Aborting...";
-    doAbort();
+    onAborted(doAbort());
 }
 
 void WebTask::timerEvent(QTimerEvent* event) {
@@ -210,13 +256,19 @@ void WebTask::timerEvent(QTimerEvent* event) {
         // ignore
         return;
     }
-    kLogger.info()
+    killTimer(m_timeoutTimerId);
+    m_timeoutTimerId = kInvalidTimerId;
+    if (m_status != Status::Aborted) {
+        m_status = Status::TimedOut;
+    }
+    kLogger.debug()
             << "Timed out";
-    slotAbort();
+    onTimedOut(doTimeOut());
 }
 
 QPair<QNetworkReply*, HttpStatusCode> WebTask::receiveNetworkReply() {
     DEBUG_ASSERT(thread() == QThread::currentThread());
+    DEBUG_ASSERT(m_status != Status::Idle);
     auto* const networkReply = qobject_cast<QNetworkReply*>(sender());
     HttpStatusCode statusCode = kHttpStatusCodeInvalid;
     VERIFY_OR_DEBUG_ASSERT(networkReply) {
@@ -229,10 +281,11 @@ QPair<QNetworkReply*, HttpStatusCode> WebTask::receiveNetworkReply() {
         m_timeoutTimerId = kInvalidTimerId;
     }
 
-    if (m_abort) {
-        onAborted();
+    if (m_status == Status::Aborted) {
+        onAborted(networkReply->request().url());
         return qMakePair(nullptr, statusCode);
     }
+    m_status = Status::Finished;
 
     if (networkReply->error() != QNetworkReply::NetworkError::NoError) {
         onNetworkError(
diff --git a/src/network/webtask.h b/src/network/webtask.h
index 358f4fed57..e78f0c2be9 100644
--- a/src/network/webtask.h
+++ b/src/network/webtask.h
@@ -136,12 +136,28 @@ class WebTask : public QObject {
             QByteArray errorContent);
 
   protected:
-    void timerEvent(QTimerEvent* event) override;
+    void timerEvent(QTimerEvent* event) final;
 
-    // Handle an aborted request and ensure that the task eventually
+    enum class Status {
+        Idle,
+        Pending,
+        Aborted,
+        TimedOut,
+        Finished,
+    };
+
+    // Handle status changes and ensure that the task eventually
     // gets deleted. The default implementation simply deletes the
     // task.
-    virtual void onAborted();
+    virtual void onAborted(
+            QUrl requestUrl);
+    virtual void onTimedOut(
+            QUrl requestUrl);
+
+    QUrl abortPendingNetworkReply(
+            QNetworkReply* pendingNetworkReply);
+    QUrl timeOutPendingNetworkReply(
+            QNetworkReply* pendingNetworkReply);
 
     // Handle the abort and ensure that the task eventually
     // gets deleted. The default implementation logs a warning
@@ -158,14 +174,16 @@ class WebTask : public QObject {
     virtual bool doStart(
             QNetworkAccessManager* networkAccessManager,
             int parentTimeoutMillis) = 0;
-    virtual void doAbort() = 0;
+    // Handle status change requests and return the request URL
+    virtual QUrl doAbort() = 0;
+    virtual QUrl doTimeOut() = 0;
 
     // All member variables must only be accessed from
     // the event loop thread!!
     const QPointer<QNetworkAccessManager> m_networkAccessManager;
 
     int m_timeoutTimerId;
-    bool m_abort;
+    Status m_status;
 };
 
 } // namespace network