Request with QNetworkAccessManager

Question

I have a strange problem with getting reply from get request with QNetworkAccessManager.

This is code of class:

requester.h

#ifndef REQUESTER_H
#define REQUESTER_H

#include <QtNetwork/QNetworkAccessManager>
#include <QtNetwork/QNetworkReply>
#include <QtCore/QtCore>
#include <QVector>
#include <QObject>
#include <QMessageBox>

class Requester : public QObject
{
    Q_OBJECT
    public:
        explicit Requester(QObject *parent = 0);
        ~Requester();
        QString get_last_reply();
        void send_request();
    private:
        QNetworkAccessManager *manager;
        QVector<QString> replies;
    private slots:
        void get_reply(QNetworkReply *reply);
        void get_reply_error(QNetworkReply::NetworkError err);
};

#endif // REQUESTER_H

requester.cpp

#include "requester.h"

Requester::Requester(QObject *p)
        : QObject(p)
        , manager(new QNetworkAccessManager)
{
    QObject::connect(manager, SIGNAL(finished(QNetworkReply*)), this, SLOT(get_reply(QNetworkReply*)));
}

Requester::~Requester() {
    delete manager;
}

void Requester::get_reply(QNetworkReply *reply) {
    QObject::connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(get_reply_error(QNetworkReply::NetworkError)));
    QByteArray res = reply->readAll();
    QString data = res.data();
    replies.push_back(data);
    QObject::disconnect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this,  SLOT(get_reply_error(QNetworkReply::NetworkError)));
    reply->deleteLater();
}

void Requester::get_reply_error(QNetworkReply::NetworkError err) {
    QMessageBox msg;
    msg.setText(QString::number(err));
    msg.setStandardButtons(QMessageBox::Discard);
    msg.exec();
}

QString Requester::get_last_reply() {
    if(!(replies.isEmpty())) {
        QString res =  replies.back();
        replies.pop_back();
        return res;
    }
    return "";
}

void Requester::send_request() {
    QNetworkRequest request;
    request.setUrl(QUrl("http://127.0.0.1"));
    request.setRawHeader("User-Agent", "Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.60 Safari/537.17");
    manager->get(request);
    //QMessageBox *msg = new QMessageBox;
    //msg->exec();
}

Reply is written in textEdit by this function

void MainWindow::ret_out(QString str) {
    ui->out->setText(str);
}

Now main.cpp

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    MainWindow w;
    Requester req;
    req.send_request();
    //QMessageBox *msg = new QMessageBox;
    //msg->exec();
    QString buf = req.get_last_reply();
    w.show();
    w.ret_out(buf);
    return a.exec();
}

Using this code I have empty textEdit. But if uncomment

QMessageBox *msg = new QMessageBox;
msg->exec();

in Requester::send_request or in main.cpp then textEdit contains server answer.

New Update

Now I have this code and it works. Maybe it's not the best variation, but I'll listen to your advices with greate pleasure :)

void Requester::send_request(QUrl url) {
    QEventLoop loop;
    loop.connect(manager, SIGNAL(finished(QNetworkReply*)), this, SLOT(get_reply(QNetworkReply*)));
    loop.connect(this, SIGNAL(done()), &loop, SLOT(quit()));
    QNetworkRequest request;
    request.setUrl(url);
    request.setRawHeader("User-Agent", "Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.60 Safari/537.17");
    manager->get(request);
    loop.exec(QEventLoop::AllEvents);
    loop.disconnect(manager, SIGNAL(finished(QNetworkReply*)), this, SLOT(get_reply(QNetworkReply*)));
    loop.disconnect(this, SIGNAL(done()), &loop, SLOT(quit()));
}

Answer 1

As I wrote earlier in my comment, there are several issues with this code, but the main issue is the last one, so if you do not care about other suggestions, jump to that one.

1) QVector replies;

You should consider using QStringList here.

2) QObject::connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(get_reply_error(QNetworkReply::NetworkError))); QByteArray res = reply->readAll();

You should do this in the constructor alongside the finished signal management.

3) QNetworkAccessManager *manager;

It is unnecessary to allocate this object on the heap, and it also has additional burden like constructing manually in the class constructor, and deleting it manually in the class destructor. You could simply allocate this object on the stack without issues, and that would result a slightly simpler code.

4) QByteArray res = reply->readAll();

You probably do not need a temporary variable in here.

5) QString data = res.data();

You need to make sure about the encoding, so I would suggest writing something like this:

QString data = QString::fromUtf8(reply->readAll());

or

QString data = QString::fromLatin1(reply->readAll());

or

QString data = QString::fromLocal8Bit(reply->readAll());

6) replies.push_back(data);

This is not Qt style. You could consider this:

replies.append(data);

7) w.ret_out(buf);

You are asking for this before entering the Qt event loop. You should write the ui element with setText() when your slot handler is called for finished. Try this:

void Requester::get_reply(QNetworkReply *reply) {
    QObject::connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(get_reply_error(QNetworkReply::NetworkError)));
    QByteArray res = reply->readAll();
    QString data = res.data();
    replies.push_back(data);
    QObject::disconnect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this,  SLOT(get_reply_error(QNetworkReply::NetworkError)));
    reply->deleteLater();

    ui->out->setText(data); // You will need access the "ui" here obviously, or the main window needs access to the Requester object, and the signal-slot has to be connected in there.
}

You could also set up a "local" QEventLoop by instantiating it, but it is better if you leave it with the main application event loop, namely return a.exec();.

Note that the QtNetwork API is async, and it needs an event loop to work. It is not blocking because that would be unfortunate for UI applications without additional worker thread management for end users. That is the reason why the mysterious QDialog::exec() code "fixes" your code because that is an event loop right there. However, that seems to be a not so good fix in this special case, so I would suggest to use the aforementioned main application event loop like probably the vast majority of the applications out there in such scenarios like this.

In your newly uploaded code, there are two issues:

...MainWindow, SLOT(MainWindow::ret_out(QString))...

1) You do not seem to have a MainWindow object on the heap anywhere available for your requester class.

2) MainWindow::ret_out(QString) is wrong... just like you do not use the other slots like that either. You should drop the MainWindow:: scope in there...

Answer 2

You're using signals and slots incorrectly. The QObject::connect method by default has Qt::AutoConnection connection type which means that when the signal is emmited from a different thread, it is posted to the object thread message queue and executes when the message is pumped. QNetworkRequest is asynchronous and runs in a different thread so it seems to be your case.

In the first case you haven't message loop running and thus there's no chance for signal to be processed. But QMessageBox::exec run it's own message loop and in this case the signal is delievered.

This means that you may either send a request after the QApplication::exec or use Qt::DirectConnection flag in QObject::connect(). In the second case you should take care of making your Requester thread-safe.

And the last question is the crash in a busy loop. I don't have an idea why it happens without a call stack but I'm sure it vanishes after you fix your code.