I'm not convinced with the way error handling is done in this plugin.
callback.call(this, file, config) will call the callback immediately, so any errors it may throw are outside the promise handling. You can make that Promise.resolve(config).then(callback.bind(this, file)) or something like this if you want exceptions to enter the error handling machinery.
emit('error', …) won't work because it is unbound. Start with var emit = this.emit.bind(this) or use var self = this and later self.emit.
As far as I understand it, the correct way to report an error from within a transformation function is by passing the error to the callback. In case of an error you never even call the callback.
I recently found out that ending the promise chain with catch instead of done is problematic, since the error reporting facility may itself throw an error. This is e.g. the case if there is no error handler registered for a stream. Without done such an exception would be eaten by the promise, only to be picked up again later by bluebird and delivered out of context.
Sounds like valid issues, this plugin was mostly a proof of concept but wasn't really battle-tested. I'll gladly merge any PR fixing the error handling part.
I'm not convinced with the way error handling is done in this plugin.
callback.call(this, file, config)
will call the callback immediately, so any errors it may throw are outside the promise handling. You can make thatPromise.resolve(config).then(callback.bind(this, file))
or something like this if you want exceptions to enter the error handling machinery.emit('error', …)
won't work because it is unbound. Start withvar emit = this.emit.bind(this)
or usevar self = this
and laterself.emit
.catch
instead ofdone
is problematic, since the error reporting facility may itself throw an error. This is e.g. the case if there is no error handler registered for a stream. Withoutdone
such an exception would be eaten by the promise, only to be picked up again later bybluebird
and delivered out of context.