fix(consumer): guard against disconnect() re-arming the reconnect loop - #55
Open
arsalan507 wants to merge 1 commit into
Open
fix(consumer): guard against disconnect() re-arming the reconnect loop#55arsalan507 wants to merge 1 commit into
arsalan507 wants to merge 1 commit into
Conversation
connection.close() (called from disconnect() via super.disconnect()) still emits 'close' on the connection object, the same event the reconnect listener in connect() is watching for. Without a way to tell a deliberate shutdown apart from a broker drop, disconnect() itself triggered handleReconnect() and resurrected the connection a caller had just intentionally closed. Add an intentionalDisconnect flag: disconnect() sets it before closing, the 'close' listener no-ops when it is set, and a fresh connect() clears it so a later broker drop on the same lifecycle still reconnects. Fixes bitrix24#27
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
1 participant
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
Fixes #27.
connection.close()— called bydisconnect()viasuper.disconnect()— still emits'close'on the connection object, the exact same event the reconnect listener registered inconnect()is watching for:So a worker calling
consumer.disconnect()to shut down cleanly would have its own close triggerhandleReconnect(), which reconnects and reasserts the topology — resurrecting a connection the caller deliberately tore down.Fix
Added an
intentionalDisconnectflag:disconnect()sets it totruebefore callingsuper.disconnect()(which closes the channel/connection).'close'listener checks the flag and no-ops instead of callinghandleReconnect()when it's set.connect()resets the flag tofalse, so a later broker-initiated drop on the same lifecycle still reconnects normally.Test plan
tests/consumer.test.ts) that callsdisconnect(), simulates the resulting'close'event (the fake connection'sclose()doesn't auto-emit it, so the test emits it explicitly to mirror real amqplib behavior), and asserts nosetTimeout(i.e. no reconnect attempt) is scheduled. Verified it fails against the pre-fix code (setTimeoutcalled with the reconnect interval) and passes after the fix.pnpm lint,pnpm typecheck,pnpm test(76 passing),pnpm buildall green.