fix(everything): iterate the transports Map with for...of, not for...in - #4768
Open
simpleqt wants to merge 1 commit into
Open
fix(everything): iterate the transports Map with for...of, not for...in#4768simpleqt wants to merge 1 commit into
simpleqt wants to merge 1 commit into
Conversation
for...in on a Map enumerates own enumerable properties of the object (none), so the SIGINT shutdown loop never ran and active session transports were never closed. Switched to for...of and reused the destructured transport instead of re-fetching it.
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.
Summary
The SIGINT handler in
src/everything/transports/streamableHttp.tsiterated thetransportsMap<string, StreamableHTTPServerTransport>withfor (const sessionId in transports).for...inenumerates own enumerable properties of the object — aMaphas none — so the shutdown loop never executed: active sessions were never closed on Ctrl-C and the "Closing transport for session …" log never appeared.Switched to
for (const [sessionId, transport] of transports), reusing the destructured transport instead of re-fetching it from the Map. Introduced in refactor commit 5de886c; still present at tip.Testing
Static-analysis provable (
for...inover aMapyields no entries). Manual repro: startnpx server-everything streamableHttp, connect a client, send SIGINT — before the fix no close logs appear; after the fix each session transport closes.