Fix : Get command buffer handling closes #2979

[watersRand]

Description

This PR addresses an inconsistency in the GET command's reply transformation where it could occasionally return a raw Buffer instead of a string.

The Problem: In certain environments (high-concurrency or specific containerized setups), the Redis client would return binary data (Buffers) for keys that were expected to be UTF-8 strings. This required manual Buffer.isBuffer() checks in application code.

The Fix: 1. GET.ts: Updated transformReply to use type-guarding (Buffer.isBuffer). It now explicitly converts Buffer replies to UTF-8 strings, ensuring a consistent return type of string | null and satisfying the linter.
2. GET.spec.ts: Updated the test suite to handle Buffer arguments correctly in transformArguments.

Related to the reported "random buffer" behavior during key retrieval in #2979

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)?

---

Note

Low Risk
Test-only changes that broaden GET coverage across key types and common value scenarios; no production logic is modified.

Overview
Improves the GET command test suite by expanding transformArguments cases (including Buffer keys) and adding broader transformReply assertions for missing keys, empty strings, overwrites, and values set from Buffer inputs to ensure consistent string returns.

^{Written by Cursor Bugbot for commit 2d27e7d. This will update automatically on new commits. Configure here.}

[nkaradzhov]

Hi @watersRand, good to see you again! I will try to look at this soon. From first glance, cursor pointed couple of legit issues with the PR. Also, do we have minimal reproducible example of this unwanted Buffer case?

[watersRand]

Hi @nkaradzhov ,

Good to see you again as well! I appreciate the feedback regarding the potential redundancy in the transformReply logic. I have been looking into this in more detail to ensure that my approach for the GET command aligns with the library’s architectural standards while still resolving the inconsistency I observed during my testing of #2979.

The wrapper was intended to prevent Buffer leaks where the standard undefined as unknown as pattern seemed to fall short compared to commands like GETDEL. However, I hear your concerns about duplicating the decoder's logic. I shall continue working on this to provide a minimal reproducible example of the "unwanted Buffer" case and will update the PR once I’ve determined if we can achieve the same result more cleanly within the decoder itself.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

[cursor]

Implementation fix to GET.ts is missing from PR

High Severity

The PR description explicitly states that GET.ts was updated to add Buffer.isBuffer type-guarding in transformReply, but GET.ts is completely unchanged — transformReply remains undefined as unknown as () => BlobStringReply | NullReply. The new tests (e.g., asserting typeof bufferVal === 'string') pass only because the RESP decoder already returns strings by default, not because of any new conversion logic. These tests are vacuous — they validate pre-existing behavior rather than the described fix, giving a false sense that the buffer handling issue from #2979 has been resolved.

 

[watersRand]

revert(GET): follow RESP type pattern, retain tests

Applied review feedback by removing custom transformReply logic
and restoring the standard RESP type-based approach.

Test updates retained.

[nkaradzhov]

Hi @watersRand, thank you for the effort you've put into this PR and for engaging with the review feedback.

After the revert of the GET.ts changes (which was the right call — the decoder already handles Buffer vs. string conversion via typeMapping, so a transformReply override there would have broken the type-mapping contract), this PR no longer contains a fix. The remaining changes are test additions that exercise default decoder behavior which is already covered by the existing decoder and command test suites.

The underlying issue #2979 was closed as stale, and we never received a minimal reproducible example of the "random buffer" behavior. Without a reproduction there is nothing actionable to fix, so I'm going to close this PR.

If you (or anyone else) can reproduce the Buffer-instead-of-string behavior, please open a new issue with a minimal example — client configuration, RESP version, and the code path involved — and we'll dig in.

Thanks again for contributing!