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

Hang up on auto reconnect #430

Closed
yahiro07 opened this issue Mar 22, 2024 · 1 comment · Fixed by #431
Closed

Hang up on auto reconnect #430

yahiro07 opened this issue Mar 22, 2024 · 1 comment · Fixed by #431

Comments

@yahiro07
Copy link
Contributor

Hello, Thank you for your help the other day.
By the way, I'm facing another problem recently.

In my application, sometimes the server hangs up and stops responding from then on.
I debugged it and found that it occurs in the auto-reconnect process of deno-redis.

I identified this part causes the problem.
https://github.com/denodrivers/redis/blob/28ead060aa467c1558b82b253c0f2f3f9bf10f40/connection.ts#L216C1-L221C10

When an error raised in processCommandQueue, it tries to reconnect to the server by backing off.
In that case, the call stack would be

sendCommand ...(1)
--> processCommandQueue
(some error raised in command.execeute)
--> connect
--> #connect
--> authenticate or selectDb
--> sendCommand ...(2)

On sendCommand ...(1), the command is pushed to the queue and processCommandQueue is executed.
But in sendCommand ...(2), the command is pushed, but processCommandQueue is not called this time.
The promise of sendCommand ...(2) never resolves and it's the same for all other subsequent sendCommand invocations.

I wrote a test to reproduce this.

yahiro07@347acb3

This occurs when db or password is specified in a connection option.

I think #318 might be related to this, but I'm not sure since it's reported before the auto reconnect feature was added (#349)

Althought I'm not sure I could do it, I'd like to give it a try to fix this.

Thanks,
yahiro

@yahiro07
Copy link
Contributor Author

Hi, I made a fix for this.
Please check it when you have free time.

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 a pull request may close this issue.

1 participant