Close the gaps left after v0.9.2's outage handling: route tests, stale cache, loading states (#397)
* docs: brainstorm resilience/adoption features and plan follow-up work
Brainstorm and deep-dive on where Immich Folio breaks under Immich
outages and what blocks self-hosted adoption, plus an implementation
plan for the remaining gaps after v0.9.2 (route-handler tests,
stale-while-error caching, loading skeletons).
* test: enable route-handler tests and cover admin guards and image content-type
vitest.config.ts only ran lib/__tests__/**, so route handlers were
structurally untestable. Two mechanisms live only at the route level and
cannot be extracted the way lib/imageSize.ts was: the admin auth guards
and the image proxy's Content-Type sanitisation (the stored-XSS fix from
c2fa8e7). Both were correct but unverified by any test.
Adds a table-driven test over all ten guarded /api/admin/* handlers,
including a filesystem walk that fails if a future route is added
without a row in the table. Adds a route-level test for
/api/image/[id] covering token validation, the six Content-Type
rewrite cases, ETag/304, and the 404 vs 503 (ImmichUnavailableError)
distinction.
Verified both catch real regressions by temporarily removing the admin
guards and the Content-Type rewrite and confirming the expected tests
fail, then restoring and confirming green again.
134 new tests (244 total, up from 210), tsc 0 errors, build succeeds.
* feat: serve stale cache entries while Immich is unavailable
v0.9.2 (#394) correctly split "gone" (404/410) from "unavailable"
(ImmichUnavailableError), and pages now render a 5xx instead of a false
404 during an outage — but that 5xx still replaced a gallery that had
loaded fine seconds earlier. lib/cache.ts deleted expired entries on
access, so nothing survived long enough to fall back to.
Cache entries now carry two deadlines: staleAt (the existing CACHE_TTL,
after which get() still reports a miss) and hardExpiresAt (the new
STALE_MAX_AGE, default 24h, after which the entry is dropped entirely).
Between the two, getStale() can still return the data.
getAlbums(), getAlbum() and getAssetInfo() catch ImmichUnavailableError
and fall back to getStale() before propagating. The two MISSING-sentinel
writes (definitive 404s) are deliberately left on the short cacheTtl
window — a corrected asset ID should take effect after the normal TTL,
not sit in the 24h stale window meant for outages.
STALE_MAX_AGE=0 disables the fallback. Verified the fallback only
engages on a real outage, not on a cache hit at the TTL boundary, and
that a definitive 404 still reports missing rather than falling back.
256 tests (up from 244), tsc 0 errors, build succeeds.
* feat: add loading skeletons for dynamic gallery routes
Every gallery route is force-dynamic and waits on a live Immich
round-trip — up to IMMICH_TIMEOUT_MS when the server is struggling —
with nothing rendered until it resolves. loading.tsx renders instantly
while the page component awaits its data.
Not browser-verified against live data: this worktree has no
.env.local and only .example content files, so the app boots into
setup mode (confirmed the dev server starts cleanly and the setup
screen renders, HTTP 200). tsc 0 errors, build succeeds, no new lint
errors.
* docs: document stale cache window, image token revocation, route-test convention
- README: STALE_MAX_AGE env var and a "Behaviour when Immich is
unreachable" section (stale-serves-until-STALE_MAX_AGE, 503 not 404,
outages never cached).
- docs/gallery-config.md: new "Image Token Revocation" subsection under
System & Security — un-publishing an album does not revoke already
issued image URLs; rotating AUTH_SECRET does.
- CLAUDE.md: updated cache description to cover the stale window and
MISSING sentinel, and a new code-convention note that route-level
logic (guards, header sanitisation) should get a route-level test
rather than being extracted into lib/ just to reach the test runner.
---------
Co-authored-by: ralksta <ralksta@users.noreply.github.com>
This commit is contained in:
66
lib/__tests__/cache-stale.test.ts
Normal file
66
lib/__tests__/cache-stale.test.ts
Normal file
@@ -0,0 +1,66 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { cache } from '../cache';
|
||||
|
||||
describe('MemoryCache stale window', () => {
|
||||
beforeEach(() => {
|
||||
cache.clear();
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('returns fresh entries from get()', () => {
|
||||
cache.set('k', { v: 1 }, 1000);
|
||||
expect(cache.get<{ v: number }>('k')).toEqual({ v: 1 });
|
||||
});
|
||||
|
||||
it('returns null from get() once the ttl elapsed', () => {
|
||||
cache.set('k', { v: 1 }, 1000);
|
||||
vi.advanceTimersByTime(1001);
|
||||
expect(cache.get('k')).toBeNull();
|
||||
});
|
||||
|
||||
it('still returns the value from getStale() after the ttl elapsed', () => {
|
||||
cache.set('k', { v: 1 }, 1000, 60_000);
|
||||
vi.advanceTimersByTime(1001);
|
||||
expect(cache.getStale<{ v: number }>('k')).toEqual({ v: 1 });
|
||||
});
|
||||
|
||||
it('drops the entry from getStale() past the hard max age', () => {
|
||||
cache.set('k', { v: 1 }, 1000, 5000);
|
||||
vi.advanceTimersByTime(5001);
|
||||
expect(cache.getStale('k')).toBeNull();
|
||||
});
|
||||
|
||||
it('does not let get() destroy the stale fallback', () => {
|
||||
// get() must not delete an expired entry — otherwise the fallback is gone
|
||||
// the moment any caller asks for fresh data first.
|
||||
cache.set('k', { v: 1 }, 1000, 60_000);
|
||||
vi.advanceTimersByTime(1001);
|
||||
expect(cache.get('k')).toBeNull();
|
||||
expect(cache.getStale<{ v: number }>('k')).toEqual({ v: 1 });
|
||||
});
|
||||
|
||||
it('never expires stale before the ttl itself', () => {
|
||||
// A misconfigured STALE_MAX_AGE below CACHE_TTL must not shorten the fresh
|
||||
// window.
|
||||
cache.set('k', { v: 1 }, 10_000, 1000);
|
||||
vi.advanceTimersByTime(9000);
|
||||
expect(cache.get<{ v: number }>('k')).toEqual({ v: 1 });
|
||||
});
|
||||
|
||||
it('evicts the oldest entry when capacity is exceeded', () => {
|
||||
for (let i = 0; i < 201; i++) cache.set(`k${i}`, i, 60_000);
|
||||
expect(cache.get('k0')).toBeNull();
|
||||
expect(cache.get('k200')).toBe(200);
|
||||
});
|
||||
|
||||
it('delete() removes the entry from get() and getStale()', () => {
|
||||
cache.set('k', { v: 1 }, 1000, 60_000);
|
||||
cache.delete('k');
|
||||
expect(cache.get('k')).toBeNull();
|
||||
expect(cache.getStale('k')).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -20,6 +20,7 @@ vi.mock('../config', async () => {
|
||||
// written, which would make any caching assertion flaky. Isolation comes
|
||||
// from the cache.clear() in beforeEach, not from an unusable TTL.
|
||||
cacheTtl: 60_000,
|
||||
staleMaxAge: 86_400_000,
|
||||
immichTimeoutMs: 15000,
|
||||
}),
|
||||
};
|
||||
@@ -496,3 +497,68 @@ describe('ImmichClient', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('stale fallback when Immich is unavailable', () => {
|
||||
const mockFetch = global.fetch as any;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
cache.clear();
|
||||
});
|
||||
|
||||
it('serves the previously cached album list instead of throwing', async () => {
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
headers: { get: () => 'application/json' },
|
||||
json: async () => [
|
||||
{
|
||||
id: 'album-1',
|
||||
albumName: 'Original',
|
||||
description: '',
|
||||
albumThumbnailAssetId: null,
|
||||
assetCount: 1,
|
||||
assets: [],
|
||||
createdAt: '',
|
||||
updatedAt: '',
|
||||
order: 'desc',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const fresh = await immich.getAlbums();
|
||||
expect(fresh).toHaveLength(1);
|
||||
|
||||
vi.useFakeTimers();
|
||||
// Strictly past cacheTtl (60_000ms) so cache.get()'s fresh-only check
|
||||
// misses and the fallback path — not a lucky cache hit — is what's tested.
|
||||
vi.advanceTimersByTime(60_001);
|
||||
mockFetch.mockRejectedValue(new Error('ECONNREFUSED'));
|
||||
|
||||
const stale = await immich.getAlbums();
|
||||
vi.useRealTimers();
|
||||
|
||||
expect(stale).toHaveLength(1);
|
||||
expect(stale[0].id).toBe('album-1');
|
||||
});
|
||||
|
||||
it('still throws when there is nothing cached to fall back to', async () => {
|
||||
mockFetch.mockRejectedValue(new Error('ECONNREFUSED'));
|
||||
await expect(immich.getAlbums()).rejects.toThrow(ImmichUnavailableError);
|
||||
});
|
||||
|
||||
it('does not swallow a definitive 404 as an outage', async () => {
|
||||
// A missing album must keep reporting missing, not fall back to stale data.
|
||||
mockFetch.mockResolvedValue({
|
||||
ok: false,
|
||||
status: 404,
|
||||
statusText: 'Not Found',
|
||||
headers: { get: () => 'application/json' },
|
||||
});
|
||||
await expect(immich.getAlbum('album-1')).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it('still refuses albums outside the allowlist', async () => {
|
||||
mockFetch.mockRejectedValue(new Error('ECONNREFUSED'));
|
||||
await expect(immich.getAlbum('not-allowed')).resolves.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
56
lib/cache.ts
56
lib/cache.ts
@@ -1,13 +1,22 @@
|
||||
/**
|
||||
* Simple in-memory LRU cache for Immich API responses.
|
||||
* Avoids hammering the Immich server with identical requests.
|
||||
*
|
||||
* Entries carry two deadlines. `staleAt` is the normal TTL — past it, get()
|
||||
* reports a miss and callers refetch. `hardExpiresAt` is the point past which
|
||||
* the data is too old to show at all. Between the two, the entry survives as a
|
||||
* fallback that getStale() hands out when Immich is unreachable, so a
|
||||
* restarting server does not turn the gallery into an error page.
|
||||
*/
|
||||
|
||||
interface CacheEntry<T> {
|
||||
data: T;
|
||||
expiresAt: number;
|
||||
staleAt: number;
|
||||
hardExpiresAt: number;
|
||||
}
|
||||
|
||||
const DEFAULT_STALE_MAX_AGE_MS = 24 * 60 * 60 * 1000; // 24 hours
|
||||
|
||||
class MemoryCache {
|
||||
private store = new Map<string, CacheEntry<unknown>>();
|
||||
private maxEntries = 200;
|
||||
@@ -16,35 +25,63 @@ class MemoryCache {
|
||||
return this.store.size;
|
||||
}
|
||||
|
||||
/** Move an entry to the newest end of the LRU order. */
|
||||
private touch(key: string, entry: CacheEntry<unknown>): void {
|
||||
this.store.delete(key);
|
||||
this.store.set(key, entry);
|
||||
}
|
||||
|
||||
/** Fresh data only. Returns null once the TTL has elapsed. */
|
||||
get<T>(key: string): T | null {
|
||||
const entry = this.store.get(key);
|
||||
if (!entry) return null;
|
||||
|
||||
if (Date.now() > entry.expiresAt) {
|
||||
const now = Date.now();
|
||||
|
||||
if (now > entry.hardExpiresAt) {
|
||||
this.store.delete(key);
|
||||
return null;
|
||||
}
|
||||
|
||||
// LRU: Move accessed item to the back of the Map (newest)
|
||||
this.store.delete(key);
|
||||
this.store.set(key, entry);
|
||||
// Past the TTL the entry is no longer fresh, but it is deliberately kept
|
||||
// so getStale() can still use it.
|
||||
if (now > entry.staleAt) return null;
|
||||
|
||||
this.touch(key, entry);
|
||||
return entry.data as T;
|
||||
}
|
||||
|
||||
set<T>(key: string, data: T, ttlMs: number): void {
|
||||
// If key exists, delete it first to ensure we push it to the back
|
||||
/**
|
||||
* Fresh *or* stale data. Only for the upstream-unreachable path — prefer
|
||||
* get() everywhere else.
|
||||
*/
|
||||
getStale<T>(key: string): T | null {
|
||||
const entry = this.store.get(key);
|
||||
if (!entry) return null;
|
||||
|
||||
if (Date.now() > entry.hardExpiresAt) {
|
||||
this.store.delete(key);
|
||||
return null;
|
||||
}
|
||||
|
||||
this.touch(key, entry);
|
||||
return entry.data as T;
|
||||
}
|
||||
|
||||
set<T>(key: string, data: T, ttlMs: number, staleMaxAgeMs = DEFAULT_STALE_MAX_AGE_MS): void {
|
||||
if (this.store.has(key)) {
|
||||
this.store.delete(key);
|
||||
} else if (this.store.size >= this.maxEntries) {
|
||||
// Evict oldest entry (first item) if at capacity
|
||||
const firstKey = this.store.keys().next().value;
|
||||
if (firstKey) this.store.delete(firstKey);
|
||||
}
|
||||
|
||||
const now = Date.now();
|
||||
this.store.set(key, {
|
||||
data,
|
||||
expiresAt: Date.now() + ttlMs,
|
||||
staleAt: now + ttlMs,
|
||||
// Measured from write time, and never shorter than the TTL itself.
|
||||
hardExpiresAt: now + Math.max(ttlMs, staleMaxAgeMs),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -58,3 +95,4 @@ class MemoryCache {
|
||||
}
|
||||
|
||||
export const cache = new MemoryCache();
|
||||
export { DEFAULT_STALE_MAX_AGE_MS };
|
||||
|
||||
@@ -276,6 +276,7 @@ export function getConfig(): AppConfig {
|
||||
albumPasswords: {},
|
||||
albumHeroImages: {},
|
||||
cacheTtl: env.CACHE_TTL * 1000,
|
||||
staleMaxAge: env.STALE_MAX_AGE * 1000,
|
||||
immichTimeoutMs: env.IMMICH_TIMEOUT_MS,
|
||||
rateLimitRpm: env.RATE_LIMIT_RPM,
|
||||
trustedProxyHops: env.TRUSTED_PROXY_HOPS,
|
||||
@@ -356,6 +357,7 @@ export function getConfig(): AppConfig {
|
||||
albumPasswords,
|
||||
albumHeroImages,
|
||||
cacheTtl: env.CACHE_TTL * 1000,
|
||||
staleMaxAge: env.STALE_MAX_AGE * 1000,
|
||||
immichTimeoutMs: env.IMMICH_TIMEOUT_MS,
|
||||
rateLimitRpm: env.RATE_LIMIT_RPM,
|
||||
trustedProxyHops: env.TRUSTED_PROXY_HOPS,
|
||||
|
||||
@@ -99,6 +99,8 @@ export interface AppConfig {
|
||||
albumPasswords: Record<string, string>;
|
||||
albumHeroImages: Record<string, string>;
|
||||
cacheTtl: number;
|
||||
/** How long (ms) an expired cache entry may still be served while Immich is unreachable. */
|
||||
staleMaxAge: number;
|
||||
/** Response-wait budget for Immich API calls. Does not cap image/video body transfer. */
|
||||
immichTimeoutMs: number;
|
||||
rateLimitRpm: number;
|
||||
|
||||
@@ -9,6 +9,7 @@ export interface Env {
|
||||
SITE_TITLE: string;
|
||||
SITE_SUBTITLE: string;
|
||||
CACHE_TTL: number;
|
||||
STALE_MAX_AGE: number;
|
||||
IMAGE_CACHE_VERSION: string;
|
||||
IMMICH_TIMEOUT_MS: number;
|
||||
RATE_LIMIT_RPM: number;
|
||||
@@ -37,6 +38,13 @@ function parseEnv(): Env {
|
||||
const cacheTtl =
|
||||
cacheTtlStr && !isNaN(parseInt(cacheTtlStr, 10)) ? parseInt(cacheTtlStr, 10) : 300;
|
||||
|
||||
// How long an expired cache entry may still be served while Immich is
|
||||
// unreachable. Keeps the public gallery online across an Immich restart
|
||||
// instead of showing an error page. 0 disables the fallback.
|
||||
const staleMaxAgeStr = process.env.STALE_MAX_AGE;
|
||||
const staleMaxAge =
|
||||
staleMaxAgeStr && !isNaN(parseInt(staleMaxAgeStr, 10)) ? parseInt(staleMaxAgeStr, 10) : 86400; // 24 hours
|
||||
|
||||
// How long to wait on Immich before giving up. Without an explicit budget the
|
||||
// only ceiling is undici's built-in default (minutes), and since every page is
|
||||
// force-dynamic an Immich that accepts connections but never answers would
|
||||
@@ -91,6 +99,7 @@ function parseEnv(): Env {
|
||||
SITE_TITLE: process.env.SITE_TITLE || 'Gallery',
|
||||
SITE_SUBTITLE: process.env.SITE_SUBTITLE || '',
|
||||
CACHE_TTL: Math.max(0, cacheTtl),
|
||||
STALE_MAX_AGE: Math.max(0, staleMaxAge),
|
||||
// Kept URL-safe: this value is appended to every image and video URL.
|
||||
IMAGE_CACHE_VERSION: encodeURIComponent(imageCacheVersion),
|
||||
// A sub-second budget would abort healthy requests; 0 must not mean "no timeout".
|
||||
|
||||
@@ -127,6 +127,28 @@ class ImmichClient {
|
||||
return getConfig();
|
||||
}
|
||||
|
||||
/** Cache write that carries the configured stale window. */
|
||||
private cacheSet<T>(key: string, data: T): void {
|
||||
cache.set(key, data, this.config.cacheTtl, this.config.staleMaxAge);
|
||||
}
|
||||
|
||||
/**
|
||||
* Last resort when Immich is unavailable: hand back the most recent known
|
||||
* good answer rather than failing the page. Only successes ever reach the
|
||||
* cache, so this cannot resurrect an outage — and past staleMaxAge the entry
|
||||
* is gone and the error propagates as before.
|
||||
*/
|
||||
private staleOrThrow<T>(cacheKey: string, error: unknown, label: string): T {
|
||||
if (error instanceof ImmichUnavailableError) {
|
||||
const stale = cache.getStale<T>(cacheKey);
|
||||
if (stale !== null) {
|
||||
console.warn(`[Immich] ⚠️ Upstream unavailable — serving stale ${label}`);
|
||||
return stale;
|
||||
}
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
private async request<T>(endpoint: string, body?: unknown): Promise<T | null> {
|
||||
if (this.config.needsSetup) return null;
|
||||
|
||||
@@ -402,8 +424,10 @@ class ImmichClient {
|
||||
console.log('─'.repeat(80) + '\n');
|
||||
}
|
||||
|
||||
cache.set(cacheKey, filtered, this.config.cacheTtl);
|
||||
this.cacheSet(cacheKey, filtered);
|
||||
return filtered;
|
||||
} catch (error) {
|
||||
return this.staleOrThrow<ImmichAlbum[]>(cacheKey, error, 'album list');
|
||||
} finally {
|
||||
this.pendingAlbumsPromise = null;
|
||||
}
|
||||
@@ -565,8 +589,10 @@ class ImmichClient {
|
||||
album.description = description;
|
||||
album.slug = slugify(name);
|
||||
|
||||
cache.set(cacheKey, album, this.config.cacheTtl);
|
||||
this.cacheSet(cacheKey, album);
|
||||
return album;
|
||||
} catch (error) {
|
||||
return this.staleOrThrow<ImmichAlbum>(cacheKey, error, `album ${albumId}`);
|
||||
} finally {
|
||||
this.pendingAlbumPromises.delete(albumId);
|
||||
}
|
||||
@@ -629,8 +655,10 @@ class ImmichClient {
|
||||
return null;
|
||||
}
|
||||
|
||||
cache.set(cacheKey, asset, this.config.cacheTtl);
|
||||
this.cacheSet(cacheKey, asset);
|
||||
return asset;
|
||||
} catch (error) {
|
||||
return this.staleOrThrow<ImmichAsset>(cacheKey, error, `asset ${assetId}`);
|
||||
} finally {
|
||||
this.pendingAssetPromises.delete(assetId);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user