Skip to content

Commit ef1f6de

Browse files
feat(observability): fan-out errors Sentry + PostHog + opt-in flags (#3489)
* feat(observability): fan-out errors Sentry + PostHog + opt-in flags (#3486) - New `lib/services/errorTracker.js`: `captureException(err, ctx)` fans out to Sentry (if dsn) and PostHog (if apiKey + errorTracking===true). Silent no-op when neither is configured. `setupExpressErrorHandler` replaces the previous Sentry-only wiring in express.js. - `analytics.js`: add `captureException(err, ctx)` — sends PostHog `$exception` event with message, type, stack, requestId. - `express.js`: gate `analyticsMiddleware` behind `posthog.autoCapture===true` (was unconditionally mounted); replace `sentry.setupExpressErrorHandler` with `errorTracker.setupExpressErrorHandler` (Sentry handler + fan-out middleware). - Config defaults: add `posthog.errorTracking: false` and `posthog.autoCapture: false` explicit opt-in flags — default-safe invariant. - Tests: 11 unit tests covering all 4 tracker combinations + init + Express middleware, including default-safe invariant verification. Closes #3486 * test(analytics): add captureException unit tests for 100% patch coverage Add 4 tests covering captureException: correct $exception event payload, anonymous fallback distinctId, no-op when client not initialised, and silent error swallowing. Ensures codecov/patch threshold is met. * fix(observability): address CodeRabbit review — no double-Sentry, _res rename, req.user._id - `errorTracker.js`: add `captureExceptionPostHogOnly` to avoid double-reporting to Sentry in the Express error middleware (Sentry already captured via its own handler). Rename unused `res` to `_res`. Derive `distinctId` from `req.user._id` / `req.user.id` (not the non-existent `req.userId`). - Tests: update setupExpressErrorHandler test to verify Sentry is NOT called from middleware. Add 2 new tests for req.user.id fallback and anonymous.
1 parent f8542e3 commit ef1f6de

6 files changed

Lines changed: 498 additions & 7 deletions

File tree

config/defaults/development.config.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ const config = {
8787
posthog: {
8888
// apiKey: process.env.DEVKIT_NODE_posthog_apiKey ?? '',
8989
// host: process.env.DEVKIT_NODE_posthog_host ?? 'https://us.i.posthog.com',
90+
errorTracking: false, // opt-in: capture exceptions to PostHog (default: off)
91+
autoCapture: false, // opt-in: auto-capture api_request events (default: off)
9092
},
9193
domain: '',
9294
cookie: {

lib/services/analytics.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,34 @@ const isFeatureEnabled = async (flag, distinctId, options) => {
9393
return client.isFeatureEnabled(flag, distinctId, options);
9494
};
9595

96+
/**
97+
* Capture an exception event in PostHog.
98+
* Only active when `errorTracking` is opted-in via config — the caller
99+
* (`errorTracker.js`) is responsible for checking the flag before calling.
100+
* Safe to call when the PostHog client was never initialised.
101+
* @param {Error} err - Error to capture
102+
* @param {Object} [ctx] - Optional context attached to the event
103+
* @param {string} [ctx.distinctId] - User identifier
104+
* @param {string} [ctx.requestId] - Request trace ID
105+
* @returns {void}
106+
*/
107+
const captureException = (err, ctx = {}) => {
108+
if (!client) return;
109+
try {
110+
const distinctId = ctx.distinctId || 'anonymous';
111+
client.capture({
112+
distinctId,
113+
event: '$exception',
114+
properties: {
115+
$exception_message: err?.message,
116+
$exception_type: err?.name,
117+
$exception_stack: err?.stack,
118+
requestId: ctx.requestId,
119+
},
120+
});
121+
} catch (_) { /* analytics must never break caller */ }
122+
};
123+
96124
/**
97125
* Flush pending events and shut down the PostHog client.
98126
* Safe to call even when the client was never initialised.
@@ -111,5 +139,6 @@ export default {
111139
groupIdentify,
112140
getFeatureFlag,
113141
isFeatureEnabled,
142+
captureException,
114143
shutdown,
115144
};

lib/services/errorTracker.js

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/**
2+
* Module dependencies
3+
*/
4+
import config from '../../config/index.js';
5+
import sentryService from './sentry.js';
6+
import analyticsService from './analytics.js';
7+
8+
/**
9+
* Capture an exception, fanning out to all active trackers.
10+
*
11+
* - Sentry : active when `config.sentry.dsn` is set (and `enabled !== false`)
12+
* - PostHog : active when `config.posthog.apiKey` is set AND
13+
* `config.posthog.errorTracking === true`
14+
*
15+
* Safe no-op when neither tracker is configured.
16+
*
17+
* @param {Error} err - Error to capture
18+
* @param {Object} [ctx] - Optional context attached to the event
19+
* @param {string} [ctx.distinctId] - User identifier for PostHog
20+
* @param {string} [ctx.requestId] - Request trace ID
21+
* @returns {void}
22+
*/
23+
const captureException = (err, ctx = {}) => {
24+
// Sentry fan-out
25+
const sentryConfig = config?.sentry ?? {};
26+
if (sentryConfig.dsn && sentryConfig.enabled !== false) {
27+
sentryService.captureException(err);
28+
}
29+
30+
// PostHog fan-out — only when errorTracking is explicitly opted-in
31+
const posthogConfig = config?.posthog ?? {};
32+
if (posthogConfig.apiKey && posthogConfig.errorTracking === true) {
33+
analyticsService.captureException(err, ctx);
34+
}
35+
};
36+
37+
/**
38+
* Capture an exception in PostHog only (skips Sentry).
39+
* Used inside the Express error middleware where Sentry's own Express handler
40+
* has already reported the error — calling captureException() here would
41+
* report it to Sentry a second time.
42+
*
43+
* @param {Error} err - Error to capture
44+
* @param {Object} [ctx] - Optional context
45+
* @returns {void}
46+
*/
47+
const captureExceptionPostHogOnly = (err, ctx = {}) => {
48+
const posthogConfig = config?.posthog ?? {};
49+
if (posthogConfig.apiKey && posthogConfig.errorTracking === true) {
50+
analyticsService.captureException(err, ctx);
51+
}
52+
};
53+
54+
/**
55+
* Initialise all configured trackers (Sentry + PostHog).
56+
* Safe to call when neither is configured.
57+
* @returns {Promise<void>}
58+
*/
59+
const init = async () => {
60+
await Promise.all([
61+
sentryService.init(),
62+
analyticsService.init(),
63+
]);
64+
};
65+
66+
/**
67+
* Set up Express error handling for all active trackers.
68+
*
69+
* Must be called after all routes are mounted.
70+
* Mounts Sentry's Express error handler first (captures structured request
71+
* context), then a PostHog-only fan-out middleware to avoid double-reporting
72+
* to Sentry (which is already covered by Sentry's own Express handler).
73+
*
74+
* @param {import('express').Express} app - Express application instance
75+
*/
76+
const setupExpressErrorHandler = (app) => {
77+
// Sentry Express handler (structured request/response context)
78+
sentryService.setupExpressErrorHandler(app);
79+
80+
// PostHog-only fan-out middleware — Sentry already handled above
81+
// `_res` is required for Express to recognise this as a 4-arg error handler
82+
app.use((err, req, _res, next) => {
83+
const distinctId = req.user?._id
84+
? String(req.user._id)
85+
: req.user?.id
86+
? String(req.user.id)
87+
: 'anonymous';
88+
captureExceptionPostHogOnly(err, { distinctId, requestId: req.id });
89+
next(err);
90+
});
91+
};
92+
93+
export default {
94+
init,
95+
captureException,
96+
captureExceptionPostHogOnly,
97+
setupExpressErrorHandler,
98+
};

lib/services/express.js

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import config from '../../config/index.js';
2121
import guidesHelper from '../helpers/guides.js';
2222
import logger from './logger.js';
2323
import requestId from '../middlewares/requestId.js';
24-
import sentry from './sentry.js';
24+
import errorTracker from './errorTracker.js';
2525
import AnalyticsService from './analytics.js';
2626
import analyticsMiddleware from '../middlewares/analytics.js';
2727

@@ -288,12 +288,15 @@ const init = async () => {
288288
app.use(requestId);
289289
// Initialize pre-parser routes (before body parsing, CSRF, and analytics)
290290
await initPreParserRoutes(app);
291-
// Initialize analytics (PostHog) and mount auto-capture middleware
292-
// Mounted after pre-parser routes so webhooks are not tracked
293-
// Wrapped in try/catch so analytics failures never prevent app startup
291+
// Initialize analytics (PostHog).
292+
// Mounted after pre-parser routes so webhooks are not tracked.
293+
// Wrapped in try/catch so analytics failures never prevent app startup.
294+
// auto-capture middleware is opt-in: only mounted when posthog.autoCapture === true.
294295
try {
295296
await AnalyticsService.init();
296-
app.use(analyticsMiddleware);
297+
if (config.posthog?.autoCapture === true) {
298+
app.use(analyticsMiddleware);
299+
}
297300
} catch (err) {
298301
logger.warn('[analytics] init failed, running without analytics: %s', err.message);
299302
}
@@ -311,8 +314,8 @@ const init = async () => {
311314
await initModulesServerPolicies(app);
312315
// Initialize modules server routes
313316
await initModulesServerRoutes(app);
314-
// Mount Sentry error handler (must be after routes, before generic error handler)
315-
sentry.setupExpressErrorHandler(app);
317+
// Mount error tracker handler (Sentry + fan-out) — must be after routes
318+
errorTracker.setupExpressErrorHandler(app);
316319
// Initialize error routes
317320
initErrorRoutes(app);
318321
return app;

lib/services/tests/analytics.service.unit.tests.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,5 +236,58 @@ describe('Analytics service unit tests:', () => {
236236
AnalyticsService.track('user-1', 'test');
237237
expect(mockPostHogInstance.capture).not.toHaveBeenCalled();
238238
});
239+
240+
test('captureException should send $exception event with correct properties', async () => {
241+
const mod = await import('../analytics.js');
242+
AnalyticsService = mod.default;
243+
244+
await AnalyticsService.init();
245+
const err = new Error('test error');
246+
err.stack = 'Error: test error\n at test.js:1:1';
247+
AnalyticsService.captureException(err, { distinctId: 'user-1', requestId: 'req-abc' });
248+
249+
expect(mockPostHogInstance.capture).toHaveBeenCalledWith({
250+
distinctId: 'user-1',
251+
event: '$exception',
252+
properties: {
253+
$exception_message: 'test error',
254+
$exception_type: 'Error',
255+
$exception_stack: err.stack,
256+
requestId: 'req-abc',
257+
},
258+
});
259+
});
260+
261+
test('captureException should use anonymous distinctId when not provided', async () => {
262+
const mod = await import('../analytics.js');
263+
AnalyticsService = mod.default;
264+
265+
await AnalyticsService.init();
266+
AnalyticsService.captureException(new Error('anon error'), {});
267+
268+
expect(mockPostHogInstance.capture).toHaveBeenCalledWith(
269+
expect.objectContaining({ distinctId: 'anonymous' }),
270+
);
271+
});
272+
273+
test('captureException should be a no-op when client is not initialised', async () => {
274+
const mod = await import('../analytics.js');
275+
AnalyticsService = mod.default;
276+
277+
// Do NOT call init — client stays null
278+
AnalyticsService.captureException(new Error('no client'), { distinctId: 'user-1' });
279+
expect(mockPostHogInstance.capture).not.toHaveBeenCalled();
280+
});
281+
282+
test('captureException should silently swallow client errors', async () => {
283+
const mod = await import('../analytics.js');
284+
AnalyticsService = mod.default;
285+
286+
await AnalyticsService.init();
287+
mockPostHogInstance.capture.mockImplementation(() => { throw new Error('client error'); });
288+
289+
// Should not throw
290+
expect(() => AnalyticsService.captureException(new Error('err'), {})).not.toThrow();
291+
});
239292
});
240293
});

0 commit comments

Comments
 (0)