Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle redis client error events #36

Merged
merged 1 commit into from
Sep 4, 2024
Merged

fix: handle redis client error events #36

merged 1 commit into from
Sep 4, 2024

Conversation

dnechay
Copy link
Contributor

@dnechay dnechay commented Sep 3, 2024

📚 Context/Description Behind The Change

Under the hood this package creates its own instance of redis client to subscribe on redis events. This client is an instance of EventEmitter and in case of errors it emits error event. When this event doesn't have at least one subscriber - Node.js process is crashed.

When using this library in some production service, it leads to uncontrolled crashes (no way to handled it via process.on listeners), so we have to add such listener here

🚨 Potential Risks & What To Monitor After Deployment

Don't see any

🧑‍🔬 How Has This Been Tested?

  • adding the same code directly on installed package and shutting down redis instance, verify service not crashed

🚚 Release Plan

Merge, get new version, bump everywhere in MM

*
* https://nodejs.org/api/events.html#error-events
*/
this._redisSubscriber.on('error', noop);

Choose a reason for hiding this comment

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

ooooh,

@dnechay dnechay merged commit 594ba06 into master Sep 4, 2024
3 checks passed
@dnechay dnechay deleted the dnechay/RCA-262 branch September 4, 2024 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants