From 0d918349090c49cc006ae6f059747bb5db330a33 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Mon, 11 May 2026 15:46:14 +0200 Subject: [PATCH 1/2] fix(observability): use posthog-node captureException SDK method for UI grouping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces legacy client.capture({event: '\$exception', props}) with SDK client.captureException(err, distinctId, additionalProperties) so PostHog auto-populates \$exception_list + \$exception_fingerprint — required by the Error Tracking UI for grouping/display. Fixes #3658. --- lib/services/analytics.js | 24 +++--- .../analytics.captureException.unit.tests.js | 74 ++++++++++++------- .../tests/analytics.service.unit.tests.js | 29 ++++---- ...ger.posthog.transport.integration.tests.js | 20 ++--- 4 files changed, 82 insertions(+), 65 deletions(-) diff --git a/lib/services/analytics.js b/lib/services/analytics.js index 0879dbbd0..c635eb72e 100644 --- a/lib/services/analytics.js +++ b/lib/services/analytics.js @@ -151,6 +151,9 @@ const isFeatureEnabled = async (flag, distinctId, options) => { * (`errorTracker.js`) is responsible for checking the flag before calling. * Safe to call when the PostHog client was never initialised. * + * Uses SDK native `client.captureException()` so PostHog Error Tracking UI + * groups events via auto-generated `$exception_list` + `$exception_fingerprint`. + * * Source attribution (highest wins): * 1. explicit `ctx.source` * 2. `ctx.properties.source` @@ -169,21 +172,14 @@ const captureException = (err, ctx = {}) => { if (!err) return; try { const distinctId = ctx.distinctId || 'anonymous'; - const defaults = { source: 'system' }; const explicit = ctx.source ? { source: ctx.source } : {}; - client.capture({ - distinctId, - event: '$exception', - properties: { - $exception_message: err?.message, - $exception_type: err?.name, - $exception_stack: err?.stack, - requestId: ctx.requestId, - ...defaults, - ...(ctx.properties ?? {}), - ...explicit, - }, - }); + const additionalProperties = { + requestId: ctx.requestId, + source: 'system', + ...(ctx.properties ?? {}), + ...explicit, + }; + client.captureException(err, distinctId, additionalProperties); } catch (_) { /* analytics must never break caller */ } }; diff --git a/lib/services/tests/analytics.captureException.unit.tests.js b/lib/services/tests/analytics.captureException.unit.tests.js index 64c7caa88..65136e16b 100644 --- a/lib/services/tests/analytics.captureException.unit.tests.js +++ b/lib/services/tests/analytics.captureException.unit.tests.js @@ -11,6 +11,7 @@ describe('Analytics captureException():', () => { jest.resetModules(); mockPostHogInstance = { capture: jest.fn(), + captureException: jest.fn(), identify: jest.fn(), groupIdentify: jest.fn(), getFeatureFlag: jest.fn().mockResolvedValue(undefined), @@ -30,58 +31,81 @@ describe('Analytics captureException():', () => { afterEach(() => { jest.restoreAllMocks(); }); - test('emits $exception with default source="system" when no ctx', () => { - AnalyticsService.captureException(new Error('boom')); - expect(mockPostHogInstance.capture).toHaveBeenCalledWith(expect.objectContaining({ - event: '$exception', - properties: expect.objectContaining({ source: 'system', $exception_message: 'boom' }), - })); + test('calls SDK captureException with default source="system" when no ctx', () => { + const err = new Error('boom'); + AnalyticsService.captureException(err); + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + err, + 'anonymous', + expect.objectContaining({ source: 'system' }), + ); }); test('honours explicit ctx.source', () => { - AnalyticsService.captureException(new Error('boom'), { source: 'worker-callback' }); - expect(mockPostHogInstance.capture).toHaveBeenCalledWith(expect.objectContaining({ - properties: expect.objectContaining({ source: 'worker-callback' }), - })); + const err = new Error('boom'); + AnalyticsService.captureException(err, { source: 'worker-callback' }); + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + err, + 'anonymous', + expect.objectContaining({ source: 'worker-callback' }), + ); }); - test('merges ctx.properties (logMessage/logLevel) into event', () => { - AnalyticsService.captureException(new Error('boom'), { + test('merges ctx.properties (logMessage/logLevel) into additionalProperties', () => { + const err = new Error('boom'); + AnalyticsService.captureException(err, { distinctId: 'u1', properties: { logMessage: 'something failed', logLevel: 'error' }, }); - expect(mockPostHogInstance.capture).toHaveBeenCalledWith(expect.objectContaining({ - distinctId: 'u1', - properties: expect.objectContaining({ + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + err, + 'u1', + expect.objectContaining({ logMessage: 'something failed', logLevel: 'error', source: 'system', }), - })); + ); }); test('ctx.properties.source wins over system default', () => { - AnalyticsService.captureException(new Error('boom'), { + const err = new Error('boom'); + AnalyticsService.captureException(err, { properties: { source: 'stripe-webhook' }, }); - expect(mockPostHogInstance.capture).toHaveBeenCalledWith(expect.objectContaining({ - properties: expect.objectContaining({ source: 'stripe-webhook' }), - })); + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + err, + 'anonymous', + expect.objectContaining({ source: 'stripe-webhook' }), + ); }); test('explicit ctx.source wins over ctx.properties.source', () => { - AnalyticsService.captureException(new Error('boom'), { + const err = new Error('boom'); + AnalyticsService.captureException(err, { source: 'cron', properties: { source: 'web' }, }); - expect(mockPostHogInstance.capture).toHaveBeenCalledWith(expect.objectContaining({ - properties: expect.objectContaining({ source: 'cron' }), - })); + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + err, + 'anonymous', + expect.objectContaining({ source: 'cron' }), + ); }); test('no-op when err is null/undefined', () => { AnalyticsService.captureException(null); AnalyticsService.captureException(undefined); - expect(mockPostHogInstance.capture).not.toHaveBeenCalled(); + expect(mockPostHogInstance.captureException).not.toHaveBeenCalled(); + }); + + test('passes requestId in additionalProperties', () => { + const err = new Error('with-req'); + AnalyticsService.captureException(err, { distinctId: 'u2', requestId: 'req-xyz' }); + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + err, + 'u2', + expect.objectContaining({ requestId: 'req-xyz' }), + ); }); }); diff --git a/lib/services/tests/analytics.service.unit.tests.js b/lib/services/tests/analytics.service.unit.tests.js index 25a6345d8..ff2ee7ffb 100644 --- a/lib/services/tests/analytics.service.unit.tests.js +++ b/lib/services/tests/analytics.service.unit.tests.js @@ -15,6 +15,7 @@ describe('Analytics service unit tests:', () => { mockPostHogInstance = { capture: jest.fn(), + captureException: jest.fn(), identify: jest.fn(), groupIdentify: jest.fn(), getFeatureFlag: jest.fn().mockResolvedValue('variant-a'), @@ -243,7 +244,7 @@ describe('Analytics service unit tests:', () => { expect(mockPostHogInstance.capture).not.toHaveBeenCalled(); }); - test('captureException should send $exception event with correct properties', async () => { + test('captureException should call SDK captureException with correct args', async () => { const mod = await import('../analytics.js'); AnalyticsService = mod.default; @@ -252,17 +253,11 @@ describe('Analytics service unit tests:', () => { err.stack = 'Error: test error\n at test.js:1:1'; AnalyticsService.captureException(err, { distinctId: 'user-1', requestId: 'req-abc' }); - expect(mockPostHogInstance.capture).toHaveBeenCalledWith({ - distinctId: 'user-1', - event: '$exception', - properties: { - $exception_message: 'test error', - $exception_type: 'Error', - $exception_stack: err.stack, - requestId: 'req-abc', - source: 'system', - }, - }); + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + err, + 'user-1', + expect.objectContaining({ requestId: 'req-abc', source: 'system' }), + ); }); test('captureException should use anonymous distinctId when not provided', async () => { @@ -272,8 +267,10 @@ describe('Analytics service unit tests:', () => { await AnalyticsService.init(); AnalyticsService.captureException(new Error('anon error'), {}); - expect(mockPostHogInstance.capture).toHaveBeenCalledWith( - expect.objectContaining({ distinctId: 'anonymous' }), + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + expect.any(Error), + 'anonymous', + expect.any(Object), ); }); @@ -283,7 +280,7 @@ describe('Analytics service unit tests:', () => { // Do NOT call init — client stays null AnalyticsService.captureException(new Error('no client'), { distinctId: 'user-1' }); - expect(mockPostHogInstance.capture).not.toHaveBeenCalled(); + expect(mockPostHogInstance.captureException).not.toHaveBeenCalled(); }); test('captureException should silently swallow client errors', async () => { @@ -291,7 +288,7 @@ describe('Analytics service unit tests:', () => { AnalyticsService = mod.default; await AnalyticsService.init(); - mockPostHogInstance.capture.mockImplementation(() => { throw new Error('client error'); }); + mockPostHogInstance.captureException.mockImplementation(() => { throw new Error('client error'); }); // Should not throw expect(() => AnalyticsService.captureException(new Error('err'), {})).not.toThrow(); diff --git a/lib/services/tests/logger.posthog.transport.integration.tests.js b/lib/services/tests/logger.posthog.transport.integration.tests.js index eae23b888..be5ee1de7 100644 --- a/lib/services/tests/logger.posthog.transport.integration.tests.js +++ b/lib/services/tests/logger.posthog.transport.integration.tests.js @@ -8,6 +8,7 @@ describe('logger.error → PostHog $exception (integration):', () => { jest.resetModules(); mockPostHogInstance = { capture: jest.fn(), + captureException: jest.fn(), identify: jest.fn(), groupIdentify: jest.fn(), getFeatureFlag: jest.fn().mockResolvedValue(undefined), @@ -33,30 +34,29 @@ describe('logger.error → PostHog $exception (integration):', () => { afterEach(() => { jest.restoreAllMocks(); }); - test('logger.error(message, { error }) emits a single $exception event', () => { + test('logger.error(message, { error }) emits a single $exception event via SDK captureException', () => { const err = new Error('payment failed'); logger.error('Charge failed for user', { error: err }); - expect(mockPostHogInstance.capture).toHaveBeenCalledWith(expect.objectContaining({ - event: '$exception', - properties: expect.objectContaining({ - $exception_message: 'payment failed', - $exception_type: 'Error', + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + err, + 'anonymous', + expect.objectContaining({ logMessage: 'Charge failed for user', logLevel: 'error', source: 'system', }), - })); + ); }); - test('logger.error(err) directly emits a single $exception event', () => { + test('logger.error(err) directly emits a single $exception event via SDK captureException', () => { const err = new Error('boom'); logger.error(err); - expect(mockPostHogInstance.capture).toHaveBeenCalledTimes(1); + expect(mockPostHogInstance.captureException).toHaveBeenCalledTimes(1); }); test('error already marked posthogCaptured does NOT re-emit', () => { const err = Object.assign(new Error('boom'), { posthogCaptured: true }); logger.error('skipped', { error: err }); - expect(mockPostHogInstance.capture).not.toHaveBeenCalled(); + expect(mockPostHogInstance.captureException).not.toHaveBeenCalled(); }); }); From 50fe7ff69f7b0f4c9473cdb17759da6128475b3d Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Mon, 11 May 2026 16:08:06 +0200 Subject: [PATCH 2/2] fix(analytics): make ctx.requestId authoritative + assert single emit CodeRabbit findings on PR #3659: - captureException additionalProperties construction let ctx.properties.requestId override ctx.requestId. Move requestId spread after ctx.properties so the explicit ctx.requestId wins. - logger.posthog.transport.integration.tests.js asserted payload shape but not call count. Add toHaveBeenCalledTimes(1). +1 unit test for ctx.requestId precedence. --- lib/services/analytics.js | 2 +- .../tests/analytics.captureException.unit.tests.js | 13 +++++++++++++ .../logger.posthog.transport.integration.tests.js | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/services/analytics.js b/lib/services/analytics.js index c635eb72e..69d73ebe9 100644 --- a/lib/services/analytics.js +++ b/lib/services/analytics.js @@ -174,9 +174,9 @@ const captureException = (err, ctx = {}) => { const distinctId = ctx.distinctId || 'anonymous'; const explicit = ctx.source ? { source: ctx.source } : {}; const additionalProperties = { - requestId: ctx.requestId, source: 'system', ...(ctx.properties ?? {}), + ...(ctx.requestId !== undefined ? { requestId: ctx.requestId } : {}), ...explicit, }; client.captureException(err, distinctId, additionalProperties); diff --git a/lib/services/tests/analytics.captureException.unit.tests.js b/lib/services/tests/analytics.captureException.unit.tests.js index 65136e16b..4782c3485 100644 --- a/lib/services/tests/analytics.captureException.unit.tests.js +++ b/lib/services/tests/analytics.captureException.unit.tests.js @@ -108,4 +108,17 @@ describe('Analytics captureException():', () => { expect.objectContaining({ requestId: 'req-xyz' }), ); }); + + test('ctx.requestId wins over ctx.properties.requestId (authoritative)', () => { + const err = new Error('req-precedence'); + AnalyticsService.captureException(err, { + requestId: 'authoritative-req', + properties: { requestId: 'should-be-overridden' }, + }); + expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( + err, + 'anonymous', + expect.objectContaining({ requestId: 'authoritative-req' }), + ); + }); }); diff --git a/lib/services/tests/logger.posthog.transport.integration.tests.js b/lib/services/tests/logger.posthog.transport.integration.tests.js index be5ee1de7..cb203b1d5 100644 --- a/lib/services/tests/logger.posthog.transport.integration.tests.js +++ b/lib/services/tests/logger.posthog.transport.integration.tests.js @@ -37,6 +37,7 @@ describe('logger.error → PostHog $exception (integration):', () => { test('logger.error(message, { error }) emits a single $exception event via SDK captureException', () => { const err = new Error('payment failed'); logger.error('Charge failed for user', { error: err }); + expect(mockPostHogInstance.captureException).toHaveBeenCalledTimes(1); expect(mockPostHogInstance.captureException).toHaveBeenCalledWith( err, 'anonymous',