Skip to content

Conversation

gurgunday
Copy link
Contributor

A bug introduced in #5 causes iovalkey to throw asynchronously from time to time and crash the instance

Before this change, if the affected methods could not find a node, they would return undefined; however, after the change, they immediately try to access the redis property even if the node doesn't exist at that time, which might happen for a variety of reasons

This is highly problematic as iovalkey runs timeout-based code periodically, and by their nature, it's very hard to catch them

Here's an example of the exception that will crash the instance:

TypeError: Cannot read properties of undefined (reading 'redis')
    at ConnectionPool.getSampleInstance (/node_modules/iovalkey/built/cluster/ConnectionPool.js:31:50)
    at tryConnection (/node_modules/iovalkey/built/cluster/index.js:455:56)
    at DelayQueue.execute (/node_modules/iovalkey/built/cluster/DelayQueue.js:49:26)
    at /node_modules/iovalkey/built/cluster/DelayQueue.js:32:26
    at wrapper (/node_modules/iovalkey/built/cluster/index.js:301:17)
    at tryNode (/node_modules/iovalkey/built/cluster/index.js:309:24)
    at /node_modules/iovalkey/built/cluster/index.js:325:21
    at /node_modules/iovalkey/built/cluster/index.js:666:24
    at run (/node_modules/iovalkey/built/utils/index.js:116:22)
    at tryCatcher (/node_modules/standard-as-callback/built/utils.js:12:23)

@gurgunday
Copy link
Contributor Author

cc @mcollina

Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day>
@gurgunday gurgunday force-pushed the fix/redis-undefined branch from bc7768e to 9dca1e6 Compare May 27, 2025 14:08
Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day>
Co-authored-by: Martin Slota <martin.slota@workday.com>
@gurgunday gurgunday force-pushed the fix/redis-undefined branch from 20ec11e to 4e1720c Compare May 27, 2025 14:21
Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day>
Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit b3b884c into valkey-io:main May 27, 2025
8 checks passed
@gurgunday gurgunday deleted the fix/redis-undefined branch May 27, 2025 14:31
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 this pull request may close these issues.

2 participants