Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

OnResourceReceived is not fired after a synchronous ajax request #11284

Closed
igormukhin opened this issue May 2, 2013 · 12 comments
Closed

OnResourceReceived is not fired after a synchronous ajax request #11284

igormukhin opened this issue May 2, 2013 · 12 comments
Labels

Comments

@igormukhin
Copy link

Which version of PhantomJS are you using?

phantomjs-1.9.0 (Windows)

What steps will reproduce the problem?

a) a page with jQuery.ajax({ url: ..., async: false });
b) open this page in phantomjs

What is the expected output? What do you see instead?

expected: OnResourceReceived should be fired
happens: OnResourceReceived never fired for this resource

@Havelock-Vetinari
Copy link

I believe that this duplicates #10498

@JamesMGreene
Copy link
Collaborator

@igormukhin: I'm assuming you meant async: false, right?

@igormukhin
Copy link
Author

@JamesMGreene Yes, async: false. Fixed the description.

@igormukhin
Copy link
Author

I will try to describe my situation. I have to render a screenshot of a webpage that loads the most of its resources after onLoad event. So I can't rely on onLoadFinished.

So I tried to track down, how many resources are loading at the moment by incrementing on the onResourceRequested event and decrementing on onResourceReceived event. And I planned to wait 200ms after the counter becomes zero. And if the counter does not change again, I can render the screenshot.

But the counter never got to zero in my case, cause onResourceRequested was fired on all synchronous requests, but onResourceReceived was never fired for these requests.

I wonder if there is another efficient way to wait until the page is completely loaded.

@JamesMGreene
Copy link
Collaborator

@igormukhin: If the page happens to be using jQuery for its AJAX calls, you could add an ajaxStop handler, e.g.:

$(document).ajaxStop(function() {
  if (typeof window.callPhantom === 'function') {
    window.callPhantom({ name: 'ajaxStop' });
  }
});

Then just add a WebPage#onCallback handler for it on the PhantomJS side, e.g.:

page.onCallback = function(data) {
  if (data && data.name === 'ajaxStop') {
    setTimeout(function() {
      page.render('thisPage.png');
      phantom.exit();
    }, 200);
  }
};

@igormukhin
Copy link
Author

@JamesMGreene Thanks for the hint. I missed that reading the docs. If I could change the page's code or be always sure it uses only jQuery for its requests, I could rely on that.

But, I think, counting the starting requests and ending responses is more reliable for purposes of making screenshots.

@optimuspaul
Copy link

I'm seeing the opposite as well, where only the OnResourceReceived is called but onResourceRequested is not.

@crayze
Copy link

crayze commented Feb 27, 2015

I investigated the problem and this is qwebkit issue - the same things happends on Qt objects level, finished() signals are not emmited never for AJAX requests.
Im my code i wrote Network Manager which is importatnt part of my tool, in other words i must know when any request starts and when ends with its details, with its bytes content. With this issue AJAX requests never ends because qt objects not emits finished() signals and I couldn't detect it.
First i wrote my program in yours phantom, but phantom have some buggs and is unstable in some situations, not emmited onResourceRespond on AJAX synchronized requests bug was main reason why I decited to go level below and I started writing same tool on Qt level.

Well in Qt problem is the same, it's not phantomjs bug, but finished() singnals are not emitted by Qt objects for AJAX requests. Situation presents this way:

  1. For AJAX asynchro requests QNetworkReply::finished() is never emitted, QNetworkAccessManager::finished(QNetworkReply*) is emitted, you connects to 2nd signal in Phantom that's why those works in Phantomjs, in my tool i connected to QNetworkReply::finished() so I couldn't detect any AJAX request ending
  2. For AJAX synchro requests any both finished() signals not fires in any case and this situation is reported as this bug here.

Why this happends? That's because webkit AJAX requester manager destroys QNetworkReply object very fast handly, in above situations QNetworkReply::destroyed() is called before QNetworkReply::finished() signal is processed (I guess webkit code calls delete, not deleteLater() on QNetworkReply object which destroys it immediately without possible to process finishing signals).

My solution is to call onFinished() or onError() signals methods handly, in QNetworkReply::destroyed() method slot if they weren't before, this makes sure to fire finishing slots allways for every QNetworkReply object.

void MyNetworkRequest::onTimeouted() {
  if(this->isActive()) {
    this->data.time.end = QDateTime::currentDateTime().toTime_t();
    this->data.state = TIMEOUTED;

    this->data.error.code = TIMEOUT_ERROR_CODE;
    this->data.error.text = "Request respond timeouted";

    this->reply->abort(); /* this crashed me before because this->reply is deleted here which I didn't know, now onFinished() or onFailed() shotsdown timeout timers correctly before object destroy */

    this->supportDelayedRequestToFile();

    emit requestStateChange(this);
  }
}

void MyNetworkRequest::onFree() { /* slot connected with QNetworkReply::destroyed() */
  if(this->isActive()) { /* isActive means request is not ended, finished() or error() wasn't fired before, but object will be delete for a moment */
  /* this is fix for AJAX requests, QNetworkReply* is correctly setup here, fire signals finished() or error(QNetworkReply::NetworkError) handly before object destroying */
    if(this->reply->error() == QNetworkReply::NoError) this->onFinished();
    else this->onFailed(this->reply->error());
  }
}```


I know that new version PhantomJS 2 under Qt5  is created but this problem exists also in Qt5.

@awakmu
Copy link

awakmu commented Jan 15, 2016

@crayze, is your patch worker in PhantomJS 2 ?
I have similar problem here #13891

@gmetais
Copy link

gmetais commented May 21, 2016

Hey @crayze, sorry to disturb, I'm wondering if you could make a pull request to the project with your solution in order to get this bug fixed?

@crayze
Copy link

crayze commented May 22, 2016

This is Qt network level simple example code to fix this problem (destroying QNetworkReply object before finishing signals execution)
I hope this simple example helps you to understand where is the problem with QNetworkAccessManager+QNetworkReply, here you have 100% sure to call error() or finished() before QNetworkReply destroy.
Test NetworkAccessManager class under QWebPage, I dont have webkitwidgets module (it's deprecated :)) installed, but as I remember this should work correctly also for AJAX requests.

#ifndef NETWORKREQUESTHANDLER_HPP
#define NETWORKREQUESTHANDLER_HPP

#include <QNetworkReply>

class NetworkRequestHandler: public QObject {
  Q_OBJECT
public:
  NetworkRequestHandler(QNetworkReply* r, QObject* parent = NULL);
  ~NetworkRequestHandler();

  // access qt level object
  // can return NULL, if requester(qtwebkit) destroys QNetworkReply by its own(which happends for AJAX synchro requests)
  QNetworkReply* reply() { return this->rep; }

signals:
  // QNetworkReply similar signals, use them instead QNetworkReply singals
  // for other signals QNetworkReply you can use this->reply() object to connect them in normal way
  // those singals will be called even for AJAX synchro request, QNetworkReply won't
  // you must connect them direct(queued will crash for ajax requests)
  void error(QNetworkReply* r, QNetworkReply::NetworkError code);
  void finished(QNetworkReply* r);



private slots:
  void onQtNetworkReplyError(QNetworkReply::NetworkError code);
  void onQtNetworkReplyFinished();
  void onQtNetworkReplyDestroy();

private:
  QNetworkReply* rep; // qt level object
  bool is_done; // true when failed or succesed singals were emitted
};

#endif // NETWORKREQUESTHANDLER_HPP
#include "networkrequesthandler.hpp"

NetworkRequestHandler::NetworkRequestHandler(QNetworkReply* r, QObject* parent): QObject(parent) {
  this->rep = r;
  this->is_done = false; // false until error() on finished() fires

  connect(r, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(onQtNetworkReplyError(QNetworkReply::NetworkError)));
  connect(r, SIGNAL(finished()), this, SLOT(onQtNetworkReplyFinished()));
  connect(r, SIGNAL(destroyed()), this, SLOT(onQtNetworkReplyDestroy()));
}

NetworkRequestHandler::~NetworkRequestHandler() {
  if(this->rep) { // if you delete handler delete also request
    this->rep->deleteLater();
    this->rep = NULL;
  }
}

void NetworkRequestHandler::onQtNetworkReplyError(QNetworkReply::NetworkError code) {
  if(this->is_done == false) {
    emit this->error(this->reply(), code);
    this->is_done = true;
  }
}

void NetworkRequestHandler::onQtNetworkReplyFinished() {
  if(this->is_done == false) {
    emit this->finished(this->reply());
    this->is_done = true;
  }
}

void NetworkRequestHandler::onQtNetworkReplyDestroy() {
  /* here is the whole trick:
   *  on AJAX synchro requests QNetworkReply is destroyed before error() or finished() are executed
   *  you must detect this here
   *  I guess this is because error()/finished() are queued and waits for execution in thread queue,
   *  but qtwebkit destroys QNetworkReply immediately, so events are popbably also removed from threads queue
   */
  if(this->is_done == false) {
    /*
     * this statement will be positive only for AJAX synchro requests
     * and there you must force emit ending singal, because qt wont do this
     */

    // you cant queue this singal event, because QNetworkReply is free when this function returns
    // and for queued signal connection you sends only dangling pointer here
    if(this->reply()->error() == QNetworkReply::NoError)
      emit this->finished(this->reply()); // must be direct call
    else
      emit this->error(this->reply(), this->reply()->error()); // must be direct call

    this->is_done = true;
  }

  this->rep = NULL; // qtwebkit destroys QNetworkReply, reply() starts return NULL now
}
#ifndef NETWORKACCESSMANAGER_HPP
#define NETWORKACCESSMANAGER_HPP

#include "networkrequesthandler.hpp"
#include <QNetworkAccessManager>


// extends QNetworkAccessManager to handle NetworkRequestHandler objects(instead using QNetworkReply)
// NetworkRequestHandler is extended QNetworkReply, the diffrence is NetworkRequestHandler has own singals level
// the problem with AJAX synchro requests 

class NetworkAccessManager: public QNetworkAccessManager {
  Q_OBJECT
public:
  NetworkAccessManager(QObject* parent = NULL): QNetworkAccessManager(parent) {}
  ~NetworkAccessManager() {}

signals:
  // new network request created, you can use this to initialize new NetworkRequestHandler object
  void networkRequestHandlerCreated(NetworkRequestHandler* request_handler);


protected:
  QNetworkReply* createRequest(Operation op, const QNetworkRequest& req, QIODevice* outgoingData);
};

#endif // NETWORKACCESSMANAGER_HPP
#include "networkaccessmanager.hpp"

QNetworkReply* NetworkAccessManager::createRequest(Operation op, const QNetworkRequest& req, QIODevice* outgoingData) {
  // create QNetworkReply as usually, you can reimplement createRequest by your own as usually
  QNetworkReply* r = QNetworkAccessManager::createRequest(op, req, outgoingData);

  // now create handler for QNetworkReply, you must intercept object use this in outer level instead QNetworkReply
  emit this->networkRequestHandlerCreated(new NetworkRequestHandler(r, this));

  return r;
}
#ifndef USAGEEXAMPLE_HPP
#define USAGEEXAMPLE_HPP

#include "networkaccessmanager.hpp"

class NetworkMonitor: public QObject {
  Q_OBJECT
public:
  NetworkMonitor(NetworkAccessManager* n) {
    connect(n, SIGNAL(networkRequestHandlerCreated(NetworkRequestHandler*)), this, SLOT(start(NetworkRequestHandler*)));
  }

public slots:
  void start(NetworkRequestHandler* rh);
  void respond();
  void error(QNetworkReply* r, QNetworkReply::NetworkError code);
  void finished(QNetworkReply* r);
};

#endif // USAGEEXAMPLE_HPP
#include "usageexample.hpp"
#include <QDebug>

void NetworkMonitor::start(NetworkRequestHandler* rh) {
  connect(rh->reply(), SIGNAL(metaDataChanged()), this, SLOT(respond()));
  connect(rh, SIGNAL(error(QNetworkReply*,QNetworkReply::NetworkError)), this, SLOT(error(QNetworkReply*,QNetworkReply::NetworkError)));
  connect(rh, SIGNAL(finished(QNetworkReply*)), this, SLOT(finished(QNetworkReply*)));

  qDebug() << "START:" << rh->reply()->url();
}

void NetworkMonitor::respond() {
  QNetworkReply* r = reinterpret_cast<QNetworkReply*>(this->sender());
  qDebug() << "RESPOND:" << r->url();
}

void NetworkMonitor::error(QNetworkReply* r, QNetworkReply::NetworkError code) {
  NetworkRequestHandler* rh = reinterpret_cast<NetworkRequestHandler*>(this->sender());
  qDebug() << "ERROR<" + r->errorString() + ">:" << r->url();
  rh->deleteLater();
}

void NetworkMonitor::finished(QNetworkReply* r) {
  NetworkRequestHandler* rh = reinterpret_cast<NetworkRequestHandler*>(this->sender());
  qDebug() << "FINISHED:" << r->url();
  rh->deleteLater();
}
#include "networkaccessmanager.hpp"
#include "usageexample.hpp"

#include <QApplication>

int main(int argc, char *argv[]) {
  QApplication a(argc, argv);
  NetworkAccessManager network_stack;
  NetworkMonitor network_monitor(&network_stack);

  network_stack.get(QNetworkRequest(QUrl("http://google.com")));

  return a.exec();
}

@stale
Copy link

stale bot commented Dec 29, 2019

Due to our very limited maintenance capacity, we need to prioritize our development focus on other tasks. Therefore, this issue will be automatically closed (see #15395 for more details). In the future, if we see the need to attend to this issue again, then it will be reopened. Thank you for your contribution!

@stale stale bot closed this as completed Dec 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants