From c732e36d72bb4212cc659fee3e557d2c9f8eb0be Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Mon, 15 Jun 2026 07:11:29 +0900 Subject: [PATCH] fix: bound diagnostics provider health checks --- .changeset/diagnostics-provider-timeouts.md | 5 ++ packages/diagnostics-core/src/index.ts | 2 + .../src/libs/DiagnosticsCollector.ts | 66 ++++++++++++-- packages/diagnostics-core/src/libs/types.ts | 10 ++- .../src/tests/DiagnosticsIntegration.spec.ts | 89 ++++++++++++++++++- 5 files changed, 163 insertions(+), 9 deletions(-) create mode 100644 .changeset/diagnostics-provider-timeouts.md diff --git a/.changeset/diagnostics-provider-timeouts.md b/.changeset/diagnostics-provider-timeouts.md new file mode 100644 index 000000000..d20511649 --- /dev/null +++ b/.changeset/diagnostics-provider-timeouts.md @@ -0,0 +1,5 @@ +--- +"@croco/diagnostics-core": patch +--- + +Keep diagnostics reports bounded by timing out hanging providers and marking timed-out components as degraded. diff --git a/packages/diagnostics-core/src/index.ts b/packages/diagnostics-core/src/index.ts index 41ead2c32..f86123337 100644 --- a/packages/diagnostics-core/src/index.ts +++ b/packages/diagnostics-core/src/index.ts @@ -4,7 +4,9 @@ export { DiagnosticsCollector } from "./libs/DiagnosticsCollector"; export { ErrorHistoryRingBuffer } from "./libs/ErrorHistoryRingBuffer"; export { DuplicateDiagnosticsProviderProblem } from "./libs/problems/DiagnosticsProblems"; export type { + DiagnosticsCollectorOptions, DiagnosticsProvider, + DiagnosticsProviderOptions, HealthStatus, ErrorRecord, DiagnosticsReport, diff --git a/packages/diagnostics-core/src/libs/DiagnosticsCollector.ts b/packages/diagnostics-core/src/libs/DiagnosticsCollector.ts index 33f3335e7..010d90ea6 100644 --- a/packages/diagnostics-core/src/libs/DiagnosticsCollector.ts +++ b/packages/diagnostics-core/src/libs/DiagnosticsCollector.ts @@ -1,7 +1,28 @@ -import type { DiagnosticsProvider, HealthStatus, ErrorRecord, DiagnosticsReport } from "./types"; +import type { + DiagnosticsCollectorOptions, + DiagnosticsProvider, + DiagnosticsProviderOptions, + DiagnosticsReport, + ErrorRecord, + HealthStatus, +} from "./types"; import { ErrorHistoryRingBuffer } from "./ErrorHistoryRingBuffer"; import { DuplicateDiagnosticsProviderProblem } from "./problems/DiagnosticsProblems"; +const DEFAULT_PROVIDER_TIMEOUT_MS = 5000; + +type RegisteredDiagnosticsProvider = { + readonly provider: DiagnosticsProvider; + readonly options: DiagnosticsProviderOptions; +}; + +class DiagnosticsProviderTimeoutError extends Error { + constructor(providerName: string, timeoutMs: number) { + super(`Provider health check timed out after ${timeoutMs}ms for ${providerName}`); + this.name = "DiagnosticsProviderTimeoutError"; + } +} + function capMessage(message: string, maxLength: number): string { if (message.length <= maxLength) { return message; @@ -23,25 +44,30 @@ function computeSummary(statuses: readonly HealthStatus[]): DiagnosticsReport["s } export class DiagnosticsCollector { - private readonly providers = new Map(); + private readonly providers = new Map(); private readonly errors = new ErrorHistoryRingBuffer(); + private readonly timeout: number; + + constructor(options: DiagnosticsCollectorOptions = {}) { + this.timeout = options.timeout ?? DEFAULT_PROVIDER_TIMEOUT_MS; + } - registerProvider(provider: DiagnosticsProvider): void { + registerProvider(provider: DiagnosticsProvider, options: DiagnosticsProviderOptions = {}): void { const existingProvider = this.providers.get(provider.name); if (existingProvider !== undefined) { - if (existingProvider === provider) { + if (existingProvider.provider === provider) { return; } throw new DuplicateDiagnosticsProviderProblem(provider.name); } - this.providers.set(provider.name, provider); + this.providers.set(provider.name, { provider, options }); } getProviders(): readonly DiagnosticsProvider[] { - return Array.from(this.providers.values()); + return Array.from(this.providers.values(), ({ provider }) => provider); } recordError(error: ErrorRecord): void { @@ -52,7 +78,9 @@ export class DiagnosticsCollector { const providerEntries = Array.from(this.providers.entries()); const settled = await Promise.allSettled( - providerEntries.map(async ([, provider]) => provider.getHealth()), + providerEntries.map(async ([, registeredProvider]) => + this.getProviderHealth(registeredProvider), + ), ); const components: HealthStatus[] = settled.map((result, index) => { @@ -82,4 +110,28 @@ export class DiagnosticsCollector { recentErrors: this.errors.getAll(), }; } + + private async getProviderHealth({ + provider, + options, + }: RegisteredDiagnosticsProvider): Promise { + const timeoutMs = options.timeout ?? this.timeout; + const controller = new AbortController(); + let timeoutId: ReturnType | undefined; + + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout(() => { + reject(new DiagnosticsProviderTimeoutError(provider.name, timeoutMs)); + controller.abort(); + }, timeoutMs); + }); + + try { + return await Promise.race([provider.getHealth(controller.signal), timeoutPromise]); + } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } + } + } } diff --git a/packages/diagnostics-core/src/libs/types.ts b/packages/diagnostics-core/src/libs/types.ts index da4b80108..bd04aab74 100644 --- a/packages/diagnostics-core/src/libs/types.ts +++ b/packages/diagnostics-core/src/libs/types.ts @@ -1,8 +1,16 @@ export interface DiagnosticsProvider { readonly name: string; - getHealth(): Promise; + getHealth(signal?: AbortSignal): Promise; } +export type DiagnosticsCollectorOptions = { + readonly timeout?: number; +}; + +export type DiagnosticsProviderOptions = { + readonly timeout?: number; +}; + export type HealthStatus = { readonly status: "healthy" | "degraded" | "unhealthy"; readonly component: string; diff --git a/packages/diagnostics-core/src/tests/DiagnosticsIntegration.spec.ts b/packages/diagnostics-core/src/tests/DiagnosticsIntegration.spec.ts index 746482a75..d295d9f29 100644 --- a/packages/diagnostics-core/src/tests/DiagnosticsIntegration.spec.ts +++ b/packages/diagnostics-core/src/tests/DiagnosticsIntegration.spec.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { DiagnosticsCollector } from "../libs/DiagnosticsCollector"; import { DuplicateDiagnosticsProviderProblem } from "../libs/problems/DiagnosticsProblems"; import type { DiagnosticsProvider, HealthStatus, ErrorRecord } from "../libs/types"; @@ -190,4 +190,91 @@ describe("DiagnosticsCollector Integration", () => { expect(report.components[0].message).toBe("Provider health check failed"); expect(report.summary).toBe("degraded"); }); + + it("should return degraded component when provider health check times out", async () => { + vi.useFakeTimers(); + + try { + let didAbort = false; + const collectorWithTimeout = new DiagnosticsCollector({ timeout: 100 }); + const fastProvider = new MockDiagnosticsProvider("fast-service", { + status: "healthy", + component: "fast-service", + lastChecked: new Date().toISOString(), + }); + const hangingProvider: DiagnosticsProvider = { + name: "stuck-service", + getHealth: vi.fn( + (signal?: AbortSignal) => + new Promise((_, reject) => { + signal?.addEventListener("abort", () => { + didAbort = true; + reject(new Error("provider observed abort")); + }); + }), + ), + }; + + collectorWithTimeout.registerProvider(fastProvider); + collectorWithTimeout.registerProvider(hangingProvider); + + const reportPromise = collectorWithTimeout.getReport(); + + await vi.advanceTimersByTimeAsync(100); + + const report = await reportPromise; + + expect(report.components).toHaveLength(2); + expect(report.components[0].status).toBe("healthy"); + expect(report.components[1].status).toBe("degraded"); + expect(report.components[1].component).toBe("stuck-service"); + expect(report.components[1].message).toBe( + "Provider health check timed out after 100ms for stuck-service", + ); + expect(report.summary).toBe("degraded"); + expect(didAbort).toBe(true); + expect(hangingProvider.getHealth).toHaveBeenCalledWith(expect.any(AbortSignal)); + } finally { + vi.useRealTimers(); + } + }); + + it("should honor per-provider timeout overrides", async () => { + vi.useFakeTimers(); + + try { + const collectorWithTimeout = new DiagnosticsCollector({ timeout: 5000 }); + const hangingProvider: DiagnosticsProvider = { + name: "slow-provider", + getHealth: vi.fn((_signal?: AbortSignal) => new Promise(() => {})), + }; + + collectorWithTimeout.registerProvider(hangingProvider, { timeout: 75 }); + + let settled = false; + const reportPromise = collectorWithTimeout.getReport().then((report) => { + settled = true; + return report; + }); + + await vi.advanceTimersByTimeAsync(74); + await Promise.resolve(); + + expect(settled).toBe(false); + + await vi.advanceTimersByTimeAsync(1); + + const report = await reportPromise; + + expect(settled).toBe(true); + expect(report.components).toHaveLength(1); + expect(report.components[0]).toMatchObject({ + status: "degraded", + component: "slow-provider", + message: "Provider health check timed out after 75ms for slow-provider", + }); + } finally { + vi.useRealTimers(); + } + }); });