fix: bound diagnostics provider health checks

.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.

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,

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<string, DiagnosticsProvider>();
+  private readonly providers = new Map<string, RegisteredDiagnosticsProvider>();
   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<HealthStatus> {
+    const timeoutMs = options.timeout ?? this.timeout;
+    const controller = new AbortController();
+    let timeoutId: ReturnType<typeof setTimeout> | undefined;
+
+    const timeoutPromise = new Promise<HealthStatus>((_, 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);
+      }
+    }
+  }
 }

packages/diagnostics-core/src/libs/types.ts

@@ -1,8 +1,16 @@
 export interface DiagnosticsProvider {
   readonly name: string;
-  getHealth(): Promise<HealthStatus>;
+  getHealth(signal?: AbortSignal): Promise<HealthStatus>;
 }
 
+export type DiagnosticsCollectorOptions = {
+  readonly timeout?: number;
+};
+
+export type DiagnosticsProviderOptions = {
+  readonly timeout?: number;
+};
+
 export type HealthStatus = {
   readonly status: "healthy" | "degraded" | "unhealthy";
   readonly component: string;

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<HealthStatus>((_, 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<HealthStatus>(() => {})),
+      };
+
+      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();
+    }
+  });
 });