Improve error handling #100
Conversation
|
I think you need to remove the |
|
My bad, i thought it was possible to use the |
index.js
Outdated
| } else { | ||
| next() | ||
| const onError = function (err) { | ||
| client.quit(() => next(err)) |
There was a problem hiding this comment.
I don't think we should quit the client on the first error. In this case we are avoiding the retry strategy that is really important imho.
If we want to go with this event path I think we need to manually handle the retry strategy.
| } | ||
|
|
||
| next() |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
When specified lazyConnect, the if clause will stop at return client and next is not reachable.
index.js
Outdated
| // Any other errors during startup will | ||
| // trigger the 'end' event. | ||
| if (err instanceof Redis.ReplyError) { | ||
| this.emit('end', err) |
There was a problem hiding this comment.
I would prefer if we did not call emit() on an object we do not maintain. Just call the onEnd() function.
Checklist
npm run testandnpm run benchmarkand the Code of conduct
Description
Closes #95
This PR improves error handling, it will make the plugin wait until
Redisinstance firesreadyevent before calling thenextfunction, it will also listen to theerrorevent and propagate errors in case anything goes wrong on initial startup.