Errors lost because done() not called

[ravenscar]

In my app I have a process listener for uncaughtException which dumps the exception along with some other info to a log file.

This works fine with deferred except where an error is raised within a promise and I have forgotten to call done() somewhere, in which case the error is swallowed. This happens a lot more often than I would like, especially when promises are getting passed around a lot.

Do you think it would be useful to have some kind of monitoring for not only promises which have been resolved in a certain time frame, but also for promises which have not been closed with done()? If so I may look at implementing it in the monitor.

Thanks.

[medikoo]

Hey @ravenscar thanks for bringing that up.

I'm not convinced in introducing such monitor, for following reasons:

• It cannot be implemented properly in plain JavaScript. It will always be vulnerable to false positives.
• It introduces another complexity and becomes source of many other bugs. If you look at libraries that implement it, you'll see its issue tracker filled with many bugs related to that feature. I'd like to keep things simple and avoid that.
• Most importantly, it's not that needed when you use done as first choice method, and use then only when you need promise object of transformed value.
  The only case where done needs to be called not as a result processor but strictly as an error revealer, is when we do some async update operations. e.g. writeFile('/some/path').done(), that's the only case when we can forget to call done. Is this your case?
  Promise implementations that provide requested monitoring solution, usually promote using then for every kind of value processing, and do not provide done at all, or if they provide it, they treat it as second class citizen (usually not implemented optimally).

I've used this lib extensively over last few years and remember only one case when I forgot to add done after update operation, so in general I take that as not big issue.

All above points lead me to conclusion: it's not worth it.

---

Still, while cases where we can forget done are rare, they definitely exist.
I thought about it recently, looked at other async API's and I think it may be easily fixable if with promises we follow one rule that's respected also in other types of async handling. Take a look at following examples:

// Generators
co(function* () {
  readFile('/not/existing/path');
  // Error not handled (via try/catch), therefore: thrown unconditionally
})();

// Streams
fs.createReadStream('/not/existing/path');
// Error not handled (via stream.on('error', cb)), therefore: thrown uncoditionally

// Promises
readFile('/not/existing/path');
// Error not handled, therefore: not exposed (!)

Technically we can also throw errors unconditionally if there's no callback attached for rejected promise. I'm thinking about implementing that behavior in next version of deferred, but still I haven't tried that in a wild, and at this point I'm not 100% sure whether it's acceptable for promise kind. If it is, I will take is as a final solution to that issue, and implement it. Stay tuned :)

I'm closing it for now, but let's keep discussion on.