Delete from RedisCoreModule.connectionTokens during shutdown instead of clearing#307
Open
drew-r wants to merge 1 commit into
Open
Delete from RedisCoreModule.connectionTokens during shutdown instead of clearing#307drew-r wants to merge 1 commit into
drew-r wants to merge 1 commit into
Conversation
bug causing connections not to be quit when >1 named provider
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.
Fixes bug causing connections not to be quit when >1 named provider.
Why it hangs with 2+ Connections:
Shared Static Set:
RedisCoreModule.connectionTokensis a staticSetshared across all module registrations. When you register both e.gREDIS_MAINandREDIS_SUBSCRIBER, both tokens are added to this single staticSet.Multiple Shutdown Invocations: Because
RedisCoreModuleis imported multiple times (once for each connection), NestJS instantiates multiple instances ofRedisCoreModuleand callsonApplicationShutdownon each of them during teardown.The Cleared Set Bug:
First Invocation (
onApplicationShutdownof Instance 1):REDIS_MAIN_...andREDIS_SUBSCRIBER_...).REDIS_MAIN_...,this.moduleRef.get()succeeds, and it successfully quits the connection gracefully.REDIS_SUBSCRIBER_...,this.moduleRef.get()fails and throws an exception because that provider belongs to a different dynamic module's local scope/context. This error is silently caught and swallowed by the emptycatch {}block.RedisCoreModule_1.connectionTokens.clear(). This wipes out the static Set entirely.Second Invocation (
onApplicationShutdownof Instance 2):connectionTokensSet was cleared in the previous step, it is now empty.REDIS_SUBSCRIBER) is never quit or disconnected.Replication steps:
Set
app.module.tslike soThen run
npm run test:e2e.Expected:
Executes test, passes + exits.
Actual:
Executes test, passes + outputs
& doesn't exit.