YahooArchive / arrow

FE Test framework designed to promote TDD
http://yahoo.github.io/arrow/arrow_intro.html
BSD 3-Clause "New" or "Revised" License
55 stars 59 forks source link

There's literally no error handling in ArrowSetup.prototype.setup #225

Open samlecuyer opened 10 years ago

samlecuyer commented 10 years ago

https://github.com/yahoo/arrow/blob/master/lib/util/arrowsetup.js#L27

cb(null, ...) ignores any error that happens.

pranavparikh commented 10 years ago

@samlecuyer Thanks for pointing out. Does this look ok - https://github.com/pranavparikh/arrow/blob/errorHandling/lib/util/arrowsetup.js ? Do you have any other recommendations ?

samlecuyer commented 10 years ago

@pranavparikh cb is never invoked with any parameters. That's a problem

pranavparikh commented 10 years ago

@samlecuyer Something like ?

ArrowSetup.prototype.startArrowServer = function(cb) {

var self = this;
try {
    if (self.config.startArrowServer) {
        Servermanager.startArrowServer(function(arrowServerStarted) {
            if (arrowServerStarted === false) {
                self.logger.error('Failed to start Arrow Server. Exiting !!!');
                cb('Failed to start Arrow Server. Exiting !!!');
                return;
            }
            cb();
        });
    } 
}
catch(e) {
    cb(e);
}

};

samlecuyer commented 10 years ago

@pranavparikh much better

pranavparikh commented 10 years ago

@samlecuyer Thanks.I'll work on this and we can go through the changes once I'm done.