Fix BUG-099: Add TTL mechanism to provisionedSessions to prevent memory leak
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 12m22s
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 12m22s
- Replace unbounded Set with Map<sessionId, timestamp> tracking insertion time - Add periodic cleanup every hour to remove entries older than 24h - Add on-demand cleanup before duplicate checks for timely cleanup - Add comprehensive TDD tests verifying TTL behavior: * Fresh entries work correctly * Stale entries (>24h) get cleaned up * Fresh entries survive cleanup * Bounded size with many entries - All 447 tests pass including 4 new TTL tests - Memory leak fixed while preserving DB-level deduplication
This commit is contained in:
parent
024fa0084d
commit
5f776db662
2 changed files with 171 additions and 6 deletions
|
|
@ -486,3 +486,138 @@ describe("POST /v1/billing/webhook", () => {
|
||||||
expect(updateEmailByCustomer).toHaveBeenCalledWith("cus_email", "newemail@test.com");
|
expect(updateEmailByCustomer).toHaveBeenCalledWith("cus_email", "newemail@test.com");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("Provisioned Sessions TTL (Memory Leak Fix)", () => {
|
||||||
|
it("should allow fresh entries that haven't expired", async () => {
|
||||||
|
mockStripe.checkout.sessions.retrieve.mockResolvedValue({
|
||||||
|
id: "cs_fresh",
|
||||||
|
customer: "cus_fresh",
|
||||||
|
customer_details: { email: "fresh@test.com" },
|
||||||
|
});
|
||||||
|
|
||||||
|
// First call - should provision
|
||||||
|
const res1 = await request(app).get("/v1/billing/success?session_id=cs_fresh");
|
||||||
|
expect(res1.status).toBe(200);
|
||||||
|
expect(res1.text).toContain("Welcome to Pro");
|
||||||
|
|
||||||
|
// Second call immediately - should be duplicate (409)
|
||||||
|
const res2 = await request(app).get("/v1/billing/success?session_id=cs_fresh");
|
||||||
|
expect(res2.status).toBe(409);
|
||||||
|
expect(res2.body.error).toContain("already been used");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should remove stale entries older than 24 hours from provisionedSessions", async () => {
|
||||||
|
// This test will verify that the cleanup mechanism removes old entries
|
||||||
|
// For now, this will fail because the current implementation doesn't have TTL
|
||||||
|
|
||||||
|
// Mock Date.now to control time
|
||||||
|
const originalDateNow = Date.now;
|
||||||
|
let currentTime = 1640995200000; // Jan 1, 2022 00:00:00 GMT
|
||||||
|
vi.spyOn(Date, 'now').mockImplementation(() => currentTime);
|
||||||
|
|
||||||
|
try {
|
||||||
|
mockStripe.checkout.sessions.retrieve.mockResolvedValue({
|
||||||
|
id: "cs_old",
|
||||||
|
customer: "cus_old",
|
||||||
|
customer_details: { email: "old@test.com" },
|
||||||
|
});
|
||||||
|
|
||||||
|
// Add an entry at time T
|
||||||
|
const res1 = await request(app).get("/v1/billing/success?session_id=cs_old");
|
||||||
|
expect(res1.status).toBe(200);
|
||||||
|
|
||||||
|
// Advance time by 25 hours (more than 24h TTL)
|
||||||
|
currentTime += 25 * 60 * 60 * 1000;
|
||||||
|
|
||||||
|
// The old entry should be cleaned up and session should work again
|
||||||
|
const { findKeyByCustomerId } = await import("../services/keys.js");
|
||||||
|
vi.mocked(findKeyByCustomerId).mockResolvedValueOnce(null); // No existing key in DB
|
||||||
|
|
||||||
|
const res2 = await request(app).get("/v1/billing/success?session_id=cs_old");
|
||||||
|
expect(res2.status).toBe(200); // Should provision again, not 409
|
||||||
|
expect(res2.text).toContain("Welcome to Pro");
|
||||||
|
|
||||||
|
} finally {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
Date.now = originalDateNow;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should preserve fresh entries during cleanup", async () => {
|
||||||
|
// This test verifies that cleanup doesn't remove fresh entries
|
||||||
|
const originalDateNow = Date.now;
|
||||||
|
let currentTime = 1640995200000;
|
||||||
|
vi.spyOn(Date, 'now').mockImplementation(() => currentTime);
|
||||||
|
|
||||||
|
try {
|
||||||
|
// Add an old entry
|
||||||
|
mockStripe.checkout.sessions.retrieve.mockResolvedValue({
|
||||||
|
id: "cs_stale",
|
||||||
|
customer: "cus_stale",
|
||||||
|
customer_details: { email: "stale@test.com" },
|
||||||
|
});
|
||||||
|
await request(app).get("/v1/billing/success?session_id=cs_stale");
|
||||||
|
|
||||||
|
// Advance time by 1 hour
|
||||||
|
currentTime += 60 * 60 * 1000;
|
||||||
|
|
||||||
|
// Add a fresh entry
|
||||||
|
mockStripe.checkout.sessions.retrieve.mockResolvedValue({
|
||||||
|
id: "cs_recent",
|
||||||
|
customer: "cus_recent",
|
||||||
|
customer_details: { email: "recent@test.com" },
|
||||||
|
});
|
||||||
|
await request(app).get("/v1/billing/success?session_id=cs_recent");
|
||||||
|
|
||||||
|
// Advance time by 24 more hours (stale entry is now 25h old, recent is 24h old)
|
||||||
|
currentTime += 24 * 60 * 60 * 1000;
|
||||||
|
|
||||||
|
// Recent entry should still be treated as duplicate (preserved), stale should be cleaned
|
||||||
|
const res = await request(app).get("/v1/billing/success?session_id=cs_recent");
|
||||||
|
expect(res.status).toBe(409); // Still duplicate - not cleaned up
|
||||||
|
expect(res.body.error).toContain("already been used");
|
||||||
|
|
||||||
|
} finally {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
Date.now = originalDateNow;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should have bounded size even with many entries", async () => {
|
||||||
|
// This test verifies that the Set/Map doesn't grow unbounded
|
||||||
|
// We'll check that it doesn't exceed a reasonable size
|
||||||
|
const originalDateNow = Date.now;
|
||||||
|
let currentTime = 1640995200000;
|
||||||
|
vi.spyOn(Date, 'now').mockImplementation(() => currentTime);
|
||||||
|
|
||||||
|
try {
|
||||||
|
// Create many entries over time
|
||||||
|
for (let i = 0; i < 50; i++) {
|
||||||
|
mockStripe.checkout.sessions.retrieve.mockResolvedValue({
|
||||||
|
id: `cs_bulk_${i}`,
|
||||||
|
customer: `cus_bulk_${i}`,
|
||||||
|
customer_details: { email: `bulk${i}@test.com` },
|
||||||
|
});
|
||||||
|
|
||||||
|
await request(app).get(`/v1/billing/success?session_id=cs_bulk_${i}`);
|
||||||
|
|
||||||
|
// Advance time by 1 hour each iteration
|
||||||
|
currentTime += 60 * 60 * 1000;
|
||||||
|
}
|
||||||
|
|
||||||
|
// After processing 50 entries over 50 hours, old ones should be cleaned up
|
||||||
|
// The first ~25 entries should be expired (older than 24h)
|
||||||
|
|
||||||
|
// Try to use a very old session - should work again (cleaned up)
|
||||||
|
const { findKeyByCustomerId } = await import("../services/keys.js");
|
||||||
|
vi.mocked(findKeyByCustomerId).mockResolvedValueOnce(null);
|
||||||
|
|
||||||
|
const res = await request(app).get("/v1/billing/success?session_id=cs_bulk_0");
|
||||||
|
expect(res.status).toBe(200); // Should provision again, indicating it was cleaned up
|
||||||
|
|
||||||
|
} finally {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
Date.now = originalDateNow;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
|
||||||
|
|
@ -18,8 +18,35 @@ function getStripe(): Stripe {
|
||||||
|
|
||||||
const router = Router();
|
const router = Router();
|
||||||
|
|
||||||
// Track provisioned session IDs to prevent duplicate key creation
|
// Track provisioned session IDs with TTL to prevent duplicate key creation and memory leaks
|
||||||
const provisionedSessions = new Set<string>();
|
// Map<sessionId, timestamp> - entries older than 24h are periodically cleaned up
|
||||||
|
const provisionedSessions = new Map<string, number>();
|
||||||
|
|
||||||
|
// TTL Configuration
|
||||||
|
const SESSION_TTL_MS = 24 * 60 * 60 * 1000; // 24 hours
|
||||||
|
const CLEANUP_INTERVAL_MS = 60 * 60 * 1000; // Clean up every 1 hour
|
||||||
|
|
||||||
|
// Cleanup old provisioned session entries
|
||||||
|
function cleanupOldSessions() {
|
||||||
|
const now = Date.now();
|
||||||
|
const cutoff = now - SESSION_TTL_MS;
|
||||||
|
|
||||||
|
let cleanedCount = 0;
|
||||||
|
for (const [sessionId, timestamp] of provisionedSessions.entries()) {
|
||||||
|
if (timestamp < cutoff) {
|
||||||
|
provisionedSessions.delete(sessionId);
|
||||||
|
cleanedCount++;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (cleanedCount > 0) {
|
||||||
|
logger.info({ cleanedCount, remainingCount: provisionedSessions.size },
|
||||||
|
"Cleaned up expired provisioned sessions");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Start periodic cleanup
|
||||||
|
setInterval(cleanupOldSessions, CLEANUP_INTERVAL_MS);
|
||||||
|
|
||||||
const DOCFAST_PRODUCT_ID = "prod_TygeG8tQPtEAdE";
|
const DOCFAST_PRODUCT_ID = "prod_TygeG8tQPtEAdE";
|
||||||
|
|
||||||
|
|
@ -150,6 +177,9 @@ router.get("/success", async (req: Request, res: Response) => {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Clean up old sessions before checking duplicates
|
||||||
|
cleanupOldSessions();
|
||||||
|
|
||||||
// Prevent duplicate provisioning from same session
|
// Prevent duplicate provisioning from same session
|
||||||
if (provisionedSessions.has(sessionId)) {
|
if (provisionedSessions.has(sessionId)) {
|
||||||
res.status(409).json({ error: "This checkout session has already been used to provision a key. If you lost your key, use the key recovery feature." });
|
res.status(409).json({ error: "This checkout session has already been used to provision a key. If you lost your key, use the key recovery feature." });
|
||||||
|
|
@ -166,10 +196,10 @@ router.get("/success", async (req: Request, res: Response) => {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check DB for existing key (survives pod restarts, unlike provisionedSessions Set)
|
// Check DB for existing key (survives pod restarts, unlike provisionedSessions Map)
|
||||||
const existingKey = await findKeyByCustomerId(customerId);
|
const existingKey = await findKeyByCustomerId(customerId);
|
||||||
if (existingKey) {
|
if (existingKey) {
|
||||||
provisionedSessions.add(session.id);
|
provisionedSessions.set(session.id, Date.now());
|
||||||
res.send(`<!DOCTYPE html>
|
res.send(`<!DOCTYPE html>
|
||||||
<html><head><title>DocFast Pro — Key Already Provisioned</title>
|
<html><head><title>DocFast Pro — Key Already Provisioned</title>
|
||||||
<style>
|
<style>
|
||||||
|
|
@ -189,7 +219,7 @@ a { color: #4f9; }
|
||||||
}
|
}
|
||||||
|
|
||||||
const keyInfo = await createProKey(email, customerId);
|
const keyInfo = await createProKey(email, customerId);
|
||||||
provisionedSessions.add(session.id);
|
provisionedSessions.set(session.id, Date.now());
|
||||||
|
|
||||||
// Return a nice HTML page instead of raw JSON
|
// Return a nice HTML page instead of raw JSON
|
||||||
res.send(`<!DOCTYPE html>
|
res.send(`<!DOCTYPE html>
|
||||||
|
|
@ -316,7 +346,7 @@ router.post("/webhook", async (req: Request, res: Response) => {
|
||||||
}
|
}
|
||||||
|
|
||||||
const keyInfo = await createProKey(email, customerId);
|
const keyInfo = await createProKey(email, customerId);
|
||||||
provisionedSessions.add(session.id);
|
provisionedSessions.set(session.id, Date.now());
|
||||||
logger.info({ email, customerId }, "checkout.session.completed: provisioned pro key");
|
logger.info({ email, customerId }, "checkout.session.completed: provisioned pro key");
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue