fix(analytics): make service error-resilient, fix failing tests (#3361)

* fix(billing): replace dot separator with underscore in counter keys

requireQuota middleware and getUsage controller built counter keys with
dots (resource.action) which are rejected by SAFE_KEY_RE in the usage
repository, causing 422 errors on quota-checked endpoints. Switch to
underscore separator to match the allowed pattern.

Closes #3350

* fix(home): isolate test user to prevent duplicate key errors

Clean up admin user in afterAll and use a unique email prefix
(admin-home-health) to avoid E11000 collisions when running all
test suites together via test:coverage.

Closes #3348

* fix(analytics): make service error-resilient, fix 5 failing tests

Wrap track/identify/groupIdentify in try/catch so the analytics service
never throws — callers no longer need defensive try/catch wrappers.
Update resilience tests to expect not.toThrow instead of toThrow.

Closes #3340

* fix(docs): align ERRORS.md checklist format with actual ERRORS.md convention

Use `[YYYY-MM-DD] <scope>: <wrong> -> <right>` format instead of
the generic "symptom, root cause, fix" description.

* fix(billing): update stale comment to match underscore separator

Author: PierreBrisorgueil

.claude/skills/feature/SKILL.md

@@ -93,7 +93,7 @@ If an action affects another user:
 - [ ] Templates created for each email type
 
 **Error documentation:**
-- [ ] If a non-obvious bug was fixed, document it in `ERRORS.md` (root of repo) with: symptom, root cause, fix
+- [ ] If a non-obvious bug was fixed, add a single-line entry to `ERRORS.md` (root of repo) using format: `[YYYY-MM-DD] <scope>: <wrong> -> <right>` (see existing examples in `ERRORS.md`)
 
 ### 8b. Elegance check
 

lib/services/analytics.js

@@ -37,7 +37,9 @@ const init = async () => {
  */
 const track = (distinctId, event, properties, groups) => {
   if (!client) return;
-  client.capture({ distinctId, event, properties, groups });
+  try {
+    client.capture({ distinctId, event, properties, groups });
+  } catch (_) { /* analytics must never break caller */ }
 };
 
 /**
@@ -48,7 +50,9 @@ const track = (distinctId, event, properties, groups) => {
  */
 const identify = (distinctId, properties) => {
   if (!client) return;
-  client.identify({ distinctId, properties });
+  try {
+    client.identify({ distinctId, properties });
+  } catch (_) { /* analytics must never break caller */ }
 };
 
 /**
@@ -60,7 +64,9 @@ const identify = (distinctId, properties) => {
  */
 const groupIdentify = (groupType, groupKey, properties) => {
   if (!client) return;
-  client.groupIdentify({ groupType, groupKey, properties });
+  try {
+    client.groupIdentify({ groupType, groupKey, properties });
+  } catch (_) { /* analytics must never break caller */ }
 };
 
 /**

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

@@ -41,31 +41,31 @@ describe('Analytics service resilience tests:', () => {
   });
 
   // ─────────────────────────────────────────────────────────────────────
-  // Error propagation — synchronous SDK failures
+  // Error resilience — synchronous SDK failures are swallowed
   // ─────────────────────────────────────────────────────────────────────
-  describe('synchronous SDK error propagation:', () => {
-    test('track should propagate when client.capture throws', () => {
+  describe('synchronous SDK error resilience:', () => {
+    test('track should not throw when client.capture throws', () => {
       mockPostHogInstance.capture.mockImplementation(() => {
         throw new Error('Capture error');
       });
 
-      expect(() => AnalyticsService.track('u1', 'evt')).toThrow('Capture error');
+      expect(() => AnalyticsService.track('u1', 'evt')).not.toThrow();
     });
 
-    test('identify should propagate when client.identify throws', () => {
+    test('identify should not throw when client.identify throws', () => {
       mockPostHogInstance.identify.mockImplementation(() => {
         throw new Error('Identify error');
       });
 
-      expect(() => AnalyticsService.identify('u1', {})).toThrow('Identify error');
+      expect(() => AnalyticsService.identify('u1', {})).not.toThrow();
     });
 
-    test('groupIdentify should propagate when client.groupIdentify throws', () => {
+    test('groupIdentify should not throw when client.groupIdentify throws', () => {
       mockPostHogInstance.groupIdentify.mockImplementation(() => {
         throw new Error('Group error');
       });
 
-      expect(() => AnalyticsService.groupIdentify('company', 'org', {})).toThrow('Group error');
+      expect(() => AnalyticsService.groupIdentify('company', 'org', {})).not.toThrow();
     });
   });
 

modules/billing/README.md

@@ -41,7 +41,7 @@ Increment counters after successful operations:
 import BillingUsageService from '../billing/services/billing.usage.service.js';
 
 // After creating a document
-await BillingUsageService.increment(organizationId, 'documents.create', 1);
+await BillingUsageService.increment(organizationId, 'documents_create', 1);
 ```
 
 ### Usage Endpoint

modules/billing/controllers/billing.controller.js

@@ -73,7 +73,7 @@ const getUsage = async (req, res) => {
     // Get usage counters (includes month field)
     const usage = await BillingUsageService.get(req.organization._id.toString());
 
-    // Flatten quotas config into { "resource.action": limit } format
+    // Flatten quotas config into { "resource_action": limit } format
     // Normalize Infinity to null for JSON-safe serialization
     const quotas = config.billing?.quotas;
     let limits = {};
@@ -82,7 +82,7 @@ const getUsage = async (req, res) => {
       for (const resource of Object.keys(planQuotas)) {
         for (const action of Object.keys(planQuotas[resource])) {
           const rawLimit = planQuotas[resource][action];
-          limits[`${resource}.${action}`] = Number.isFinite(rawLimit) ? rawLimit : null;
+          limits[`${resource}_${action}`] = Number.isFinite(rawLimit) ? rawLimit : null;
         }
       }
     }

modules/billing/middlewares/billing.requireQuota.js

@@ -54,7 +54,7 @@ function requireQuota(resource, action) {
 
       // Check current usage
       const usage = await BillingUsageService.get(req.organization._id.toString());
-      const counterKey = `${resource}.${action}`;
+      const counterKey = `${resource}_${action}`;
       const current = usage.counters[counterKey] || 0;
 
       if (current >= limit) {

modules/billing/tests/billing.quota.unit.tests.js

@@ -69,7 +69,7 @@ describe('requireQuota middleware:', () => {
 
   test('should allow request when under quota', async () => {
     mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
-    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 1 } });
+    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 1 } });
 
     await requireQuota('scraps', 'create')(req, res, next);
 
@@ -79,7 +79,7 @@ describe('requireQuota middleware:', () => {
 
   test('should return 429 when at quota limit', async () => {
     mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
-    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } });
+    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } });
 
     await requireQuota('scraps', 'create')(req, res, next);
 
@@ -95,7 +95,7 @@ describe('requireQuota middleware:', () => {
 
   test('should return 429 when over quota limit', async () => {
     mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
-    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 5 } });
+    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 5 } });
 
     await requireQuota('scraps', 'create')(req, res, next);
 
@@ -105,7 +105,7 @@ describe('requireQuota middleware:', () => {
 
   test('should treat missing subscription as free plan', async () => {
     mockSubscriptionRepository.findByOrganization.mockResolvedValue(null);
-    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } });
+    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } });
 
     await requireQuota('scraps', 'create')(req, res, next);
 
@@ -121,7 +121,7 @@ describe('requireQuota middleware:', () => {
     'should treat %s subscription as free plan',
     async (status) => {
       mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status });
-      mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } });
+      mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } });
 
       await requireQuota('scraps', 'create')(req, res, next);
 
@@ -145,7 +145,7 @@ describe('requireQuota middleware:', () => {
 
   test('should return correct error payload with upgradeUrl', async () => {
     mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
-    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.execute': 100 } });
+    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_execute': 100 } });
 
     await requireQuota('scraps', 'execute')(req, res, next);
 
@@ -178,7 +178,7 @@ describe('requireQuota middleware:', () => {
 
   test('should use subscription plan when status is trialing', async () => {
     mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'trialing' });
-    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 15 } });
+    mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 15 } });
 
     await requireQuota('scraps', 'create')(req, res, next);
 

modules/billing/tests/billing.usage.endpoint.unit.tests.js

@@ -63,7 +63,7 @@ describe('Billing usage endpoint unit tests:', () => {
 
   test('should return usage and limits for active subscription', async () => {
     mockBillingService.getSubscription.mockResolvedValue({ plan: 'starter', status: 'active' });
-    mockBillingUsageService.get.mockResolvedValue({ month: '2026-03', counters: { 'documents.create': 5, 'requests.execute': 42 } });
+    mockBillingUsageService.get.mockResolvedValue({ month: '2026-03', counters: { 'documents_create': 5, 'requests_execute': 42 } });
 
     const req = { organization: { _id: orgId } };
     await billingController.getUsage(req, res);
@@ -74,8 +74,8 @@ describe('Billing usage endpoint unit tests:', () => {
       message: 'billing usage',
       data: expect.objectContaining({
         plan: 'starter',
-        usage: { 'documents.create': 5, 'requests.execute': 42 },
-        limits: { 'documents.create': 20, 'requests.execute': 2000 },
+        usage: { 'documents_create': 5, 'requests_execute': 42 },
+        limits: { 'documents_create': 20, 'requests_execute': 2000 },
       }),
     }));
   });
@@ -92,7 +92,7 @@ describe('Billing usage endpoint unit tests:', () => {
       type: 'success',
       data: expect.objectContaining({
         plan: 'free',
-        limits: { 'documents.create': 5, 'requests.execute': 100 },
+        limits: { 'documents_create': 5, 'requests_execute': 100 },
       }),
     }));
   });
@@ -106,7 +106,7 @@ describe('Billing usage endpoint unit tests:', () => {
     'paused',
   ])('should return free plan when subscription status is %s', async (status) => {
     mockBillingService.getSubscription.mockResolvedValue({ plan: 'starter', status });
-    mockBillingUsageService.get.mockResolvedValue({ month: '2026-03', counters: { 'documents.create': 2 } });
+    mockBillingUsageService.get.mockResolvedValue({ month: '2026-03', counters: { 'documents_create': 2 } });
 
     const req = { organization: { _id: orgId } };
     await billingController.getUsage(req, res);
@@ -115,7 +115,7 @@ describe('Billing usage endpoint unit tests:', () => {
     expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
       data: expect.objectContaining({
         plan: 'free',
-        limits: { 'documents.create': 5, 'requests.execute': 100 },
+        limits: { 'documents_create': 5, 'requests_execute': 100 },
       }),
     }));
   });
@@ -131,7 +131,7 @@ describe('Billing usage endpoint unit tests:', () => {
     expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
       data: expect.objectContaining({
         plan: 'starter',
-        limits: { 'documents.create': 20, 'requests.execute': 2000 },
+        limits: { 'documents_create': 20, 'requests_execute': 2000 },
       }),
     }));
   });
@@ -163,7 +163,7 @@ describe('Billing usage endpoint unit tests:', () => {
     expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
       data: expect.objectContaining({
         plan: 'pro',
-        limits: { 'documents.create': null, 'requests.execute': null },
+        limits: { 'documents_create': null, 'requests_execute': null },
       }),
     }));
   });

modules/home/tests/home.integration.tests.js

@@ -18,6 +18,7 @@ describe('Home integration tests:', () => {
   let agent;
   let HomeService;
   let adminToken;
+  let adminUser;
   let originalOrganizationsEnabled;
 
   //  init
@@ -41,15 +42,16 @@ describe('Home integration tests:', () => {
 
       // Create admin user and sign JWT for health endpoint test
       const User = mongoose.model('User');
-      const admin = await User.create({
+      await User.deleteOne({ email: 'admin-home-health@test.com' });
+      adminUser = await User.create({
         firstName: 'Admin',
         lastName: 'Health',
-        email: 'admin-health@test.com',
+        email: 'admin-home-health@test.com',
         password: 'W@os.jsI$Aw3$0m3',
         provider: 'local',
         roles: ['admin'],
       });
-      adminToken = jwt.sign({ userId: admin.id }, config.jwt.secret, { expiresIn: config.jwt.expiresIn });
+      adminToken = jwt.sign({ userId: adminUser.id }, config.jwt.secret, { expiresIn: config.jwt.expiresIn });
     } catch (err) {
       console.log(err);
       expect(err).toBeFalsy();
@@ -204,10 +206,16 @@ describe('Home integration tests:', () => {
     });
   });
 
-  // Mongoose disconnect
+  // Cleanup and mongoose disconnect
   afterAll(async () => {
     jest.restoreAllMocks();
     config.organizations.enabled = originalOrganizationsEnabled;
+    try {
+      if (adminUser) {
+        const User = mongoose.model('User');
+        await User.deleteOne({ _id: adminUser._id });
+      }
+    } catch (_) { /* cleanup – ignore errors */ }
     try {
       await mongooseService.disconnect();
     } catch (err) {