Skip to content

fix(redis): preserve empty-string values in getHashFieldsBatch#4060

Open
fuleinist wants to merge 1 commit into
koala73:mainfrom
fuleinist:fix/redis-gethashfieldsbatch-empty-string
Open

fix(redis): preserve empty-string values in getHashFieldsBatch#4060
fuleinist wants to merge 1 commit into
koala73:mainfrom
fuleinist:fix/redis-gethashfieldsbatch-empty-string

Conversation

@fuleinist
Copy link
Copy Markdown
Contributor

Summary

getHashFieldsBatch in server/_shared/redis.ts used a truthy check (if (values[i])) on HMGET pipeline results. That treats the empty string "" the same as null / missing, so any caller that legitimately stores an empty-string hash value in Redis silently loses it on read.

This is a one-line correctness fix: switch the truthy check to a null/undefined check (if (values[i] != null)) so "" round-trips intact.

Problem

// server/_shared/redis.ts:740 (before)
for (let i = 0; i < fields.length; i++) {
  if (values[i]) result.set(fields[i]!, values[i]!);
}

Upstash's HMGET pipeline response is Array<(string | null)> in field order — null means the key is missing, and any string (including "") is the stored value. The truthy check cannot distinguish "field is present and empty" from "field is missing", so the caller gets an empty Map for the affected key instead of Map { field -> "" }.

Current callers store JSON or non-empty strings so the bug is rarely hit, but it will surprise the next caller that legitimately needs to round-trip an empty string.

Solution

// server/_shared/redis.ts:740 (after)
for (let i = 0; i < fields.length; i++) {
  // Use a null/undefined check rather than a truthy test: "" is a
  // legitimate Redis hash value and must be preserved (see #3530).
  if (values[i] != null) result.set(fields[i]!, values[i]!);
}

!= null matches both null and undefined and is the idiomatic way to express "value is present" in TypeScript.

Testing

  • Unit test added for the bug (tests/redis-caching.test.mjs)
  • Test fails on the unfixed code (verified — pre-fix run produced undefined !== '')
  • Test passes on the fixed code (verified — 29/29 green)
  • Existing tests still pass (28 prior tests still green)

New test asserts the full HMGET return mix:

result: ['alice', '', null, 'ok']
fields: ['name',  'empty',  'missing',  'real']
map.get('name')    === 'alice'   // non-empty kept
map.get('empty')   === ''        // empty-string preserved (the fix)
map.has('missing') === false     // null omitted
map.get('real')    === 'ok'      // non-empty kept
map.size           === 3         // only the 3 resolvable fields

Files Changed

  • server/_shared/redis.ts — 1 line changed, 2 lines of comment added
  • tests/redis-caching.test.mjs — 47 lines added (new describe block)

Fixes #3530

What was wrong and what this fixes.

## Changes
- server/_shared/redis.ts: replace the truthy check on HMGET results
  (if (values[i])) with a null/undefined check (if (values[i] != null))
  so valid empty-string Redis hash values are no longer silently dropped.
- tests/redis-caching.test.mjs: add a regression test that mocks the
  HMGET pipeline response with a mix of real values, '', and null, and
  asserts that '' is preserved in the returned Map.

## Testing
- [x] Unit test added for the bug (tests/redis-caching.test.mjs)
- [x] Test fails on the unfixed code (verified)
- [x] Test passes on the fixed code (verified; 29/29 green)
- [x] Existing tests still pass (28 prior tests still green)

Fixes koala73#3530
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 2, 2026

@fuleinist is attempting to deploy a commit to the World Monitor Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added the trust:safe Brin: contributor trust score safe label Jun 2, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 2, 2026

Greptile Summary

This PR fixes a correctness bug in getHashFieldsBatch where a truthy check (if (values[i])) incorrectly dropped valid empty-string values returned by Redis HMGET, treating them the same as null (missing field). The fix replaces it with a loose null-equality check (!= null) that correctly distinguishes "field is missing" from "field is present but empty".

  • server/_shared/redis.ts: Single-line change from if (values[i]) to if (values[i] != null) with an explanatory comment.
  • tests/redis-caching.test.mjs: New regression test covering all four HMGET response variants — non-empty string, empty string, null/missing, and a second non-empty string — asserting correct Map contents and size.

Confidence Score: 5/5

Safe to merge — the change is a minimal, well-tested correctness fix with no behavioral change for existing callers that store non-empty strings.

The fix is a single-character change from a truthy guard to a null-equality guard. Existing callers are unaffected because they store non-empty JSON or string values. The new regression test covers all four possible HMGET response states. No edge cases are introduced.

No files require special attention.

Important Files Changed

Filename Overview
server/_shared/redis.ts One-line correctness fix: replaces truthy check with != null so empty-string Redis hash values are preserved on read.
tests/redis-caching.test.mjs Adds a focused regression test covering all four HMGET response cases: non-empty string, empty string, null/missing, and another non-empty string.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant getHashFieldsBatch
    participant Upstash REST API

    Caller->>getHashFieldsBatch: key, fields ["name","empty","missing","real"]
    getHashFieldsBatch->>Upstash REST API: POST /pipeline [["HMGET", key, ...fields]]
    Upstash REST API-->>getHashFieldsBatch: [{result: ["alice", "", null, "ok"]}]
    note over getHashFieldsBatch: for each i: if (values[i] != null)<br/>  → "" passes (fix)<br/>  → null is skipped
    getHashFieldsBatch-->>Caller: "Map { "name"→"alice", "empty"→"", "real"→"ok" }"
Loading

Reviews (1): Last reviewed commit: "fix(redis): preserve empty-string values..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

trust:safe Brin: contributor trust score safe

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getHashFieldsBatch silently drops empty-string Redis hash values

1 participant