From 253eef405b192af818d4c5b785b937fb10221cab Mon Sep 17 00:00:00 2001 From: Moreno Feltscher Date: Mon, 16 Feb 2026 13:16:32 +0100 Subject: [PATCH 1/6] feat: make error handling of cache tags providers configurable --- src/cache-tags/provider/base.ts | 41 +++++++++++ src/cache-tags/provider/neon.ts | 106 +++++++++++++++++++---------- src/cache-tags/provider/redis.ts | 112 ++++++++++++++++++++----------- src/cache-tags/types.ts | 13 ++++ 4 files changed, 196 insertions(+), 76 deletions(-) create mode 100644 src/cache-tags/provider/base.ts diff --git a/src/cache-tags/provider/base.ts b/src/cache-tags/provider/base.ts new file mode 100644 index 0000000..65317d8 --- /dev/null +++ b/src/cache-tags/provider/base.ts @@ -0,0 +1,41 @@ +import type { CacheTag, CacheTagsProvider, CacheTagsProviderErrorHandlingConfig } from '../types.js'; + +/** + * A wrapper for `CacheTagsProvider` implementations that adds error handling and logging. + */ +export abstract class AbstractErrorHandlingCacheTagsProvider implements CacheTagsProvider { + protected readonly throwOnError: boolean; + protected readonly onError?: CacheTagsProviderErrorHandlingConfig['onError']; + + protected constructor( + protected readonly providerName: string, + config: CacheTagsProviderErrorHandlingConfig = {}, + ) { + this.throwOnError = config.throwOnError ?? false; + this.onError = config.onError; + } + + public abstract storeQueryCacheTags(queryId: string, cacheTags: CacheTag[]): Promise; + + public abstract queriesReferencingCacheTags(cacheTags: CacheTag[]): Promise; + + public abstract deleteCacheTags(cacheTags: CacheTag[]): Promise; + + public abstract truncateCacheTags(): Promise; + + protected async wrap(method: keyof CacheTagsProvider, args: unknown[], fn: () => Promise, fallback: T): Promise { + try { + return await fn(); + } catch (error) { + const provider = this.providerName; + this.onError?.(error, { provider, method, args }); + + if (this.throwOnError) { + throw error; + } + console.debug(`Error occurred in ${provider}.${method} with args ${JSON.stringify(args)}.`, { error }); + + return fallback; + } + } +} diff --git a/src/cache-tags/provider/neon.ts b/src/cache-tags/provider/neon.ts index f02ca3e..fad6386 100644 --- a/src/cache-tags/provider/neon.ts +++ b/src/cache-tags/provider/neon.ts @@ -1,7 +1,8 @@ import { neon } from '@neondatabase/serverless'; -import { type CacheTag, type CacheTagsProvider } from '../types.js'; +import type { CacheTag, CacheTagsProvider, CacheTagsProviderErrorHandlingConfig } from '../types.js'; +import { AbstractErrorHandlingCacheTagsProvider } from './base.js'; -export type NeonCacheTagsProviderConfig = { +type NeonCacheTagsProviderBaseConfig = { /** * Neon connection string. You can find it in the "Connection" tab of your Neon project dashboard. * Has the format `postgresql://user:pass@host/db` @@ -21,61 +22,94 @@ export type NeonCacheTagsProviderConfig = { readonly table: string; }; +export type NeonCacheTagsProviderConfig = NeonCacheTagsProviderBaseConfig & CacheTagsProviderErrorHandlingConfig; + /** * A `CacheTagsProvider` implementation that uses Neon as the storage backend. */ -export class NeonCacheTagsProvider implements CacheTagsProvider { +export class NeonCacheTagsProvider extends AbstractErrorHandlingCacheTagsProvider implements CacheTagsProvider { private readonly sql; private readonly table; - constructor({ connectionUrl, table }: NeonCacheTagsProviderConfig) { + constructor({ connectionUrl, table, throwOnError, onError }: NeonCacheTagsProviderConfig) { + super('NeonCacheTagsProvider', { throwOnError, onError }); this.sql = neon(connectionUrl, { fullResults: true }); this.table = NeonCacheTagsProvider.quoteIdentifier(table); } public async storeQueryCacheTags(queryId: string, cacheTags: CacheTag[]) { - if (!cacheTags?.length) { - return; - } - - const tags = cacheTags.flatMap((_, i) => [queryId, cacheTags[i]]); - const placeholders = cacheTags.map((_, i) => `($${2 * i + 1}, $${2 * i + 2})`).join(','); - - await this.sql.query(`INSERT INTO ${this.table} VALUES ${placeholders} ON CONFLICT DO NOTHING`, tags); + return this.wrap( + 'storeQueryCacheTags', + [queryId, cacheTags], + async () => { + if (cacheTags.length === 0) { + return; + } + + const tags = cacheTags.flatMap((_, i) => [queryId, cacheTags[i]]); + const placeholders = cacheTags.map((_, i) => `($${2 * i + 1}, $${2 * i + 2})`).join(','); + + await this.sql.query(`INSERT INTO ${this.table} VALUES ${placeholders} ON CONFLICT DO NOTHING`, tags); + }, + undefined, + ); } public async queriesReferencingCacheTags(cacheTags: CacheTag[]): Promise { - if (!cacheTags?.length) { - return []; - } - - const placeholders = cacheTags.map((_, i) => `$${i + 1}`).join(','); - - const { rows } = await this.sql.query( - `SELECT DISTINCT query_id FROM ${this.table} WHERE cache_tag IN (${placeholders})`, - cacheTags, + return this.wrap( + 'queriesReferencingCacheTags', + [cacheTags], + async () => { + if (cacheTags.length === 0) { + return []; + } + + const placeholders = cacheTags.map((_, i) => `$${i + 1}`).join(','); + + const { rows } = await this.sql.query( + `SELECT DISTINCT query_id FROM ${this.table} WHERE cache_tag IN (${placeholders})`, + cacheTags, + ); + + return rows.reduce((queryIds, row) => { + if (typeof row.query_id === 'string') { + queryIds.push(row.query_id); + } + + return queryIds; + }, []); + }, + [], ); - - return rows.reduce((queryIds, row) => { - if (typeof row.query_id === 'string') { - queryIds.push(row.query_id); - } - - return queryIds; - }, []); } public async deleteCacheTags(cacheTags: CacheTag[]) { - if (!cacheTags?.length) { - return 0; - } - const placeholders = cacheTags.map((_, i) => `$${i + 1}`).join(','); - - return (await this.sql.query(`DELETE FROM ${this.table} WHERE cache_tag IN (${placeholders})`, cacheTags)).rowCount ?? 0; + return this.wrap( + 'deleteCacheTags', + [cacheTags], + async () => { + if (cacheTags.length === 0) { + return 0; + } + const placeholders = cacheTags.map((_, i) => `$${i + 1}`).join(','); + + return ( + (await this.sql.query(`DELETE FROM ${this.table} WHERE cache_tag IN (${placeholders})`, cacheTags)).rowCount ?? 0 + ); + }, + 0, + ); } public async truncateCacheTags() { - return (await this.sql.query(`DELETE FROM ${this.table}`)).rowCount ?? 0; + return this.wrap( + 'truncateCacheTags', + [], + async () => { + return (await this.sql.query(`DELETE FROM ${this.table}`)).rowCount ?? 0; + }, + 0, + ); } /** diff --git a/src/cache-tags/provider/redis.ts b/src/cache-tags/provider/redis.ts index a112401..d4353df 100644 --- a/src/cache-tags/provider/redis.ts +++ b/src/cache-tags/provider/redis.ts @@ -1,7 +1,8 @@ import { Redis } from 'ioredis'; -import { type CacheTag, type CacheTagsProvider } from '../types.js'; +import type { CacheTag, CacheTagsProvider, CacheTagsProviderErrorHandlingConfig } from '../types.js'; +import { AbstractErrorHandlingCacheTagsProvider } from './base.js'; -export type RedisCacheTagsProviderConfig = { +type RedisCacheTagsProviderBaseConfig = { /** * Redis connection string. For example, `redis://user:pass@host:port/db`. */ @@ -14,14 +15,17 @@ export type RedisCacheTagsProviderConfig = { readonly keyPrefix?: string; }; +export type RedisCacheTagsProviderConfig = RedisCacheTagsProviderBaseConfig & CacheTagsProviderErrorHandlingConfig; + /** * A `CacheTagsProvider` implementation that uses Redis as the storage backend. */ -export class RedisCacheTagsProvider implements CacheTagsProvider { +export class RedisCacheTagsProvider extends AbstractErrorHandlingCacheTagsProvider implements CacheTagsProvider { private readonly redis; private readonly keyPrefix; - constructor({ connectionUrl, keyPrefix }: RedisCacheTagsProviderConfig) { + constructor({ connectionUrl, keyPrefix, throwOnError, onError }: RedisCacheTagsProviderConfig) { + super('RedisCacheTagsProvider', { throwOnError, onError }); this.redis = new Redis(connectionUrl, { maxRetriesPerRequest: 3, lazyConnect: true, @@ -30,51 +34,79 @@ export class RedisCacheTagsProvider implements CacheTagsProvider { } public async storeQueryCacheTags(queryId: string, cacheTags: CacheTag[]) { - if (!cacheTags?.length) { - return; - } - - const pipeline = this.redis.pipeline(); - - for (const tag of cacheTags) { - pipeline.sadd(`${this.keyPrefix}${tag}`, queryId); - } - - const results = await pipeline.exec(); - const error = results?.find(([err]) => err)?.[0]; - if (error) { - throw error; - } + return this.wrap( + 'storeQueryCacheTags', + [queryId, cacheTags], + async () => { + if (cacheTags.length === 0) { + return; + } + + const pipeline = this.redis.pipeline(); + + for (const tag of cacheTags) { + pipeline.sadd(`${this.keyPrefix}${tag}`, queryId); + } + + const results = await pipeline.exec(); + const error = results?.find(([err]) => err)?.[0]; + if (error) { + throw error; + } + }, + undefined, + ); } public async queriesReferencingCacheTags(cacheTags: CacheTag[]) { - if (!cacheTags?.length) { - return []; - } - - const keys = cacheTags.map((tag) => `${this.keyPrefix}${tag}`); - - return this.redis.sunion(...keys); + return this.wrap( + 'queriesReferencingCacheTags', + [cacheTags], + async () => { + if (cacheTags.length === 0) { + return []; + } + + const keys = cacheTags.map((tag) => `${this.keyPrefix}${tag}`); + + return this.redis.sunion(...keys); + }, + [], + ); } public async deleteCacheTags(cacheTags: CacheTag[]) { - if (!cacheTags?.length) { - return 0; - } - - const keys = cacheTags.map((tag) => `${this.keyPrefix}${tag}`); - - return this.redis.del(...keys); + return this.wrap( + 'deleteCacheTags', + [cacheTags], + async () => { + if (cacheTags.length === 0) { + return 0; + } + + const keys = cacheTags.map((tag) => `${this.keyPrefix}${tag}`); + + return this.redis.del(...keys); + }, + 0, + ); } public async truncateCacheTags() { - const keys = await this.getKeys(); - - if (keys.length === 0) { - return 0; - } - - return await this.redis.del(...keys); + return this.wrap( + 'truncateCacheTags', + [], + async () => { + const keys = await this.getKeys(); + + if (keys.length === 0) { + return 0; + } + + return await this.redis.del(...keys); + }, + 0, + ); } /** diff --git a/src/cache-tags/types.ts b/src/cache-tags/types.ts index 8e74898..e32fca0 100644 --- a/src/cache-tags/types.ts +++ b/src/cache-tags/types.ts @@ -64,3 +64,16 @@ export interface CacheTagsProvider { */ truncateCacheTags(): Promise; } + +export type CacheTagsProviderErrorHandlingConfig = { + /** + * If false, errors are suppressed and a fallback value is returned. + * Default: true + */ + throwOnError?: boolean; + + /** + * Optional hook (logging/telemetry). + */ + onError?: (error: unknown, ctx: { provider: string; method: keyof CacheTagsProvider; args: unknown[] }) => void; +}; From 14affddfcf218a90e8824caa2797777b29048a60 Mon Sep 17 00:00:00 2001 From: Moreno Feltscher Date: Mon, 16 Feb 2026 13:18:38 +0100 Subject: [PATCH 2/6] fix: adjust default --- src/cache-tags/provider/base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache-tags/provider/base.ts b/src/cache-tags/provider/base.ts index 65317d8..9855639 100644 --- a/src/cache-tags/provider/base.ts +++ b/src/cache-tags/provider/base.ts @@ -11,7 +11,7 @@ export abstract class AbstractErrorHandlingCacheTagsProvider implements CacheTag protected readonly providerName: string, config: CacheTagsProviderErrorHandlingConfig = {}, ) { - this.throwOnError = config.throwOnError ?? false; + this.throwOnError = config.throwOnError ?? true; this.onError = config.onError; } From 9492d84555eb0955879680f517d114bef372fbf0 Mon Sep 17 00:00:00 2001 From: Moreno Feltscher Date: Mon, 16 Feb 2026 13:24:53 +0100 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/cache-tags/provider/base.ts | 2 +- src/cache-tags/types.ts | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cache-tags/provider/base.ts b/src/cache-tags/provider/base.ts index 9855639..fa89b2e 100644 --- a/src/cache-tags/provider/base.ts +++ b/src/cache-tags/provider/base.ts @@ -1,7 +1,7 @@ import type { CacheTag, CacheTagsProvider, CacheTagsProviderErrorHandlingConfig } from '../types.js'; /** - * A wrapper for `CacheTagsProvider` implementations that adds error handling and logging. + * An abstract base class for `CacheTagsProvider` implementations that adds error handling and logging. */ export abstract class AbstractErrorHandlingCacheTagsProvider implements CacheTagsProvider { protected readonly throwOnError: boolean; diff --git a/src/cache-tags/types.ts b/src/cache-tags/types.ts index e32fca0..b874c7b 100644 --- a/src/cache-tags/types.ts +++ b/src/cache-tags/types.ts @@ -73,7 +73,11 @@ export type CacheTagsProviderErrorHandlingConfig = { throwOnError?: boolean; /** - * Optional hook (logging/telemetry). + * Optional callback invoked when an error occurs in a `CacheTagsProvider` method, + * useful for logging and telemetry. + * + * Called before the error is either thrown (when `throwOnError` is true or + * undefined) or suppressed (when `throwOnError` is false). */ onError?: (error: unknown, ctx: { provider: string; method: keyof CacheTagsProvider; args: unknown[] }) => void; }; From 816af1618431606e8bda52026b16fb14957dd542 Mon Sep 17 00:00:00 2001 From: Moreno Feltscher Date: Mon, 16 Feb 2026 13:25:03 +0100 Subject: [PATCH 4/6] erroooooor --- src/cache-tags/provider/base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache-tags/provider/base.ts b/src/cache-tags/provider/base.ts index fa89b2e..14a819d 100644 --- a/src/cache-tags/provider/base.ts +++ b/src/cache-tags/provider/base.ts @@ -33,7 +33,7 @@ export abstract class AbstractErrorHandlingCacheTagsProvider implements CacheTag if (this.throwOnError) { throw error; } - console.debug(`Error occurred in ${provider}.${method} with args ${JSON.stringify(args)}.`, { error }); + console.debug(`Error occurred in ${provider}.${method}.`, { error, args }); return fallback; } From d75b4bef419bdee4fca7ddbd1898f53873f55b47 Mon Sep 17 00:00:00 2001 From: Moreno Feltscher Date: Mon, 16 Feb 2026 13:39:33 +0100 Subject: [PATCH 5/6] errorrrrrs --- src/cache-tags/provider/neon.ts | 6 +++--- src/cache-tags/provider/redis.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cache-tags/provider/neon.ts b/src/cache-tags/provider/neon.ts index fad6386..f49e5e6 100644 --- a/src/cache-tags/provider/neon.ts +++ b/src/cache-tags/provider/neon.ts @@ -42,7 +42,7 @@ export class NeonCacheTagsProvider extends AbstractErrorHandlingCacheTagsProvide 'storeQueryCacheTags', [queryId, cacheTags], async () => { - if (cacheTags.length === 0) { + if (!cacheTags?.length) { return; } @@ -60,7 +60,7 @@ export class NeonCacheTagsProvider extends AbstractErrorHandlingCacheTagsProvide 'queriesReferencingCacheTags', [cacheTags], async () => { - if (cacheTags.length === 0) { + if (!cacheTags?.length) { return []; } @@ -88,7 +88,7 @@ export class NeonCacheTagsProvider extends AbstractErrorHandlingCacheTagsProvide 'deleteCacheTags', [cacheTags], async () => { - if (cacheTags.length === 0) { + if (!cacheTags?.length) { return 0; } const placeholders = cacheTags.map((_, i) => `$${i + 1}`).join(','); diff --git a/src/cache-tags/provider/redis.ts b/src/cache-tags/provider/redis.ts index d4353df..78e2e79 100644 --- a/src/cache-tags/provider/redis.ts +++ b/src/cache-tags/provider/redis.ts @@ -38,7 +38,7 @@ export class RedisCacheTagsProvider extends AbstractErrorHandlingCacheTagsProvid 'storeQueryCacheTags', [queryId, cacheTags], async () => { - if (cacheTags.length === 0) { + if (!cacheTags?.length) { return; } @@ -63,7 +63,7 @@ export class RedisCacheTagsProvider extends AbstractErrorHandlingCacheTagsProvid 'queriesReferencingCacheTags', [cacheTags], async () => { - if (cacheTags.length === 0) { + if (!cacheTags?.length) { return []; } @@ -80,7 +80,7 @@ export class RedisCacheTagsProvider extends AbstractErrorHandlingCacheTagsProvid 'deleteCacheTags', [cacheTags], async () => { - if (cacheTags.length === 0) { + if (!cacheTags?.length) { return 0; } From bf0e7c853c6b2b21d957591a370a9e3111872485 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 13:42:19 +0100 Subject: [PATCH 6/6] fix: guard onError callback to prevent masking provider errors (#218) * Initial plan * fix: wrap onError callback in try-catch to prevent masking original errors Co-authored-by: mfeltscher <1352744+mfeltscher@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mfeltscher <1352744+mfeltscher@users.noreply.github.com> Co-authored-by: Moreno Feltscher --- src/cache-tags/provider/base.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/cache-tags/provider/base.ts b/src/cache-tags/provider/base.ts index 14a819d..7a7c191 100644 --- a/src/cache-tags/provider/base.ts +++ b/src/cache-tags/provider/base.ts @@ -28,7 +28,14 @@ export abstract class AbstractErrorHandlingCacheTagsProvider implements CacheTag return await fn(); } catch (error) { const provider = this.providerName; - this.onError?.(error, { provider, method, args }); + + // Call onError callback if provided, but guard against exceptions + // to prevent masking the original provider error + try { + this.onError?.(error, { provider, method, args }); + } catch (handlerError) { + console.error(`Error handler itself failed in ${provider}.${method}.`, { handlerError }); + } if (this.throwOnError) { throw error;