diff --git a/.changeset/shaggy-geese-flow.md b/.changeset/shaggy-geese-flow.md new file mode 100644 index 0000000000..860a10e625 --- /dev/null +++ b/.changeset/shaggy-geese-flow.md @@ -0,0 +1,9 @@ +--- +"miniflare": patch +--- + +Fix KV bulk `getWithMetadata` dropping metadata for present-but-falsy values + +The bulk-get handler decided whether to wrap an entry as `{ value, metadata }` based on the truthiness of the value rather than on whether the key existed. Keys holding a falsy value — an empty string, or `0`, `false` and `null` when reading with `type: "json"` — were returned as bare values, losing their metadata and diverging from the shape the runtime binding expects for present keys. Empty-string values were the most common trigger. + +The check now gates on entry presence, so present keys always keep their `{ value, metadata }` wrapper and only missing keys return a bare `null`. diff --git a/packages/miniflare/src/workers/kv/namespace.worker.ts b/packages/miniflare/src/workers/kv/namespace.worker.ts index 2796c59614..a73a37d61e 100644 --- a/packages/miniflare/src/workers/kv/namespace.worker.ts +++ b/packages/miniflare/src/workers/kv/namespace.worker.ts @@ -79,10 +79,14 @@ async function processKeyValue( type: "text" | "json" = "text", withMetadata = false ) { + // `obj` is `null` only when the key is missing. A present key always has a + // `value` stream, so presence — not value truthiness — decides the response + // shape: present keys are wrapped as `{ value, metadata }`, missing keys are + // returned as a bare `null`. const decoder = new TextDecoder(); let decodedValue = ""; - if (obj?.value) { - for await (const chunk of obj?.value) { + if (obj !== null) { + for await (const chunk of obj.value) { decodedValue += decoder.decode(chunk, { stream: true }); } decodedValue += decoder.decode(); @@ -91,22 +95,23 @@ async function processKeyValue( let val = null; const size = decodedValue.length; try { - val = !obj?.value - ? null - : type === "json" - ? JSON.parse(decodedValue) - : decodedValue; + val = + obj === null + ? null + : type === "json" + ? JSON.parse(decodedValue) + : decodedValue; } catch { throw new HttpError( 400, `At least one of the requested keys corresponds to a non-${type} value` ); } - if (val && withMetadata) { + if (obj !== null && withMetadata) { return [ { value: val, - metadata: obj?.metadata ?? null, + metadata: obj.metadata ?? null, }, size, ]; diff --git a/packages/miniflare/test/plugins/kv/index.spec.ts b/packages/miniflare/test/plugins/kv/index.spec.ts index bb16aed11c..787efd1e3b 100644 --- a/packages/miniflare/test/plugins/kv/index.spec.ts +++ b/packages/miniflare/test/plugins/kv/index.spec.ts @@ -253,6 +253,57 @@ test("bulk get: check metadata as string", async ({ expect }) => { expect(result).toEqual(expectedResult); }); +test("bulk get: check metadata for empty string value", async ({ expect }) => { + const { kv } = ctx; + await kv.put("key1", "", { metadata: { testing: true } }); + + const result = await kv.getWithMetadata(["key1"]); + const expectedResult = new Map([ + ["key1", { value: "", metadata: { testing: true } }], + ]); + expect(result).toEqual(expectedResult); +}); + +test("bulk get: check metadata for falsy json values", async ({ expect }) => { + const { kv } = ctx; + await kv.put("key1", "0", { metadata: { testing: "zero" } }); + await kv.put("key2", "false", { metadata: { testing: "false" } }); + await kv.put("key3", "null", { metadata: { testing: "null" } }); + + const result = await kv.getWithMetadata(["key1", "key2", "key3"], "json"); + const expectedResult = new Map([ + ["key1", { value: 0, metadata: { testing: "zero" } }], + ["key2", { value: false, metadata: { testing: "false" } }], + ["key3", { value: null, metadata: { testing: "null" } }], + ]); + expect(result).toEqual(expectedResult); +}); + +test("bulk get: distinguishes falsy values from missing keys", async ({ + expect, +}) => { + const { kv } = ctx; + await kv.put("key1", "0", { metadata: { testing: true } }); + + // A present key holding a falsy value keeps its `{ value, metadata }` + // wrapper, while a missing key is still returned as a bare `null` + const result = await kv.getWithMetadata(["key1", "key2"], "json"); + const expectedResult = new Map([ + ["key1", { value: 0, metadata: { testing: true } }], + ["key2", null], + ]); + expect(result).toEqual(expectedResult); +}); + +test("bulk get: returns falsy values without metadata", async ({ expect }) => { + const { kv } = ctx; + await kv.put("key1", ""); + await kv.put("key2", "0"); + + expect(await kv.get(["key1"])).toEqual(new Map([["key1", ""]])); + expect(await kv.get(["key2"], "json")).toEqual(new Map([["key2", 0]])); +}); + test("bulk get: get with metadata for 404", async ({ expect }) => { const { kv } = ctx;