Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-11-24 20:06:55 +01:00
Code Issues Projects Releases Wiki Activity

fix: invalid metrics should not crash (#11010)

Browse Source 
## About the changes
Properly awaits all submitted promises, preventing the node's main
process from seeing rejected & unawaited promises.

What's going on?
- The bulk metrics handler pushes `registerBackendClient` promises into
promises.
- The next step (`clientMetricsEnvBulkSchema.validateAsync`) throws for
invalid metrics (e.g., `appName: null`), so we jump to catch and return
400.
- Because the code never reaches `Promise.all(...)`, the previously
spawned promises are never awaited. Node later detects the rejected
`registerBackendClient` promise as an **unhandled rejection** and
crashes the process. If that promise hadn’t been rejected, there’d be no
crash, but with invalid input, it does reject.
- **Fix:** always await the spawned tasks (using `Promise.allSettled`)
so every rejection is observed, even when validation later throws.
This commit is contained in:
Gastón Fournier 2025-11-21 10:27:00 +01:00 committed by GitHub
parent 3bca150cd8
commit e977689571
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 26 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/lib/features/metrics/instance/metrics.ts
View File

@ -239,8 +239,8 @@ export default class ClientMetricsController extends Controller {
} else {
const { body, ip: clientIp } = req;
const { metrics, applications, impactMetrics } = body;
const promises: Promise<void>[] = [];
try {
const promises: Promise<void>[] = [];
for (const app of applications) {
if (
app.sdkType === 'frontend' &&
@ -287,10 +287,32 @@ export default class ClientMetricsController extends Controller {
);
}
await Promise.all(promises);
res.status(202).end();
const results = await Promise.allSettled(promises);
const rejected = results.filter(
(result): result is PromiseRejectedResult =>
result.status === 'rejected',
);
if (rejected.length) {
this.logger.warn(
'Some bulkMetrics tasks failed',
rejected.map((r) => r.reason?.message || r.reason),
);
res.status(400).end();
} else {
res.status(202).end();
}
} catch (e) {
const results = await Promise.allSettled(promises);
const rejected = results.filter(
(result): result is PromiseRejectedResult =>
result.status === 'rejected',
);
if (rejected.length) {
this.logger.warn(
'Some bulkMetrics tasks failed',
rejected.map((r) => r.reason?.message || r.reason),
);
}
res.status(400).end();
}
}