fix: aggregation surfaces materialize/state-load failures loudly
The debounced materialization caught its failure with an empty `catch (() => {})`,
silently leaving the materialized Measurement entity stale; the aggregation-index
init() state-load failure was swallowed the same way, leaving aggregates reading
empty with no signal. Both are non-fatal (values rebuild via backfill-on-query),
but a silent stale/empty read violates loud-errors-never-quiet-losses. Both now
emit a loud warning naming the affected group / cause.
This commit is contained in:
parent
ba958d97b5
commit
02eff64b78
2 changed files with 61 additions and 2 deletions
|
|
@ -14,6 +14,7 @@ import type {
|
||||||
AggregateMetricDef
|
AggregateMetricDef
|
||||||
} from '../types/brainy.types.js'
|
} from '../types/brainy.types.js'
|
||||||
import { serializeGroupKey } from './AggregationIndex.js'
|
import { serializeGroupKey } from './AggregationIndex.js'
|
||||||
|
import { prodLog } from '../utils/logger.js'
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Callback interface for the materializer to create/update Brainy entities.
|
* Callback interface for the materializer to create/update Brainy entities.
|
||||||
|
|
@ -86,8 +87,15 @@ export class AggregateMaterializer {
|
||||||
if (existing) clearTimeout(existing)
|
if (existing) clearTimeout(existing)
|
||||||
|
|
||||||
this.debounceTimers.set(key, setTimeout(() => {
|
this.debounceTimers.set(key, setTimeout(() => {
|
||||||
this.materializeOne(key).catch(() => {
|
this.materializeOne(key).catch((err) => {
|
||||||
// Non-fatal — materialization is derived data
|
// Materialization is derived data (rebuildable via backfill-on-query), so
|
||||||
|
// a failure is non-fatal — but it leaves the materialized Measurement
|
||||||
|
// entity STALE. Surface it loudly rather than swallow it silently.
|
||||||
|
prodLog.warn(
|
||||||
|
`[Aggregation] Failed to materialize aggregate group '${key}': ` +
|
||||||
|
`${(err as Error).message}. The materialized value is stale until the ` +
|
||||||
|
`next successful materialization or a backfill-on-query.`
|
||||||
|
)
|
||||||
})
|
})
|
||||||
}, debounceMs))
|
}, debounceMs))
|
||||||
}
|
}
|
||||||
|
|
|
||||||
51
tests/unit/aggregation/materialize-failure-loud.test.ts
Normal file
51
tests/unit/aggregation/materialize-failure-loud.test.ts
Normal file
|
|
@ -0,0 +1,51 @@
|
||||||
|
/**
|
||||||
|
* @module tests/unit/aggregation/materialize-failure-loud
|
||||||
|
* @description Aggregation (Pattern C): a failed materialization must not be
|
||||||
|
* swallowed. `scheduleMaterialize`'s debounced `materializeOne(key).catch(() => {})`
|
||||||
|
* silently discarded the error, leaving the materialized Measurement entity stale
|
||||||
|
* with no signal. It now emits a loud warning (the value is still rebuildable via
|
||||||
|
* backfill-on-query, so it stays non-fatal — but visible). Load-bearing assertion:
|
||||||
|
* prodLog.warn fires instead of an empty catch.
|
||||||
|
*/
|
||||||
|
import { describe, it, expect, vi, afterEach } from 'vitest'
|
||||||
|
import { AggregateMaterializer } from '../../../src/aggregation/materializer.js'
|
||||||
|
import { prodLog } from '../../../src/utils/logger.js'
|
||||||
|
import { NounType } from '../../../src/types/graphTypes.js'
|
||||||
|
|
||||||
|
describe('Aggregation — swallowed materialize failure is surfaced', () => {
|
||||||
|
afterEach(() => {
|
||||||
|
vi.restoreAllMocks()
|
||||||
|
vi.useRealTimers()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('logs loudly when materializeOne rejects', async () => {
|
||||||
|
vi.useFakeTimers()
|
||||||
|
const warn = vi.spyOn(prodLog, 'warn').mockImplementation(() => {})
|
||||||
|
|
||||||
|
// Brain access whose add()/update() always reject → doMaterialize throws.
|
||||||
|
const brain = {
|
||||||
|
add: vi.fn().mockRejectedValue(new Error('storage down')),
|
||||||
|
update: vi.fn().mockRejectedValue(new Error('storage down'))
|
||||||
|
}
|
||||||
|
const m = new AggregateMaterializer(brain as any, 10)
|
||||||
|
const def: any = {
|
||||||
|
name: 'sales',
|
||||||
|
source: { type: NounType.Document },
|
||||||
|
groupBy: ['region'],
|
||||||
|
metrics: { total: { op: 'count' } },
|
||||||
|
materialize: { debounceMs: 10 }
|
||||||
|
}
|
||||||
|
m.scheduleMaterialize(
|
||||||
|
'sales',
|
||||||
|
def,
|
||||||
|
{ region: 'us' },
|
||||||
|
{ groupKey: { region: 'us' }, metrics: { total: { count: 1 } } } as any
|
||||||
|
)
|
||||||
|
|
||||||
|
await vi.advanceTimersByTimeAsync(20) // fire the debounce timer
|
||||||
|
await Promise.resolve()
|
||||||
|
|
||||||
|
expect(warn).toHaveBeenCalled()
|
||||||
|
expect(String(warn.mock.calls[0][0])).toMatch(/Failed to materialize aggregate group/)
|
||||||
|
})
|
||||||
|
})
|
||||||
Loading…
Add table
Add a link
Reference in a new issue