From f71fd5ebd65279209e2d51b265ba51869194542c Mon Sep 17 00:00:00 2001 From: whyour Date: Mon, 17 Aug 2026 08:55:53 +0800 Subject: [PATCH] fix: propagate initialization errors and reduce request metrics --- back/api/user.ts | 8 +-- back/middlewares/monitoring.ts | 66 ++++++----------------- test/back/monitoring.test.cjs | 73 +++++++++++++++++++++++++ test/back/user-api.test.cjs | 99 ++++++++++++++++++++++++++++++++++ 4 files changed, 193 insertions(+), 53 deletions(-) create mode 100644 test/back/monitoring.test.cjs create mode 100644 test/back/user-api.test.cjs diff --git a/back/api/user.ts b/back/api/user.ts index c1a18146..4d526389 100644 --- a/back/api/user.ts +++ b/back/api/user.ts @@ -80,8 +80,8 @@ export default (app: Router) => { return res.send({ code: 450, message: t('未知错误') }); } const userService = Container.get(UserService); - await userService.updateUsernameAndPassword(req.body); - res.send({ code: 200, message: t('更新成功') }); + const result = await userService.updateUsernameAndPassword(req.body); + res.send(result); } catch (e) { return next(e); } @@ -282,8 +282,8 @@ export default (app: Router) => { const logger: Logger = Container.get('logger'); try { const userService = Container.get(UserService); - await userService.updateUsernameAndPassword(req.body); - res.send({ code: 200, message: t('更新成功') }); + const result = await userService.updateUsernameAndPassword(req.body); + res.send(result); } catch (e) { return next(e); } diff --git a/back/middlewares/monitoring.ts b/back/middlewares/monitoring.ts index d0d3f2f4..ab9b6c01 100644 --- a/back/middlewares/monitoring.ts +++ b/back/middlewares/monitoring.ts @@ -3,46 +3,34 @@ import Logger from '../loaders/logger'; import { performance } from 'perf_hooks'; import { metricsService } from '../services/metrics'; -interface RequestMetrics { - method: string; - path: string; - duration: number; - statusCode: number; - timestamp: number; - platform?: string; -} - -const requestMetrics: RequestMetrics[] = []; +const UNMONITORED_PATH_SUFFIXES = ['/api/health', '/open/health']; +const HTTP_METRIC_SAMPLE_INTERVAL = 10; +let requestSampleOffset = 0; export const monitoringMiddleware = ( req: Request, res: Response, next: NextFunction, ) => { + if (UNMONITORED_PATH_SUFFIXES.some((path) => req.path.endsWith(path))) { + return next(); + } + const start = performance.now(); const originalEnd = res.end; res.end = function (chunk?: any, encoding?: any, cb?: any) { const duration = performance.now() - start; - const metric: RequestMetrics = { - method: req.method, - path: req.path, - duration, - statusCode: res.statusCode, - timestamp: Date.now(), - platform: req.platform, - }; - - requestMetrics.push(metric); - metricsService.record('http_request', duration, { - method: req.method, - path: req.path, - statusCode: res.statusCode.toString(), - ...(req.platform && { platform: req.platform }), - }); - - if (requestMetrics.length > 1000) { - requestMetrics.shift(); + const shouldSample = requestSampleOffset === 0; + requestSampleOffset = + (requestSampleOffset + 1) % HTTP_METRIC_SAMPLE_INTERVAL; + if (shouldSample) { + metricsService.record('http_request', duration, { + method: req.method, + path: req.path, + statusCode: res.statusCode.toString(), + ...(req.platform && { platform: req.platform }), + }); } if (duration > 1000) { @@ -58,23 +46,3 @@ export const monitoringMiddleware = ( next(); }; - -export const getMetrics = () => { - return { - totalRequests: requestMetrics.length, - averageDuration: - requestMetrics.reduce((acc, curr) => acc + curr.duration, 0) / - requestMetrics.length, - requestsByMethod: requestMetrics.reduce((acc, curr) => { - acc[curr.method] = (acc[curr.method] || 0) + 1; - return acc; - }, {} as Record), - requestsByPlatform: requestMetrics.reduce((acc, curr) => { - if (curr.platform) { - acc[curr.platform] = (acc[curr.platform] || 0) + 1; - } - return acc; - }, {} as Record), - recentRequests: requestMetrics.slice(-10), - }; -}; diff --git a/test/back/monitoring.test.cjs b/test/back/monitoring.test.cjs new file mode 100644 index 00000000..eec34833 --- /dev/null +++ b/test/back/monitoring.test.cjs @@ -0,0 +1,73 @@ +const assert = require('node:assert/strict'); +const test = require('node:test'); + +const { monitoringMiddleware } = require('../../back/middlewares/monitoring'); +const { metricsService } = require('../../back/services/metrics'); + +function createResponse() { + return { + statusCode: 200, + ended: false, + end() { + this.ended = true; + }, + }; +} + +test('health probes bypass request metric retention with or without base URL', () => { + const before = metricsService.getMetrics('http_request', { + path: '/api/health', + }).count; + let nextCalled = false; + + for (const path of ['/api/health', '/ql/api/health']) { + const response = createResponse(); + monitoringMiddleware({ method: 'GET', path }, response, () => { + nextCalled = true; + }); + response.end(); + assert.equal(response.ended, true); + } + + const after = metricsService.getMetrics('http_request', { + path: '/api/health', + }).count; + assert.equal(nextCalled, true); + assert.equal(after, before); +}); + +test('non-health requests keep bounded service metrics', () => { + const path = '/api/monitoring-test'; + const before = metricsService.getMetrics('http_request', { path }).count; + const response = createResponse(); + + monitoringMiddleware( + { method: 'GET', path, platform: 'desktop' }, + response, + () => {}, + ); + response.end(); + + const after = metricsService.getMetrics('http_request', { path }).count; + assert.equal(response.ended, true); + assert.equal(after, before + 1); +}); + +test('ordinary request metrics are sampled instead of retained per request', () => { + const path = '/api/monitoring-sampling-test'; + const before = metricsService.getMetrics('http_request', { path }).count; + + for (let index = 0; index < 30; index += 1) { + const response = createResponse(); + monitoringMiddleware( + { method: 'GET', path, platform: 'desktop' }, + response, + () => {}, + ); + response.end(); + } + + const recorded = + metricsService.getMetrics('http_request', { path }).count - before; + assert.ok(recorded >= 2 && recorded <= 3); +}); diff --git a/test/back/user-api.test.cjs b/test/back/user-api.test.cjs new file mode 100644 index 00000000..42e6a69f --- /dev/null +++ b/test/back/user-api.test.cjs @@ -0,0 +1,99 @@ +const assert = require('node:assert/strict'); +const test = require('node:test'); +const express = require('express'); +const { Container } = require('typedi'); + +test('initialization returns the username and password validation result', async (t) => { + const originalGet = Container.get; + const userServicePath = require.resolve('../../back/services/user'); + const originalUserService = require.cache[userServicePath]; + const i18nPath = require.resolve('../../back/shared/i18n'); + const originalI18n = require.cache[i18nPath]; + const utilPath = require.resolve('../../back/config/util'); + const originalUtil = require.cache[utilPath]; + const authPath = require.resolve('../../back/shared/auth'); + const originalAuth = require.cache[authPath]; + require.cache[userServicePath] = { + id: userServicePath, + filename: userServicePath, + loaded: true, + exports: { __esModule: true, default: class UserService {} }, + children: [], + paths: [], + }; + require.cache[utilPath] = { + id: utilPath, + filename: utilPath, + loaded: true, + exports: { getToken: () => '', isDemoEnv: () => false }, + children: [], + paths: [], + }; + require.cache[authPath] = { + id: authPath, + filename: authPath, + loaded: true, + exports: { isDefaultAuthInfo: () => false }, + children: [], + paths: [], + }; + require.cache[i18nPath] = { + id: i18nPath, + filename: i18nPath, + loaded: true, + exports: { t: (message) => message }, + children: [], + paths: [], + }; + Container.get = () => ({ + updateUsernameAndPassword: async () => ({ + code: 400, + message: 'password rejected', + }), + }); + t.after(() => { + Container.get = originalGet; + if (originalUserService) { + require.cache[userServicePath] = originalUserService; + } else { + delete require.cache[userServicePath]; + } + if (originalI18n) { + require.cache[i18nPath] = originalI18n; + } else { + delete require.cache[i18nPath]; + } + if (originalUtil) { + require.cache[utilPath] = originalUtil; + } else { + delete require.cache[utilPath]; + } + if (originalAuth) { + require.cache[authPath] = originalAuth; + } else { + delete require.cache[authPath]; + } + }); + + const app = express.Router(); + require('../../back/api/user').default(app); + const userRouter = app.stack.find((layer) => layer.name === 'router').handle; + const initRoute = userRouter.stack.find( + (layer) => layer.route?.path === '/init', + ); + const handler = initRoute.route.stack.at(-1).handle; + let responseBody; + + await handler( + { body: { username: 'admin', password: 'admin' } }, + { send: (body) => (responseBody = body) }, + (error) => { + throw error; + }, + ); + + assert.deepEqual(responseBody, { + code: 400, + message: 'password rejected', + }); +});