From cee0cd62a9ee40406475b72b564331602d3537be Mon Sep 17 00:00:00 2001 From: anonpenguin23 Date: Fri, 7 Nov 2025 06:03:47 +0200 Subject: [PATCH] Enhance CacheClient to handle cache misses gracefully - Updated the `get` method in CacheClient to return null for cache misses instead of throwing an error. - Improved error handling to differentiate between expected 404 errors and other exceptions. - Adjusted related tests to verify null returns for non-existent keys, ensuring robust cache behavior. --- src/cache/client.ts | 26 +++++++++++++++++++++----- src/core/http.ts | 6 ++++-- tests/e2e/cache.test.ts | 39 ++++++++++++++++++--------------------- 3 files changed, 43 insertions(+), 28 deletions(-) diff --git a/src/cache/client.ts b/src/cache/client.ts index 0bbdea2..89b05f2 100644 --- a/src/cache/client.ts +++ b/src/cache/client.ts @@ -1,4 +1,5 @@ import { HttpClient } from "../core/http"; +import { SDKError } from "../errors"; export interface CacheGetRequest { dmap: string; @@ -67,12 +68,27 @@ export class CacheClient { /** * Get a value from cache + * Returns null if the key is not found (cache miss/expired), which is normal behavior */ - async get(dmap: string, key: string): Promise { - return this.httpClient.post("/v1/cache/get", { - dmap, - key, - }); + async get(dmap: string, key: string): Promise { + try { + return await this.httpClient.post("/v1/cache/get", { + dmap, + key, + }); + } catch (error) { + // Cache misses (404 or "key not found" messages) are normal behavior - return null instead of throwing + if ( + error instanceof SDKError && + (error.httpStatus === 404 || + (error.httpStatus === 500 && + error.message?.toLowerCase().includes("key not found"))) + ) { + return null; + } + // Re-throw other errors (network issues, server errors, etc.) + throw error; + } } /** diff --git a/src/core/http.ts b/src/core/http.ts index fb5e515..14035ea 100644 --- a/src/core/http.ts +++ b/src/core/http.ts @@ -152,11 +152,13 @@ export class HttpClient { } catch (error) { const duration = performance.now() - startTime; if (typeof console !== "undefined") { - // Cache "key not found" (404) is expected behavior - don't log as error + // Cache "key not found" (404 or error message) is expected behavior - don't log as error const isCacheGetNotFound = path === "/v1/cache/get" && error instanceof SDKError && - error.httpStatus === 404; + (error.httpStatus === 404 || + (error.httpStatus === 500 && + error.message?.toLowerCase().includes("key not found"))); // "Not found" (404) for blocked_users is expected behavior - don't log as error // This happens when checking if users are blocked (most users aren't blocked) diff --git a/tests/e2e/cache.test.ts b/tests/e2e/cache.test.ts index ff96c28..ae750eb 100644 --- a/tests/e2e/cache.test.ts +++ b/tests/e2e/cache.test.ts @@ -42,9 +42,10 @@ describe("Cache", () => { // Get value const getResult = await client.cache.get(testDMap, testKey); - expect(getResult.key).toBe(testKey); - expect(getResult.value).toBe(testValue); - expect(getResult.dmap).toBe(testDMap); + expect(getResult).not.toBeNull(); + expect(getResult!.key).toBe(testKey); + expect(getResult!.value).toBe(testValue); + expect(getResult!.dmap).toBe(testDMap); }); it("should put and get complex objects", async () => { @@ -61,9 +62,10 @@ describe("Cache", () => { // Get object const getResult = await client.cache.get(testDMap, testKey); - expect(getResult.value).toBeDefined(); - expect(getResult.value.name).toBe(testValue.name); - expect(getResult.value.age).toBe(testValue.age); + expect(getResult).not.toBeNull(); + expect(getResult!.value).toBeDefined(); + expect(getResult!.value.name).toBe(testValue.name); + expect(getResult!.value.age).toBe(testValue.age); }); it("should put value with TTL", async () => { @@ -82,7 +84,8 @@ describe("Cache", () => { // Verify value exists const getResult = await client.cache.get(testDMap, testKey); - expect(getResult.value).toBe(testValue); + expect(getResult).not.toBeNull(); + expect(getResult!.value).toBe(testValue); }); it("should delete a value", async () => { @@ -95,20 +98,17 @@ describe("Cache", () => { // Verify it exists const before = await client.cache.get(testDMap, testKey); - expect(before.value).toBe(testValue); + expect(before).not.toBeNull(); + expect(before!.value).toBe(testValue); // Delete value const deleteResult = await client.cache.delete(testDMap, testKey); expect(deleteResult.status).toBe("ok"); expect(deleteResult.key).toBe(testKey); - // Verify it's deleted - try { - await client.cache.get(testDMap, testKey); - expect.fail("Expected get to fail after delete"); - } catch (err: any) { - expect(err.message).toBeDefined(); - } + // Verify it's deleted (should return null, not throw) + const after = await client.cache.get(testDMap, testKey); + expect(after).toBeNull(); }); it("should scan keys", async () => { @@ -148,12 +148,9 @@ describe("Cache", () => { const client = await createTestClient(); const nonExistentKey = "non-existent-key"; - try { - await client.cache.get(testDMap, nonExistentKey); - expect.fail("Expected get to fail for non-existent key"); - } catch (err: any) { - expect(err.message).toBeDefined(); - } + // Cache misses should return null, not throw an error + const result = await client.cache.get(testDMap, nonExistentKey); + expect(result).toBeNull(); }); it("should handle empty dmap name", async () => {