Skip to content

Emitting error event when connection permanently goes down#615

Closed
tobek wants to merge 1 commit into
redis:masterfrom
tobek:master
Closed

Emitting error event when connection permanently goes down#615
tobek wants to merge 1 commit into
redis:masterfrom
tobek:master

Conversation

@tobek

@tobek tobek commented Jun 26, 2014

Copy link
Copy Markdown

Also documenting the "reconnecting" event.

Happy to discuss this further, but either way, when working with sharded Redis instances (for instance using https://github.com/blindsey/redis-shard) it's helpful to have some way to know when a Redis server seems to have totally gone down so that we can send out alerts, remove it from the cluster, spin up another server, etc.

@ghost

ghost commented Jul 8, 2014

Copy link
Copy Markdown

👍

@brycebaril

brycebaril commented Jul 9, 2014

Copy link
Copy Markdown
Contributor

How about emitting an error instead of a "broken" event, then we will get the throw behavior. This won't do anything if nobody is listening for the "broken" event.

@tobek

tobek commented Jul 9, 2014

Copy link
Copy Markdown
Author

That sounds good to me too, I only went for a special broken event because the comments talk about a "redis is broken mode", which suggests this is not so much an error as a change in state. I'm not fussed though, just need to able to listen for them somehow. error instead, then?

@brycebaril

brycebaril commented Jul 11, 2014

Copy link
Copy Markdown
Contributor

Yeah, an "error" with an error something to the effect of new Error("Redis connection in broken state") or similar would be great!

@tobek tobek changed the title Adding "broken" event for when connection permanently goes down Emitting error event when connection permanently goes down Jul 14, 2014
@tobek

tobek commented Jul 14, 2014

Copy link
Copy Markdown
Author

Sounds good - done, and squashed into single commit.

Emitting error event after hitting reconnect max_attempts or connect_…
0727937 
…timeout

Also documenting the "reconnecting" event
@PizzaBrandon

PizzaBrandon commented Aug 7, 2014

Copy link
Copy Markdown

This appears to be an easily-fixed hole in the library. I provide a connection timeout option to the client and I need my program to know when the Redis client has given up; today it cannot know this without regularly inspecting the client object's retry_totaltime property. Please merge this and update in npm.

@aryehischechter

aryehischechter commented Jul 8, 2015

Copy link
Copy Markdown

this fixes issue #586

BridgeAR
BridgeAR reviewed Sep 3, 2015
View reviewed changes
Comment thread index.js

@BridgeAR BridgeAR Sep 3, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove both TODOs including the console.error

@BridgeAR

BridgeAR commented Sep 3, 2015

Copy link
Copy Markdown
Contributor

This needs a rebase and two small comments otherwise LGTM

@BridgeAR BridgeAR mentioned this pull request Sep 4, 2015
Closed
@BridgeAR BridgeAR self-assigned this Sep 4, 2015
@BridgeAR BridgeAR mentioned this pull request Sep 10, 2015
Merged
BridgeAR pushed a commit that referenced this pull request Sep 11, 2015
Emit an error when connection permanently goes down
4a7b115 
Closes #724 and #615
BridgeAR pushed a commit that referenced this pull request Sep 11, 2015
Emit an error when connection permanently goes down
d837013 
Closes #724 and #615
@tobek

tobek commented Sep 15, 2015

Copy link
Copy Markdown
Author

#829 has got this covered, closing this PR

@tobek tobek closed this Sep 15, 2015
BridgeAR added a commit that referenced this pull request Sep 16, 2015
@BridgeAR
Merge pull request #829 from fintura/broken-mode
57b5c7f 
Implement redis connection broken mode and more shiny things

Fixes #569
Fixes #587
Fixes #566 
Fixes #586 
Fixes #280 

This includes the fixes as suggested in #671, #615 and #533. Thx a lot to @qdb, @tobek and @chrishamant 

Closes #675, #463, #362, #438 and #724
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@BridgeAR BridgeAR

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tobek @brycebaril @PizzaBrandon @aryehischechter @BridgeAR