Emit errors instead of throwing uncatchable errors

[indexzero]

Anywhere that you are throwing errors on the nextTick those errors should be emitted on the client instance.

• https://github.com/mranney/node_redis/blob/master/index.js#L586-L588
• https://github.com/mranney/node_redis/blob/master/index.js#L558-L565
• https://github.com/mranney/node_redis/blob/master/index.js#L567-L571

[indexzero]

FYI @mranney and I discussed this at NodeSummit

[brycebaril]

Hi! I'm also at nodesummit. Will try to find you.
On Dec 4, 2013 4:35 PM, "Charlie Robbins" notifications@github.com wrote:

FYI @mranney https://github.com/mranney and I discussed this at
NodeSummit

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/522#issuecomment-29861095
.

[mranney]

@brycebaril sorry I missed you at the event today.

I talked with @indexzero about this, and it seems like we are doing the wrong thing by throwing instead of emitting. This ends up crashing programs when they could have otherwise continued.

[indexzero]

@brycebaril @jcrugzz bump.

[brycebaril]

Went ahead and added this this morning. Will likely push as 0.10.0 later today.

6a3ccf6

[brycebaril]

Pushed as 0.10.0

[jcrugzz]

@brycebaril awesome, thanks!

[indexzero]

Very nice.

[brycebaril]

A revert of this behavior is gaining steam and likely to happen.

The issue is the isolation of error type, client errors vs user errors. People expect to be able to handle client errors but not have to try and detect the error is in user code in the same place.

See: #591 #593 #597 (and perhaps more)

[jcrugzz]

@brycebaril the problem we were having specifically is that node_redis was rethrowing something that was not an actual fatal error and was handled in user code. It just trapped a callback that returned an error as its first argument and rethrew that somehow. (there was some recursion involved and I can see if I can find the isolated case once again).

I think the core issue here is that node_redis errors are commonly non-fatal and the redis parser is trapping user errors that ARE fatal. I would love to help solve this specific problem if we can make the parser not swallow errors outside of its scope.

[brycebaril]

@jcrugzz that sounds great.

It seems that perhaps a good plan for the short term is to revert 6a3ccf6 in the next push and we can work on finding a test case for the problem you were seeing so that we can address it in a way that doesn't get in the way of how other people are expecting error-handling to work.

I'll bump minor for that release.