Skip to content

Commit f7ad3b3

Browse files
PierreBrisorgueilPierreBrisorgueil
committed
refactor(analytics): move from module to lib
Analytics has no model, routes, or CASL policy — it's infrastructure. Move service + middleware to lib/ alongside logger, sentry, requestId. Config moves to development.config.js. Middleware mounts in express.js. Also adds try/catch in middleware finish handler (fixes #3321). Closes #3322 Closes #3321
1 parent 1cdba5a commit f7ad3b3

20 files changed

Lines changed: 152 additions & 242 deletions

‎config/defaults/development.config.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ const config = {
9999
environment: process.env.DEVKIT_NODE_sentry_environment || 'development',
100100
enabled: false,
101101
},
102+
posthog: {
103+
// apiKey: process.env.DEVKIT_NODE_posthog_apiKey ?? '',
104+
// host: process.env.DEVKIT_NODE_posthog_host ?? 'https://us.i.posthog.com',
105+
},
102106
domain: '',
103107
cookie: {
104108
secure: false, // false in dev (HTTP localhost)

‎jest.config.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@ export default {
4141
'!<rootDir>/modules/auth/strategies/clerk/**',
4242
// Exclude passport init glue — just serializeUser/deserializeUser + strategy loader
4343
'!<rootDir>/modules/auth/auth.init.js',
44-
// Exclude analytics init glue — just calls AnalyticsService.init()
45-
'!<rootDir>/modules/analytics/analytics.init.js',
4644
// Exclude audit init glue — just logs activation status
4745
'!<rootDir>/modules/audit/audit.init.js',
4846
// Exclude dead code — never imported anywhere in the codebase

‎lib/app.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import config from '../config/index.js';
99
import express from './services/express.js';
1010
import mongooseService from './services/mongoose.js';
1111
import migrations from './services/migrations.js';
12-
import AnalyticsService from '../modules/analytics/services/analytics.service.js';
12+
import AnalyticsService from './services/analytics.js';
1313
import SentryService from './services/sentry.js';
1414

1515
// Establish an SQL server connection, instantiating all models and schemas

modules/analytics/middlewares/analytics.middleware.js renamed to lib/middlewares/analytics.js

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* Module dependencies
33
*/
4-
import AnalyticsService from '../services/analytics.service.js';
4+
import AnalyticsService from '../services/analytics.js';
55

66
/**
77
* Default route prefixes to skip when auto-capturing API request events.
@@ -41,15 +41,17 @@ const createAnalyticsMiddleware = (options = {}) => {
4141
const start = Date.now();
4242

4343
res.on('finish', () => {
44-
const distinctId = req.user?._id ? String(req.user._id) : 'anonymous';
45-
const groups = req.organization?._id ? { company: String(req.organization._id) } : undefined;
44+
try {
45+
const distinctId = req.user?._id ? String(req.user._id) : 'anonymous';
46+
const groups = req.organization?._id ? { company: String(req.organization._id) } : undefined;
4647

47-
AnalyticsService.track(distinctId, 'api_request', {
48-
endpoint,
49-
method: req.method,
50-
statusCode: res.statusCode,
51-
responseTime: Date.now() - start,
52-
}, groups);
48+
AnalyticsService.track(distinctId, 'api_request', {
49+
endpoint,
50+
method: req.method,
51+
statusCode: res.statusCode,
52+
responseTime: Date.now() - start,
53+
}, groups);
54+
} catch (_) { /* analytics must not break request flow */ }
5355
});
5456

5557
return next();

modules/analytics/middlewares/analytics.requireFeatureFlag.js renamed to lib/middlewares/analytics.requireFeatureFlag.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
/**
22
* Module dependencies
33
*/
4-
import FeatureFlagsService from '../services/analytics.featureFlags.service.js';
4+
import FeatureFlagsService from '../services/analytics.featureFlags.js';
55

6-
import responses from '../../../lib/helpers/responses.js';
6+
import responses from '../helpers/responses.js';
77

88
/**
99
* Returns Express middleware that gates access based on a PostHog feature flag.

modules/analytics/tests/analytics.comprehensive.unit.tests.js renamed to lib/middlewares/tests/analytics.comprehensive.unit.tests.js

Lines changed: 6 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@ import express from 'express';
66
import request from 'supertest';
77

88
/**
9-
* Comprehensive tests for the analytics module.
9+
* Comprehensive tests for analytics (lib).
1010
*
1111
* Covers Express integration (middleware in a real server via supertest),
12-
* concurrent request tracking, module init wiring (middleware registration +
13-
* billing event listener), and feature-flag middleware edge cases that
12+
* concurrent request tracking, and feature-flag middleware edge cases that
1413
* individual unit tests do not reach.
1514
*/
1615

@@ -32,7 +31,7 @@ describe('Analytics comprehensive tests:', () => {
3231

3332
mockTrack = jest.fn();
3433

35-
jest.unstable_mockModule('../services/analytics.service.js', () => ({
34+
jest.unstable_mockModule('../../services/analytics.js', () => ({
3635
default: {
3736
track: mockTrack,
3837
init: jest.fn(),
@@ -42,7 +41,7 @@ describe('Analytics comprehensive tests:', () => {
4241
},
4342
}));
4443

45-
const mod = await import('../middlewares/analytics.middleware.js');
44+
const mod = await import('../analytics.js');
4645
analyticsMiddleware = mod.default;
4746

4847
app = express();
@@ -183,106 +182,6 @@ describe('Analytics comprehensive tests:', () => {
183182
});
184183
});
185184

186-
// ─────────────────────────────────────────────────────────────────────
187-
// Init — module wiring
188-
// ─────────────────────────────────────────────────────────────────────
189-
describe('init — module wiring:', () => {
190-
let mockInit;
191-
let mockGroupIdentify;
192-
let mockApp;
193-
194-
beforeEach(async () => {
195-
jest.resetModules();
196-
197-
mockInit = jest.fn();
198-
mockGroupIdentify = jest.fn();
199-
200-
jest.unstable_mockModule('../services/analytics.service.js', () => ({
201-
default: {
202-
init: mockInit.mockResolvedValue(undefined),
203-
track: jest.fn(),
204-
identify: jest.fn(),
205-
groupIdentify: mockGroupIdentify,
206-
shutdown: jest.fn(),
207-
},
208-
}));
209-
210-
jest.unstable_mockModule('../middlewares/analytics.middleware.js', () => ({
211-
default: (_req, _res, next) => next(),
212-
}));
213-
214-
jest.unstable_mockModule('../../billing/lib/events.js', async () => {
215-
const { EventEmitter } = await import('events');
216-
return { default: new EventEmitter() };
217-
});
218-
219-
mockApp = { use: jest.fn() };
220-
});
221-
222-
test('should call AnalyticsService.init() and register middleware', async () => {
223-
const { default: initAnalytics } = await import('../analytics.init.js');
224-
await initAnalytics(mockApp);
225-
226-
expect(mockInit).toHaveBeenCalledTimes(1);
227-
expect(mockApp.use).toHaveBeenCalledTimes(1);
228-
expect(mockApp.use).toHaveBeenCalledWith(expect.any(Function));
229-
});
230-
231-
test('should register billing plan.changed listener', async () => {
232-
const { default: billingEvents } = await import('../../billing/lib/events.js');
233-
const { default: initAnalytics } = await import('../analytics.init.js');
234-
235-
await initAnalytics(mockApp);
236-
237-
billingEvents.emit('plan.changed', {
238-
organizationId: 'org-test',
239-
newPlan: 'enterprise',
240-
});
241-
242-
expect(mockGroupIdentify).toHaveBeenCalledWith(
243-
'company',
244-
'org-test',
245-
{ plan: 'enterprise' },
246-
);
247-
});
248-
249-
test('billing plan.changed should not throw when groupIdentify fails', async () => {
250-
mockGroupIdentify.mockImplementation(() => {
251-
throw new Error('PostHog down');
252-
});
253-
254-
const { default: billingEvents } = await import('../../billing/lib/events.js');
255-
const { default: initAnalytics } = await import('../analytics.init.js');
256-
257-
await initAnalytics(mockApp);
258-
259-
expect(() => {
260-
billingEvents.emit('plan.changed', {
261-
organizationId: 'org-test',
262-
newPlan: 'free',
263-
});
264-
}).not.toThrow();
265-
});
266-
267-
test('should coerce organizationId to string in plan.changed handler', async () => {
268-
const { default: billingEvents } = await import('../../billing/lib/events.js');
269-
const { default: initAnalytics } = await import('../analytics.init.js');
270-
271-
await initAnalytics(mockApp);
272-
273-
billingEvents.emit('plan.changed', {
274-
organizationId: { toString: () => '507f1f77bcf86cd799439011' },
275-
newPlan: 'pro',
276-
});
277-
278-
expect(mockGroupIdentify).toHaveBeenCalledWith(
279-
'company',
280-
'507f1f77bcf86cd799439011',
281-
{ plan: 'pro' },
282-
);
283-
});
284-
});
285-
286185
// ─────────────────────────────────────────────────────────────────────
287186
// requireFeatureFlag — edge cases
288187
// ─────────────────────────────────────────────────────────────────────
@@ -301,11 +200,11 @@ describe('Analytics comprehensive tests:', () => {
301200
getVariant: jest.fn(),
302201
};
303202

304-
jest.unstable_mockModule('../services/analytics.featureFlags.service.js', () => ({
203+
jest.unstable_mockModule('../../services/analytics.featureFlags.js', () => ({
305204
default: mockFeatureFlagsService,
306205
}));
307206

308-
const mod = await import('../middlewares/analytics.requireFeatureFlag.js');
207+
const mod = await import('../analytics.requireFeatureFlag.js');
309208
requireFeatureFlag = mod.default;
310209

311210
req = {

modules/analytics/tests/analytics.middleware.integration.tests.js renamed to lib/middlewares/tests/analytics.middleware.integration.tests.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@ describe('Analytics middleware integration tests:', () => {
1919

2020
mockTrack = jest.fn();
2121

22-
jest.unstable_mockModule('../services/analytics.service.js', () => ({
22+
jest.unstable_mockModule('../../services/analytics.js', () => ({
2323
default: {
2424
track: mockTrack,
2525
},
2626
}));
2727

28-
const { createAnalyticsMiddleware } = await import('../middlewares/analytics.middleware.js');
28+
const { createAnalyticsMiddleware } = await import('../analytics.js');
2929

3030
app = express();
3131
app.use(createAnalyticsMiddleware());

modules/analytics/tests/analytics.middleware.unit.tests.js renamed to lib/middlewares/tests/analytics.middleware.unit.tests.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ describe('Analytics middleware unit tests:', () => {
2323

2424
mockTrack = jest.fn();
2525

26-
jest.unstable_mockModule('../services/analytics.service.js', () => ({
26+
jest.unstable_mockModule('../../services/analytics.js', () => ({
2727
default: {
2828
track: mockTrack,
2929
},
3030
}));
3131

32-
const mod = await import('../middlewares/analytics.middleware.js');
32+
const mod = await import('../analytics.js');
3333
analyticsMiddleware = mod.default;
3434
createAnalyticsMiddleware = mod.createAnalyticsMiddleware;
3535

@@ -226,4 +226,16 @@ describe('Analytics middleware unit tests:', () => {
226226
// /api/health is NOT in the custom prefixes, so it should be tracked
227227
expect(res.on).toHaveBeenCalledWith('finish', expect.any(Function));
228228
});
229+
230+
test('should not throw when track throws inside finish handler', () => {
231+
mockTrack.mockImplementation(() => {
232+
throw new Error('PostHog exploded');
233+
});
234+
235+
analyticsMiddleware(req, res, next);
236+
237+
// The finish handler should swallow the error
238+
expect(() => triggerFinish()).not.toThrow();
239+
expect(mockTrack).toHaveBeenCalledTimes(1);
240+
});
229241
});

modules/analytics/tests/analytics.requireFeatureFlag.unit.tests.js renamed to lib/middlewares/tests/analytics.requireFeatureFlag.unit.tests.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ describe('requireFeatureFlag middleware unit tests:', () => {
2121
getVariant: jest.fn(),
2222
};
2323

24-
jest.unstable_mockModule('../services/analytics.featureFlags.service.js', () => ({
24+
jest.unstable_mockModule('../../services/analytics.featureFlags.js', () => ({
2525
default: mockFeatureFlagsService,
2626
}));
2727

28-
const mod = await import('../middlewares/analytics.requireFeatureFlag.js');
28+
const mod = await import('../analytics.requireFeatureFlag.js');
2929
requireFeatureFlag = mod.default;
3030

3131
req = {

modules/analytics/services/analytics.featureFlags.service.js renamed to lib/services/analytics.featureFlags.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* Module dependencies
33
*/
4-
import AnalyticsService from './analytics.service.js';
4+
import AnalyticsService from './analytics.js';
55

66
/**
77
* Check whether a feature flag is enabled for a given user.

0 commit comments

Comments
 (0)