Fix socket event listener leak, pin Node version, and standardize formatting#22
Open
0xHristo wants to merge 1 commit intowebuniverseio:masterfrom
Open
Fix socket event listener leak, pin Node version, and standardize formatting#220xHristo wants to merge 1 commit intowebuniverseio:masterfrom
0xHristo wants to merge 1 commit intowebuniverseio:masterfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Memory Leak Fix and Tooling Standardization
Summary
This MR addresses a memory leak in the socket event listener setup, pins the supported Node.js range for test stability, and standardizes formatting using Prettier and VS Code settings.
Fixes #21
Background / Motivation
We have been using this package in production with a
keepAliveagent for a long time.More than a year ago, we started hitting a gradual memory leak that manifested as:
MaxListenersExceededWarningwarningsAt the time, this was particularly difficult to diagnose because everything appeared correct at the request level. The root cause turned out to be socket reuse combined with duplicate listener attachment on each request.
We ultimately had to identify, debug, and mitigate the issue ourselves in production by introducing a per‑socket guard to prevent re‑instrumentation.
After running with that fix successfully for a long period of time, it felt appropriate to give back to the community by upstreaming the solution, adding coverage, and improving tooling to help prevent regressions.
What’s Included
socket.haveEventsguard to prevent duplicate socket event listeners and cover the behavior with tests.>=15.14.0 <24inpackage.jsonto ensure stable and reproducible test results.How to Verify
socket.haveEventsguard located at line 16 inindex.js, then run the test suite and observe the listener leak.index.jsand confirm the leak is resolved.All tests should pass, and formatting should remain stable across saves.
Disclaimer
The issue description and this MR description were drafted with the assistance of ChatGPT for clarity and structure.
The technical investigation, reproduction, production mitigation, and final implementation were performed manually.
Notes
This change ensures socket reuse with keep-alive agents remains safe over time and prevents listener accumulation that could otherwise lead to memory leaks and warnings.