fix(favicon): short TTL for negative favicon cache entries
A failed favicon lookup was cached for a full week (same TTL as a success), so a transient miss (e.g. the icon not yet indexed upstream) blacklisted the domain for days. Cache negatives for 6 hours instead so the next email retries. Co-Authored-By: Claude Opus 4.7 <[email protected]>
This commit is contained in:
1 parent
bb9fce72ff
commit
5f13126b35
4 files changed
+60
-3
No files matched your search
@@ -1,4 +1,4 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { http, HttpResponse } from "msw";
|
||||
import { server, createMockEnv } from "../test/setup";
|
||||
import {
|
||||
@@ -6,7 +6,12 @@ import {
|
||||
extractEmailDomain,
|
||||
getCachedIcon,
|
||||
} from "./favicon-fetcher";
|
||||
import { MAX_ICON_BYTES } from "../config/constants";
|
||||
import { IconRepository } from "./icon-repository";
|
||||
import {
|
||||
ICON_NEGATIVE_TTL_SECONDS,
|
||||
ICON_TTL_SECONDS,
|
||||
MAX_ICON_BYTES,
|
||||
} from "../config/constants";
|
||||
|
||||
const iconKey = (domain: string) => `icon:${domain}`;
|
||||
import type { Env } from "../types";
|
||||
@@ -89,6 +94,45 @@ describe("cacheFaviconForDomain", () => {
|
||||
expect(await getCachedIcon("nope.test", env)).toBeNull();
|
||||
});
|
||||
|
||||
it("gives a negative entry a short TTL so transient misses self-heal", async () => {
|
||||
const env = createMockEnv() as unknown as Env;
|
||||
const put = vi.spyOn(IconRepository.prototype, "put");
|
||||
server.use(
|
||||
http.get("https://transient.test/favicon.ico", () =>
|
||||
HttpResponse.text("", { status: 404 }),
|
||||
),
|
||||
http.get("https://icons.duckduckgo.com/ip3/transient.test.ico", () =>
|
||||
HttpResponse.text("", { status: 404 }),
|
||||
),
|
||||
);
|
||||
|
||||
await cacheFaviconForDomain("transient.test", env);
|
||||
|
||||
expect(put).toHaveBeenCalledWith(
|
||||
"transient.test",
|
||||
expect.any(String),
|
||||
ICON_NEGATIVE_TTL_SECONDS,
|
||||
);
|
||||
put.mockRestore();
|
||||
});
|
||||
|
||||
it("gives a positive entry the full TTL", async () => {
|
||||
const env = createMockEnv() as unknown as Env;
|
||||
const put = vi.spyOn(IconRepository.prototype, "put");
|
||||
server.use(
|
||||
http.get("https://hit.test/favicon.ico", () => imageResponse(PNG)),
|
||||
);
|
||||
|
||||
await cacheFaviconForDomain("hit.test", env);
|
||||
|
||||
expect(put).toHaveBeenCalledWith(
|
||||
"hit.test",
|
||||
expect.any(String),
|
||||
ICON_TTL_SECONDS,
|
||||
);
|
||||
put.mockRestore();
|
||||
});
|
||||
|
||||
it("rejects oversized responses as negative", async () => {
|
||||
const env = createMockEnv() as unknown as Env;
|
||||
const big = new Uint8Array(MAX_ICON_BYTES + 1);
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { Env } from "../types";
|
||||
import {
|
||||
ICON_FETCH_TIMEOUT_MS,
|
||||
ICON_NEGATIVE_TTL_SECONDS,
|
||||
ICON_TTL_SECONDS,
|
||||
MAX_ICON_BYTES,
|
||||
} from "../config/constants";
|
||||
@@ -102,7 +103,8 @@ export async function cacheFaviconForDomain(
|
||||
}
|
||||
: { data: null, contentType: "" };
|
||||
|
||||
await repo.put(domain, JSON.stringify(record), ICON_TTL_SECONDS);
|
||||
const ttl = icon ? ICON_TTL_SECONDS : ICON_NEGATIVE_TTL_SECONDS;
|
||||
await repo.put(domain, JSON.stringify(record), ttl);
|
||||
} catch (error) {
|
||||
logger.warn("Favicon cache failed", { domain, error: String(error) });
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user