[Feature] Do not silently discard integer parse errors in CLUSTER INFO / CLUSTER NODES parsing

Open
  • Is something wrong?
#46 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
62/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Active
Tech stack
go, redis
Domain
backend, databases

Research direction

Start in internal/redis/client.go with GetClusterInfo and ParseClusterNodesOutput, then read the existing parser tests. Decide and document whether each malformed integer is warn-and-skip or propagated as an error, keeping valid-output behavior unchanged. Add malformed-input tests that verify warnings or errors, defined zero-value fallback, and no panic.

Written by the indexing model from the issue text.

Description

enhancement

Problem

Robin's Redis client parses CLUSTER INFO and CLUSTER NODES output and silently discards strconv.Atoi errors, defaulting the field to 0 (internal/redis/client.go):

// GetClusterInfo
case "cluster_slots_ok":
    info.SlotsOK, _ = strconv.Atoi(value)
case "cluster_slots_fail":
    info.SlotsFail, _ = strconv.Atoi(value)
case "cluster_known_nodes":
    info.KnownNodes, _ = strconv.Atoi(value)
case "cluster_size":
    info.ClusterSize, _ = strconv.Atoi(value)

// ParseClusterNodesOutput
pingSent, _ := strconv.Atoi(parts[4])
pongRecv, _ := strconv.Atoi(parts[5])
epoch, _   := strconv.Atoi(parts[6])

In practice this is low impact — Redis reliably returns integers for these fields, and a 0 fallback is usually conservative (e.g. SlotsOK == 0 != 16384 makes the cluster look "not ready", triggering retries rather than a false-healthy result or data corruption). The real drawback is that a malformed response would be swallowed silently: a wrong value with no log and no signal, making diagnosis hard if it ever happens.

Proposed solution

Make parse failures observable instead of silent. Suggested approach:

  • On Atoi failure, log a warning including the raw field/value and node address, and skip the field (keep the zero value) rather than dropping the error on the floor.
  • For the health-relevant CLUSTER INFO fields (cluster_slots_ok, cluster_slots_fail, cluster_known_nodes, cluster_size), consider propagating an error from GetClusterInfo (it already returns error) so a genuinely malformed cluster-state response surfaces to the caller instead of being masked as 0.
  • For the informational CLUSTER NODES fields (PingSent, PongRecv, Epoch), a warning-and-skip is sufficient; propagating an error would require changing ParseClusterNodesOutput's signature (currently returns only []ClusterNode) — decide during implementation whether that's warranted.

Acceptance criteria

  • Atoi failures in GetClusterInfo and ParseClusterNodesOutput are logged (warning) with enough context (field name, raw value, node address) instead of silently discarded.
  • Decision documented per field: warn-and-skip vs. error propagation. At minimum, cluster_slots_ok failures are surfaced (log and/or error).
  • No behavior change on well-formed Redis output (existing parse tests still pass).
  • Unit tests cover malformed input for both parsers (non-numeric field ⇒ logged, no panic, defined fallback).

Implementation notes

  • GetClusterInfo already returns error, so propagating is cheap there if chosen.
  • ParseClusterNodesOutput is a free function returning []ClusterNode; changing it to also return an error touches GetClusterNodes and its callers — keep scope in mind, warn-and-skip may be the pragmatic choice.
  • Keep a logger available in the client (or pass one) so the parser can emit the warning.

Severity

Low — robustness/observability improvement, not a fix for an active bug. Redis returns valid integers in normal operation; this hardens against malformed/corrupt responses and makes them diagnosable.

Dominant language
Go
Stars
18
Forks
0
Avg merge
11h 16m
Merged PRs (30d)
11

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from InditexTech/redkey-robin

All issues in InditexTech/redkey-robin

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.