From 1f1031b6383fb6ff5ffea9d6f19aa8a9f5c83bad Mon Sep 17 00:00:00 2001 From: Daniel Beckham Date: Mon, 30 Mar 2026 09:25:05 -0500 Subject: [PATCH] Clear pending promise on rejection in memoized function The memoized() function was caching failed promises permanently, causing all subsequent calls to return the same rejected promise. This prevented retries after timeout or rejection errors. Changes: - Added rejection handler to clear 'pending' variable on promise failure - Allows retries after failures instead of permanent failure state - Added tests to cover successful caching, rejection handling, and retry behavior Fixes the issue where workspace initialization timeouts would permanently block all script operations with 'lock timeout' errors until server restart. --- src/lib/memoized.ts | 21 +++-- test/memoized.test.ts | 183 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+), 7 deletions(-) create mode 100644 test/memoized.test.ts diff --git a/src/lib/memoized.ts b/src/lib/memoized.ts index b33cb95..c240f08 100644 --- a/src/lib/memoized.ts +++ b/src/lib/memoized.ts @@ -1,5 +1,5 @@ /** - * Creates a memoized version of a function that caches its result forever. + * Creates a memoized version of a function that caches its successful result forever. * The function is called once, and subsequent calls return the cached result. * * @param fn - Function to memoize (can be sync or async) @@ -46,12 +46,19 @@ export function memoized(fn: (() => Promise) | (() => T)) { // Detect if result is a promise if (result && typeof (result as any).then === "function") { isAsync = true; - pending = (result as Promise).then((v) => { - value = v; - cached = true; - pending = null; - return v; - }); + pending = (result as Promise).then( + (v) => { + value = v; + cached = true; + pending = null; + return v; + }, + (err) => { + // Clear pending on rejection so retries are possible + pending = null; + throw err; + }, + ); return pending; } else { // Synchronous result diff --git a/test/memoized.test.ts b/test/memoized.test.ts new file mode 100644 index 0000000..cfc000d --- /dev/null +++ b/test/memoized.test.ts @@ -0,0 +1,183 @@ +import { describe, expect, it } from "vitest"; +import { memoized } from "../src/lib/memoized.js"; + +describe("memoized", () => { + describe("successful memoization", () => { + it("should cache successful sync results", () => { + let callCount = 0; + const fn = memoized(() => { + callCount++; + return "result"; + }); + + const result1 = fn(); + const result2 = fn(); + const result3 = fn(); + + expect(result1).toBe("result"); + expect(result2).toBe("result"); + expect(result3).toBe("result"); + expect(callCount).toBe(1); // Function called only once + }); + + it("should cache successful async results", async () => { + let callCount = 0; + const fn = memoized(async () => { + callCount++; + await new Promise((resolve) => setTimeout(resolve, 10)); + return "async-result"; + }); + + const result1 = await fn(); + const result2 = await fn(); + const result3 = await fn(); + + expect(result1).toBe("async-result"); + expect(result2).toBe("async-result"); + expect(result3).toBe("async-result"); + expect(callCount).toBe(1); // Function called only once + }); + + it("should return the same promise while async call is in-flight", async () => { + let callCount = 0; + const fn = memoized(async () => { + callCount++; + await new Promise((resolve) => setTimeout(resolve, 100)); + return "result"; + }); + + // Start multiple calls simultaneously + const promise1 = fn(); + const promise2 = fn(); + const promise3 = fn(); + + // All should be the same promise + expect(promise1).toBe(promise2); + expect(promise2).toBe(promise3); + + const results = await Promise.all([promise1, promise2, promise3]); + expect(results).toEqual(["result", "result", "result"]); + expect(callCount).toBe(1); // Function called only once + }); + }); + + describe("promise rejection handling", () => { + it("should clear pending on promise rejection", async () => { + let callCount = 0; + const fn = memoized(async () => { + callCount++; + if (callCount === 1) { + throw new Error("First call fails"); + } + return "success"; + }); + + // First call should fail + await expect(fn()).rejects.toThrow("First call fails"); + expect(callCount).toBe(1); + + // Second call should retry and succeed + const result = await fn(); + expect(result).toBe("success"); + expect(callCount).toBe(2); + }); + + it("should not cache failed values", async () => { + let callCount = 0; + const fn = memoized(async () => { + callCount++; + throw new Error(`Failure ${callCount}`); + }); + + // Multiple failed calls should each throw + await expect(fn()).rejects.toThrow("Failure 1"); + await expect(fn()).rejects.toThrow("Failure 2"); + await expect(fn()).rejects.toThrow("Failure 3"); + + expect(callCount).toBe(3); // Each call retries + }); + + it("should allow retry after rejection", async () => { + let shouldFail = true; + let callCount = 0; + + const fn = memoized(async () => { + callCount++; + if (shouldFail) { + throw new Error("Temporary failure"); + } + return "recovered"; + }); + + // First call fails + await expect(fn()).rejects.toThrow("Temporary failure"); + expect(callCount).toBe(1); + + // Fix the condition + shouldFail = false; + + // Retry should succeed + const result = await fn(); + expect(result).toBe("recovered"); + expect(callCount).toBe(2); + + // Subsequent calls should use cached success + const result2 = await fn(); + expect(result2).toBe("recovered"); + expect(callCount).toBe(2); // No additional calls + }); + + it("should propagate the original error", async () => { + const customError = new Error("Custom error message"); + const fn = memoized(async () => { + throw customError; + }); + + try { + await fn(); + expect.fail("Should have thrown"); + } catch (err) { + expect(err).toBe(customError); + expect((err as Error).message).toBe("Custom error message"); + } + }); + }); + + describe("mixed scenarios", () => { + it("should handle success after multiple failures", async () => { + let attempts = 0; + const fn = memoized(async () => { + attempts++; + if (attempts < 3) { + throw new Error(`Attempt ${attempts} failed`); + } + return "finally succeeded"; + }); + + await expect(fn()).rejects.toThrow("Attempt 1 failed"); + await expect(fn()).rejects.toThrow("Attempt 2 failed"); + + const result = await fn(); + expect(result).toBe("finally succeeded"); + expect(attempts).toBe(3); + + // Verify success is cached + const result2 = await fn(); + expect(result2).toBe("finally succeeded"); + expect(attempts).toBe(3); // No additional attempts + }); + + it("should not interfere with sync function behavior", () => { + let callCount = 0; + const syncFn = memoized(() => { + callCount++; + return callCount; + }); + + expect(syncFn()).toBe(1); + expect(syncFn()).toBe(1); // Cached + expect(syncFn()).toBe(1); // Cached + expect(callCount).toBe(1); + }); + }); +});