From 90aa5cd307d73626a7bc414bb8a34eb9b6e75d05 Mon Sep 17 00:00:00 2001 From: mwawacek <136260960+mwawacek@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:54:08 +0000 Subject: [PATCH] fix: keep module defaults for fields a preset leaves undefined Drop `undefined` fields of circuit breaker, bulkhead and backoff options before merging them over the module defaults, as retry options already do. A preset or `ResilienceService.create()` field set to `undefined`, such as an unset setting from `ConfigService.get()`, replaced the module default for that field: `circuitBreaker: { minimumCalls: undefined }` under `defaults.circuitBreaker.minimumCalls: 1` gave the built-in 10. Move `definedFields()` to `utils/options.util.ts`, so the registries can use it without importing `PolicyFactory`. --- lib/services/bulkhead-registry.service.ts | 3 +- .../circuit-breaker-registry.service.ts | 4 ++- lib/services/entrypoint-planner.service.ts | 3 +- lib/services/policy-factory.service.ts | 19 +++++----- lib/utils/options.util.ts | 5 +++ tests/module.spec.ts | 36 +++++++++++++++++++ 6 files changed, 56 insertions(+), 14 deletions(-) diff --git a/lib/services/bulkhead-registry.service.ts b/lib/services/bulkhead-registry.service.ts index f1fa667..f7dd523 100644 --- a/lib/services/bulkhead-registry.service.ts +++ b/lib/services/bulkhead-registry.service.ts @@ -4,6 +4,7 @@ import type { BulkheadOptions } from '../interfaces/bulkhead-options.interface.j import type { ResilienceModuleOptions } from '../interfaces/resilience-module-options.interface.js'; import { BulkheadPolicy } from '../policies/bulkhead.policy.js'; import { RESILIENCE_MODULE_OPTIONS } from '../resilience.module-definition.js'; +import { definedFields } from '../utils/options.util.js'; import { ms, NamedRegistry } from './named-registry.service.js'; @Injectable() @@ -18,7 +19,7 @@ export class BulkheadRegistry extends NamedRegistry this.stateful(spec.circuitBreaker!, this.breakers, source, (options) => - new CircuitBreakerPolicy({ ...defaults.circuitBreaker, ...options, name }), + new CircuitBreakerPolicy({ ...defaults.circuitBreaker, ...definedFields(options), name }), ), ); } @@ -179,7 +179,7 @@ export class PolicyFactory { if (spec.bulkhead !== undefined) { stages.bulkhead = underOption('bulkhead', () => this.stateful(spec.bulkhead!, this.bulkheads, source, (options) => - new BulkheadPolicy({ ...defaults.bulkhead, ...options, name }), + new BulkheadPolicy({ ...defaults.bulkhead, ...definedFields(options), name }), ), ); } @@ -245,9 +245,11 @@ function validateStages( ); check('timeout', spec.timeout, (timeout) => new TimeoutPolicy(timeout)); check('circuitBreaker', spec.circuitBreaker, unlessNamed((options) => - new CircuitBreakerPolicy({ ...defaults.circuitBreaker, ...options }), + new CircuitBreakerPolicy({ ...defaults.circuitBreaker, ...definedFields(options) }), + )); + check('bulkhead', spec.bulkhead, unlessNamed((options) => + new BulkheadPolicy({ ...defaults.bulkhead, ...definedFields(options) }), )); - check('bulkhead', spec.bulkhead, unlessNamed((options) => new BulkheadPolicy({ ...defaults.bulkhead, ...options }))); if ('outboundRateLimit' in spec) { check('outboundRateLimit', spec.outboundRateLimit, unlessNamed((options) => new OutboundRateLimitPolicy(options))); @@ -269,7 +271,7 @@ function normalizeDefaultRetry(retry: number | RetryOptions | undefined): RetryO export function mergeRetry(base: RetryOptions, override: R): R { const backoff = typeof base.backoff === 'object' && typeof override.backoff === 'object' - ? { ...base.backoff, ...override.backoff } + ? { ...base.backoff, ...definedFields(override.backoff) } : (override.backoff ?? base.backoff); const merged = { ...base, ...definedFields(override) } as R; @@ -278,8 +280,3 @@ export function mergeRetry(base: RetryOptions, override: } return merged; } - -/** Drops `undefined` fields, so that `{ attempts: undefined }` doesn't erase an inherited value. */ -export function definedFields(value: T): Partial { - return Object.fromEntries(Object.entries(value).filter(([, v]) => v !== undefined)) as Partial; -} diff --git a/lib/utils/options.util.ts b/lib/utils/options.util.ts index f676855..3b191e0 100644 --- a/lib/utils/options.util.ts +++ b/lib/utils/options.util.ts @@ -67,3 +67,8 @@ export function callPredicate( } return !!result; } + +/** @internal Drops `undefined` fields, so that `{ attempts: undefined }` doesn't erase an inherited value. */ +export function definedFields(value: T): Partial { + return Object.fromEntries(Object.entries(value).filter(([, v]) => v !== undefined)) as Partial; +} diff --git a/tests/module.spec.ts b/tests/module.spec.ts index 2fca12f..04364e8 100644 --- a/tests/module.spec.ts +++ b/tests/module.spec.ts @@ -35,6 +35,7 @@ import { Signal, Timeout, TimeoutPolicy, + type Duration, type ResilienceModuleOptions, type ResilienceOptionsFactory, type ResiliencePolicy, @@ -573,6 +574,41 @@ describe('ResilienceModule', () => { expect(stages[1]).toMatchObject({ timeout: 1_000 }); }); + it('keeps module defaults for fields a preset or create() leaves undefined', async () => { + // As `ConfigService.get()` answers for a setting that is not set. + const config = { get: (_key: string): T | undefined => undefined }; + const moduleRef = await boot( + { + defaults: { + circuitBreaker: { minimumCalls: 1, openDuration: '1m' }, + bulkhead: { maxConcurrent: 2 }, + retry: { attempts: 4, backoff: { delay: '1s', jitter: 'none' } }, + }, + presets: { + carrier: { + circuitBreaker: { minimumCalls: config.get('CARRIER_MIN_CALLS') }, + bulkhead: { maxConcurrent: config.get('CARRIER_MAX_CONCURRENT') }, + retry: { backoff: { delay: config.get('CARRIER_DELAY') } }, + }, + }, + }, + {}, + ); + const resilience = moduleRef.get(ResilienceService); + expect(resilience.circuitBreaker('carrier').options).toMatchObject({ minimumCalls: 1, openDuration: 60_000 }); + expect(resilience.bulkhead('carrier').maxConcurrent).toBe(2); + + const retry = (resilience.preset('carrier') as PolicyWrap).policies[0] as RetryPolicy; + const events = recordEvents(); + await expect( + retry.execute(() => Promise.reject(new Error('down')), { signal: AbortSignal.timeout(50) }), + ).rejects.toThrow(); + expect(events[0]).toMatchObject({ type: 'retry', delayMs: 1_000 }); + + const created = resilience.create({ bulkhead: { maxConcurrent: config.get('MAX_CONCURRENT') } }); + expect(((created as PolicyWrap).policies[0] as BulkheadPolicy).maxConcurrent).toBe(2); + }); + it('the options are injectable as RESILIENCE_MODULE_OPTIONS, and tests can override them', async () => { const options = { presets: { carrier: { circuitBreaker: { openDuration: '30s' as const } } } }; expect((await boot(options, {})).get(RESILIENCE_MODULE_OPTIONS)).toEqual(options);