a load error can still trigger ready and idle events

[kapouer]

Especially in the non-chaining API.
See #26 to reproduce.

Is it something to keep or something that should not happen ?

[gchudnov]

Do you recommend using the chaining API? Are there any difference?

[kapouer]

The chaining API is somewhat safer indeed, since you can call
WebKit().load(url1).wait('idle').png(file1).load(url2).wait('idle').png(file2)
rightaway and do not worry about details.
OR you can insert an error handler in between:

WebKit().load(url1).wait('idle').png(file1, function(err) {
  // err is any error that happened from implicit init() to load, wait, png
}).load(url2).wait('idle').png(file2, function(err) {
  // err is any error that happened from latest load, wait, png
})

[kapouer]

Reproducing here the code given in #26

gchidnov
In the following code that uses the async to print images one after the other, the second load callback (marked as '---> LOAD-COMPLETE') is never called:

    var arr = [
      "https://www.mezilla.org/en-US/",
      "https://www.google.com"
    ];
    async.eachSeries(arr, function(it, cb) {
      var href = it;
      var filePath = path.resolve(__dirname, './shots/outN.png');
      console.error('---> LOAD', href);
      w.load(href, {}, function(err) {
        console.error('---> LOAD-COMPLETE');
        if(err) {
          console.error('---> LOAD-ERR', err);
          cb();
        } else {
          console.error('---> SETTING-IDLE');
          w.once('idle', function() {
            w.png(filePath, function(err) {
              console.error('---> PNG', href, err);
              cb();
            });
          });
        }
      });
    }, function(err) {
      done(err);
    });

Not sure what is the problem?

A complete test might something like this:

var WebKit = require('../');
var expect = require('expect.js');
var fs = require('fs');
var path = require('path');
var async = require('async');

describe.only("ordered png render calls", function suite() {
  this.timeout(20000);
  var w;
  before(function(cb) {
    var inst = new WebKit();
    inst.init({}, function(err) {
      if (err) return cb(err);
      w = inst;
      cb();
    });
  });
  it("should not stall", function(done) {
    var arr = [
      "https://www.mezilla.org/en-US/",
      "https://www.google.com/",
      "https://www.debian.org/"
    ];
    async.eachSeries(arr, function(it, cb) {
      var href = it;
      var filePath = path.resolve(__dirname, './shots/outN.png');
      console.error('---> LOAD', href);
      w.load(href, {}, function(err) {
        console.error('---> LOAD-COMPLETE');  // <--- callback not called for the second time
        if(err) {
          console.error('---> LOAD-ERR', err);
          cb();
        } else {
          console.error('---> SETTING-IDLE');
          w.once('idle', function() {
            w.png(filePath, function(err) {
              console.error('---> PNG', href, err);
              cb();
            });
          });
        }
      });
    }, function(err) {
      done(err);
    });
  });
});

And the same problem reproduced in case of 404:
e.g.

    var arr = [
      "http://www.google.com/asdsad"
      "https://www.google.com/"
    ];

[gchudnov]

Right, it looks the load callback function is never called after the first failure.

No matter what type of API is used:

  it('should not stall', function(done) {
    var filePath = path.resolve(__dirname, './shots/outX.png');
    WebKit().load("http://www.google.com/asdsad").wait('idle').png(filePath, function(err) {
      //
    }).load("https://www.google.com/").wait('idle').png(filePath, function(err) {
      done(); // <---- never hits this line
    });
  });

[kapouer]

Simplifying further,

W().load('http://mezilla.fr', function(err) {console.error(err);}).load('http://www.google.com', function(err) {console.error(err);})

second callback is not called, because there was a failure and that stopped the chain.
This is a behavior of chainit, not something specific here, so we must be careful about which API is used.

[kapouer]

but

var W = require('./')
var inst = new W();
inst.init({}, function(err) {
  inst.load('http://mozilla.org', {}, function(err) {
    console.error(err);
    inst.load('http://mozilla.org', {}, function(err) {console.error(err)})
  });
});

does not call second callback either.

[kapouer]

Ok, i'm NULL-ing the loadCallback in the c++ code at a moment it could have been set by a new Load call.

[kapouer]

Fixed by c888b54, and published a new version.

Note that it doesn't change the fact that

W().load('http://fsgdfgdfgdfgdferrpr.com', console.error).load('http://localhost', console.log.bind('ok'))

does not call the second load.

[gchudnov]

got it, thank you!