Skip to content

fix createCluster - copy options.defaults.socket before modifying it #2783

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

soccermax
Copy link
Contributor

Description

Describe your pull request here

#2373 introduced a bug into the creation of cluster clients. The issue is that provided options for creating a socket are changed. The function clientOptionsDefaults in cluster-slots.ts is called twice once with disableReconnect false and the second call is the real call for creating clients but the function clientOptionsDefaults is overriding this.#options.defaults.socket at line 289 because result.socket is the same reference as this.#options.defaults.socket

fix #2721


Checklist

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

@soccermax soccermax changed the title shallow copy of this.#options.defaults.socket fix createClusterClient - shallow copy of this.#options.defaults.socket Jul 2, 2024
Copy link
Contributor

@leibale leibale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I'm working on releasing it right now.. :)

@soccermax
Copy link
Contributor Author

Good catch! I'm working on releasing it right now.. :)

nice, thanks for the fast reaction

@leibale leibale changed the title fix createClusterClient - shallow copy of this.#options.defaults.socket fix createCluster - copy options.defaults.socket before modifying it Jul 2, 2024
@leibale leibale merged commit 4ac97ee into redis:master Jul 2, 2024
12 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

createCluster clients don't handle on('error') correctly
2 participants