From 1cafa22b41b0eb06220fb0ba81f42911a54af961 Mon Sep 17 00:00:00 2001 From: fuleinist Date: Wed, 3 Jun 2026 05:19:01 +1000 Subject: [PATCH] fix(redis): preserve empty-string values in getHashFieldsBatch 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 #3530 --- server/_shared/redis.ts | 4 ++- tests/redis-caching.test.mjs | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/server/_shared/redis.ts b/server/_shared/redis.ts index 23156d0a28..55c7e5410b 100644 --- a/server/_shared/redis.ts +++ b/server/_shared/redis.ts @@ -738,7 +738,9 @@ export async function getHashFieldsBatch(key: string, fields: string[], raw = fa const values = data[0]?.result; if (values) { for (let i = 0; i < fields.length; i++) { - if (values[i]) result.set(fields[i]!, values[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]!); } } } catch (err) { diff --git a/tests/redis-caching.test.mjs b/tests/redis-caching.test.mjs index 0ff04756ca..0b6799d879 100644 --- a/tests/redis-caching.test.mjs +++ b/tests/redis-caching.test.mjs @@ -1414,3 +1414,50 @@ describe('setCachedJson wire shape and failure reporting', { concurrency: 1 }, ( } }); }); + +describe('getHashFieldsBatch empty-string handling (#3530)', { concurrency: 1 }, () => { + it('preserves empty-string values, omits null/missing, and retains real strings', async () => { + // Regression: getHashFieldsBatch used a truthy check (`if (values[i])`) that + // dropped valid empty-string hash values. Real Redis hash values are + // allowed to be the empty string, so a caller that round-trips "" will + // silently lose it. The fix switches to a null/undefined check so "" is + // preserved. + const redis = await importRedisFresh(); + const restoreEnv = withEnv({ + UPSTASH_REDIS_REST_URL: 'https://redis.test', + UPSTASH_REDIS_REST_TOKEN: 'token', + VERCEL_ENV: undefined, + VERCEL_GIT_COMMIT_SHA: undefined, + }); + const originalFetch = globalThis.fetch; + + let pipelineCalls = 0; + globalThis.fetch = async (_url, init = {}) => { + pipelineCalls += 1; + const pipeline = JSON.parse(String(init.body)); + assert.equal(pipeline.length, 1); + assert.equal(pipeline[0][0], 'HMGET'); + assert.deepEqual(pipeline[0].slice(2), ['name', 'empty', 'missing', 'real']); + // Upstash HMGET returns an array of (string | null) in field order: + // - real value -> "alice" + // - valid "" -> "" + // - missing key -> null + // - real value -> "ok" + return jsonResponse([{ result: ['alice', '', null, 'ok'] }]); + }; + + try { + const map = await redis.getHashFieldsBatch('user:42', ['name', 'empty', 'missing', 'real']); + assert.equal(pipelineCalls, 1, 'should batch into one HMGET pipeline call'); + assert.equal(map.get('name'), 'alice', 'non-empty value is kept'); + assert.equal(map.get('empty'), '', 'empty-string value must be preserved (this is the bug)'); + assert.equal(map.has('missing'), false, 'null entries are omitted'); + assert.equal(map.get('real'), 'ok', 'non-empty value is kept'); + assert.equal(map.size, 3, 'map should contain only the three resolvable fields'); + } finally { + globalThis.fetch = originalFetch; + restoreEnv(); + } + }); +}); +