How to properly abort a network request in Qt?

Question

I have a class that performs a request to the network and parses the data. How do I properly implement the request abort for it?

Imagine that I have such a class:

class MyClass
{
public:
...
    void doRequest()
    {
        m_reply = m_manager.get(...);

        QEventLoop waitForResponse;
        connect(m_reply, &QNetworkReply::finished, &waitForResponse, &QEventLoop::quit);
        waitForResponse.exec();

        // Check if request was aborted (otherwise, application will crash)
        if (m_reply == nullptr)
            return;

        // Check for network errors, write result to m_data and delete m_reply;
        ...
    }
    void abort()
    {
        if (m_reply != nullptr)
            m_reply->abort();
    }
    QString data()
    {
        return m_data;
    }
...
private:
    QNetworkAccessManager *m_manager;
    QPiinter<QNetworkReply> m_reply;
    QString m_data;
}

Here is an example of its use by pressing the button:

class MainWindow : public QMainWindow
{
...
private slots:
    MainWindow::on_myButton_pressed()
    {
        m_myClass->abort();
        m_myClass->doRequest();
        ui->myTextEdit->setText(m_myClass->data());
    }

private:
    MyClass m_myClass;
}

When you press the button, if the previous request is not completed, then it is canceled. This works. But under the hood in this case a new request writing data into the QTextEdit and exit the function, then old request returning from it's own loop and writes the same m_data to QTextEdit again.

Is it corrent? Maybe there is a more correct way to implement this?

Answer 1

Nested event loops are the root of all evil. It is much simpler to write a function like doRequest without pretending to the user that it is a synchronous function. It seems that you have already traced the convoluted control-flow that happens when you call abort() and you understand how subsequent calls to doRequest() end up being nested calls due to re-entering the event loop. If you restart your request multiple times, your stack would look something like (the stack grows downwards):

1.  main function
2.  main event loop
3.  [...] (Qt functions)
4.  MainWindow::on_myButton_pressed()
5.  MyClass::doRequest()
6.  QEventLoop::exec()
7.  [...] (Qt functions)
8.  MainWindow::on_myButton_pressed()
9.  MyClass::doRequest()
10. QEventLoop::exec()
11. [...] (Qt functions)
12. MainWindow::on_myButton_pressed()   and so on...

Each one of the calls to MainWindow::on_myButton_pressed() need to call ui->myTextEdit->setText() when its nested event loop exits.

An alternative would be make your functions fully asynchronous and rely on signals/slots if you need things to be executed when a particular operation finishes. Here is a minimal implementation for what you are trying to achieve:

#include <QtNetwork>
#include <QtWidgets>

/// A class responsible for communication with the web backend
class Backend : public QObject {
  Q_OBJECT
public:
  explicit Backend(QObject *parent = nullptr)
      : QObject(parent), m_reply(nullptr) {}
public slots:
  void doRequest() {
    // abort and delete ongoing request (if any)
    if (m_reply) {
      m_reply->abort();
      delete m_reply;
      m_reply = nullptr;
    }
    emit textUpdated(QString());
    // send request
    QUrl url("http://wtfismyip.com/text");
    QNetworkRequest request(url);
    m_reply = m_manager.get(request);
    // when the request is finished,
    QObject::connect(m_reply, &QNetworkReply::finished, [this] {
      // if the request ended successfully, read the received ip into m_lastData
      if (m_reply->error() == QNetworkReply::NoError)
        m_lastData = QString::fromUtf8(m_reply->readAll());
      // otherwise, emit errorOccured() signal (if the request has not been
      // actively canceled)
      else if (m_reply->error() != QNetworkReply::OperationCanceledError)
        emit errorOccured(m_reply->errorString());
      // in all cases, emit updateText and do cleanup
      emit textUpdated(m_lastData);
      m_reply->deleteLater();
      m_reply = nullptr;
    });
  }

  void abort() {
    if (m_reply != nullptr)
      m_reply->abort();
  }

signals:
  void textUpdated(const QString &);
  void errorOccured(const QString &);

private:
  QNetworkAccessManager m_manager;
  QNetworkReply *m_reply;
  QString m_lastData;
};

/// A minimal widget that contains a QPushButton and a QLabel
class Widget : public QWidget {
  Q_OBJECT
public:
  explicit Widget(QWidget *parent = nullptr) : QWidget(parent) {
    m_layout.addWidget(&m_pushButton);
    m_layout.addWidget(&m_label);

    connect(&m_pushButton, &QPushButton::clicked, this, &Widget::buttonClicked);
  }

signals:
  void buttonClicked();

public slots:
  void updateText(const QString &text) { m_label.setText(text); }
  void showError(const QString &error) {
    QMessageBox::warning(this, tr("Error"), error);
  }

private:
  QVBoxLayout m_layout{this};
  QPushButton m_pushButton{"Retrieve Name"};
  QLabel m_label;
};

int main(int argc, char *argv[]) {
  QApplication a(argc, argv);

  Backend backend;
  Widget widget;

  // connect components
  QObject::connect(&backend, &Backend::textUpdated, &widget,
                   &Widget::updateText);
  QObject::connect(&backend, &Backend::errorOccured, &widget,
                   &Widget::showError);
  QObject::connect(&widget, &Widget::buttonClicked, &backend,
                   &Backend::doRequest);
  widget.show();

  return a.exec();
}

#include "main.moc"