Error reporting - Node.js-style callbacks and custom logger

[hdachev]

First of all, thanks for this! I've been missing a SASL-capable memcached node.js client for a while now.

In any case - shouldn't all client callbacks follow the node.js style of (err) errbacks and (err, success) or (err, data) callbacks? IMO, reporting success with callback(response.val) or callback(true) is confusing, as the first argument is for reporting errors. In this line of thought, I also believe errors shouldn't be just logged to console but passed to the callback so that userland code can decide on its recovery strategy, or perhaps so that errors can bubble up so that higher-level operations fail with an informative error.

Finally, I don't think you should be hard-coding console as a logger, e.g. I might want to log with util, or Winston, or not log at all.

What do you think?

Regards,
Hristo

[alevy]

Thanks for the comments! I'm going to break them up into separate issues so they can be addressed and closed independently, but also responding here:

1. For the (err) parameter, this makes sense in general. Worth discussing this more in depth in a separate issue, specifically when/which errors to pass in specific cases.
2. I disagree that errors should be bubbled up rather than logged. Definitely for certain errors (like failure of operations that will be retried), I think it's important for the caller to be oblivious to this happening (notice that in the case of retries it's still a user tunable parameter). I do agree that some errors (e.g. out of memory) should bubble up as well.
3. RE: console logging. Again, this makes sense to me. The one caveat is that with memcache you often want to silently ignore failures (as long as they are transient) but log them so you know. I'm happy allowing other logging mechanisms as long as it doesn't complicate the common case. Perhaps just a config var that's a logging function the user can replace? Would just default to console.log...

Anyway, I'll create some specific issue around these comments, please feel free to comment/discuss/send pull requests on those as well. Thanks!

[alevy]

Marking as duplicate since I broke this into two separate issues.

[hdachev]

Of course, thanks!
On Jun 19, 2012 10:12 PM, "Amit Levy" <
reply@reply.github.com>
wrote:

Marking as duplicate since I broke this into two separate issues.

---

Reply to this email directly or view it on GitHub:
#1 (comment)