fix(observability): use posthog-node captureException SDK method for UI grouping (#3659)

* fix(observability): use posthog-node captureException SDK method for UI grouping

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.

* 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.

Author: PierreBrisorgueil

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 = {
+      source: 'system',
+      ...(ctx.properties ?? {}),
+      ...(ctx.requestId !== undefined ? { requestId: ctx.requestId } : {}),
+      ...explicit,
+    };
+    client.captureException(err, distinctId, additionalProperties);
   } catch (_) { /* analytics must never break caller */ }
 };
 

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,94 @@ 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' }),
+    );
+  });
+
+  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' }),
+    );
   });
 });

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,15 +280,15 @@ 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 () => {
       const mod = await import('../analytics.js');
       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();

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,30 @@ 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).toHaveBeenCalledTimes(1);
+    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();
   });
 });