From fa8f2117558e384819b441deca8d82d014537b63 Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Fri, 6 Mar 2026 21:09:48 +0300 Subject: [PATCH 1/2] Invalidate event cache on duplicate key error When a DB duplicate key error indicates the event already exists, clear any cached null result so a subsequent fetch retrieves the newly created document. This change computes the event cache key via getEventCacheKey(projectId, uniqueEventHash) and deletes it (this.cache.del) before retrying handle(task), preventing stale null caches from causing missed event repetitions. --- workers/grouper/src/index.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/workers/grouper/src/index.ts b/workers/grouper/src/index.ts index f016c850..dbfab314 100644 --- a/workers/grouper/src/index.ts +++ b/workers/grouper/src/index.ts @@ -226,6 +226,14 @@ export default class GrouperWorker extends Worker { * and we need to process this event as repetition */ if (e.code?.toString() === DB_DUPLICATE_KEY_ERROR) { + /** + * Invalidate cached null from getEvent so the retry + * fetches the now-existing document from the database + */ + const eventCacheKey = await this.getEventCacheKey(task.projectId, uniqueEventHash); + + this.cache.del(eventCacheKey); + await this.handle(task); return; From 2ee8a1f170d2af9aa66026a180c516644b947fbd Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Fri, 6 Mar 2026 21:53:13 +0300 Subject: [PATCH 2/2] add test case --- .../tests/duplicate-key-recovery.test.ts | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 workers/grouper/tests/duplicate-key-recovery.test.ts diff --git a/workers/grouper/tests/duplicate-key-recovery.test.ts b/workers/grouper/tests/duplicate-key-recovery.test.ts new file mode 100644 index 00000000..3ab7380d --- /dev/null +++ b/workers/grouper/tests/duplicate-key-recovery.test.ts @@ -0,0 +1,135 @@ +import GrouperWorker from '../src'; +import type { GroupWorkerTask } from '../types/group-worker-task'; +import type { ErrorsCatcherType, EventAddons, EventData } from '@hawk.so/types'; + +jest.mock('amqplib'); + +/** + * Ensure DatabaseController constructor sees some connection string + * so it does not throw before we stub database dependencies on worker instance. + */ +process.env.MONGO_EVENTS_DATABASE_URI = process.env.MONGO_EVENTS_DATABASE_URI || 'mongodb://127.0.0.1:27017/hawk-test'; +process.env.MONGO_ACCOUNTS_DATABASE_URI = process.env.MONGO_ACCOUNTS_DATABASE_URI || 'mongodb://127.0.0.1:27017/hawk-test'; + +/** + * Generates minimal task for testing + * + * @param event - allows to override some event properties in generated task + */ +function generateTask(event: Partial> = undefined): GroupWorkerTask { + return { + projectId: '5d206f7f9aaf7c0071d64596', + catcherType: 'errors/javascript', + timestamp: Math.floor(Date.now() / 1000), + payload: Object.assign({ + title: 'Duplicate key recovery test', + backtrace: [], + user: { + id: 'user-1', + }, + context: {}, + addons: {}, + }, event), + }; +} + +describe('GrouperWorker duplicate key error recovery (cache interaction)', () => { + let worker: GrouperWorker; + + beforeEach(() => { + worker = new GrouperWorker(); + + /** + * Prepare real in-memory cache controller + */ + (worker as unknown as { prepareCache: () => void }).prepareCache(); + + /** + * Stub external dependencies that are not relevant for this unit test + */ + (worker as any).eventsDb = {}; + (worker as any).accountsDb = {}; + (worker as any).redis = { + checkOrSetlockEventForAffectedUsersIncrement: jest.fn().mockResolvedValue(false), + checkOrSetlockDailyEventForAffectedUsersIncrement: jest.fn().mockResolvedValue(false), + safeTsAdd: jest.fn().mockResolvedValue(undefined), + }; + }); + + test('Should invalidate stale event cache and process duplicate-key as repetition instead of recursing infinitely', async () => { + const task = generateTask(); + const cache = (worker as any).cache; + + const uniqueEventHash = 'test-hash'; + + /** + * Always use the same hash and force grouping by hash (no patterns) + */ + jest.spyOn(worker as any, 'getUniqueEventHash').mockResolvedValue(uniqueEventHash); + jest.spyOn(worker as any, 'findSimilarEvent').mockResolvedValue(undefined); + + const eventCacheKey = await (worker as any).getEventCacheKey(task.projectId, uniqueEventHash); + + /** + * Simulate stale cache entry created before another worker inserted the event + * Cached value is null, but the "database" already contains the event + */ + cache.set(eventCacheKey, null); + + const dbEvent = { + groupHash: uniqueEventHash, + payload: task.payload, + timestamp: task.timestamp, + totalCount: 1, + }; + + /** + * Use real CacheController semantics for getEvent: first call returns cached null, + * subsequent calls should see the real event once the cache key is deleted. + */ + (worker as any).getEvent = jest.fn(async () => { + return cache.get(eventCacheKey, async () => dbEvent); + }); + + /** + * saveEvent always throws duplicate-key error to trigger the recursive path. + * With the fix, this branch is executed only once; without the fix it will + * recurse indefinitely because isFirstOccurrence stays true. + */ + const duplicateError: NodeJS.ErrnoException = new Error('E11000 duplicate key error') as NodeJS.ErrnoException; + + duplicateError.code = '11000'; + + const saveEventMock = jest.fn(async () => { + throw duplicateError; + }); + + (worker as any).saveEvent = saveEventMock; + + const incrementMock = jest.fn().mockResolvedValue(1); + const saveRepetitionMock = jest.fn().mockResolvedValue('rep-1'); + const saveDailyEventsMock = jest.fn().mockResolvedValue(undefined); + const recordMetricsMock = jest.fn().mockResolvedValue(undefined); + + (worker as any).incrementEventCounterAndAffectedUsers = incrementMock; + (worker as any).saveRepetition = saveRepetitionMock; + (worker as any).saveDailyEvents = saveDailyEventsMock; + (worker as any).recordProjectMetrics = recordMetricsMock; + + await worker.handle(task); + + /** + * Without the cache invalidation fix, this call above would never resolve + * because handle() would recurse indefinitely on duplicate-key error. + * The assertions below verify that we only tried to insert once and then + * proceeded as a repetition with a cached original event. + */ + expect(saveEventMock).toHaveBeenCalledTimes(1); + expect((worker as any).getEvent).toHaveBeenCalledTimes(2); + expect(incrementMock).toHaveBeenCalledTimes(1); + expect(saveRepetitionMock).toHaveBeenCalledTimes(1); + expect(saveDailyEventsMock).toHaveBeenCalledTimes(1); + expect(recordMetricsMock).toHaveBeenCalledTimes(1); + }, 10000); +}); +