iriscouch / follow

Very stable, very reliable, NodeJS CouchDB _changes follower
Apache License 2.0
393 stars 82 forks source link

Don't crash on timeout if the feed was stopped #55

Closed josip closed 9 years ago

josip commented 9 years ago

on_timeout keeps causing crashing node for me because it gets called even after you do a feed.stop(). This may not be the proper solution but it at least stops things from crashing [in my case].

jcrugzz commented 9 years ago

@josip this seems reasonable. kind of crazy that this type of race condition can happen though!

josip commented 9 years ago

My only guess is that wait() somehow gets called from somewhere, because that's the only place that doesn't check if the feed has been declared dead (besides on_timeout). But yeah, no idea how or why it happens.

jcrugzz commented 9 years ago

@josip let me know if you can get a small reproducible case of this for testing. That would be super helpful. I understand its tricky to create the right conditions though

josip commented 9 years ago

var cradle = require('cradle');
var db = (new cradle.Connection('ip', 80, {
    auth: {username: '', password: ''}
})).database('');

var feed = db.changes({
    since: 'now',
    filter: 'some/very/simple/filter',
    include_docs: true,
    query_params: {
        arg1: '1',
        arg2: '2',
    },
});

feed.on('change', function (change) {
    console.log('change', change);
    console.log('feed stopped');
    feed.stop();
});

feed.on('error', function (err) {
    console.log('feed error', err);
});

feed.follow();

setTimeout(function () { console.log('timeout'); }, 10*60*1000);

This is going to crash it a few seconds after .stop() is called:

/Users/josip/tmp/node_modules/cradle/node_modules/follow/lib/feed.js:448
apsed_ms:elapsed_ms, heartbeat:self.heartbeat, id:self.pending.request.id()});
                                                                       ^
TypeError: Cannot call method 'id' of null
    at Feed.on_timeout (/Users/josip/tmp/node_modules/cradle/node_modules/follow/lib/feed.js:448:98)
    at null._onTimeout (/Users/josip/tmp/node_modules/cradle/node_modules/follow/lib/feed.js:345:58)
    at Timer.listOnTimeout [as ontimeout] (timers.js:112:15)

But I think I've just figured out the problem - it's the call to feed.follow() - I failed to realise that cradle calls follow() already which I guess leads to this kind of undefined behaviour?

jcrugzz commented 9 years ago

@josip hmm cradle is using an older version of follow as well. let me see if i can fix that

josip commented 9 years ago

https://github.com/flatiron/cradle/pull/272 :)