Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/safe-health-timeouts.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@croco/diagnostics-core": minor
"@croco/health-core": minor
"@croco/problems-core": patch
---

Reject timeout values that Node.js would clamp before health or diagnostics checks are registered.
64 changes: 62 additions & 2 deletions docs/problem-code-registry.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"version": "croco.problem-code-registry.v1",
"problemCount": 569,
"problemCount": 571,
"problems": [
{
"code": "ACCESS_DENIED",
Expand Down Expand Up @@ -4556,7 +4556,37 @@
"sources": [
{
"file": "packages/diagnostics-core/src/libs/problems/DiagnosticsProblems.ts",
"line": 7,
"line": 25,
"column": 3,
"kind": "problem-class"
}
]
},
{
"code": "diagnostics-core/invalid-timeout",
"category": "ValidationError",
"status": 422,
"title": "Validation Error",
"cookbookPath": "/reference/problem-recovery-cookbook/#diagnostics-core-invalid-timeout",
"recovery": {
"cause": "The request or generated contract failed schema or semantic validation.",
"userAction": "Fix the invalid fields and retry with schema-conformant input.",
"operatorAction": "Inspect schema diagnostics, generated contracts, and validation metadata.",
"retryability": "not-retryable",
"redactionPolicy": "public",
"telemetry": {
"eventName": "croco.problem.info",
"severity": "info",
"attributes": ["problem.code", "problem.category", "problem.status"]
}
},
"lifecycle": {
"status": "active"
},
"sources": [
{
"file": "packages/diagnostics-core/src/libs/problems/DiagnosticsProblems.ts",
"line": 9,
"column": 3,
"kind": "problem-class"
}
Expand Down Expand Up @@ -6962,6 +6992,36 @@
}
]
},
{
"code": "health-core/invalid-timeout",
"category": "ValidationError",
"status": 422,
"title": "Validation Error",
"cookbookPath": "/reference/problem-recovery-cookbook/#health-core-invalid-timeout",
"recovery": {
"cause": "The request or generated contract failed schema or semantic validation.",
"userAction": "Fix the invalid fields and retry with schema-conformant input.",
"operatorAction": "Inspect schema diagnostics, generated contracts, and validation metadata.",
"retryability": "not-retryable",
"redactionPolicy": "public",
"telemetry": {
"eventName": "croco.problem.info",
"severity": "info",
"attributes": ["problem.code", "problem.category", "problem.status"]
}
},
"lifecycle": {
"status": "active"
},
"sources": [
{
"file": "packages/health-core/src/libs/problems/HealthProblems.ts",
"line": 9,
"column": 3,
"kind": "problem-class"
}
]
},
{
"code": "idempotency-core/invalid-key",
"category": "BadRequest",
Expand Down
5 changes: 5 additions & 0 deletions packages/diagnostics-core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ reported without ad hoc strings.
codes.
- `DuplicateDiagnosticsProviderProblem` - Problem emitted for duplicate provider
registration.
- `InvalidDiagnosticsTimeoutProblem` - Problem emitted when a default or per-provider
timeout is outside the safe Node.js timer range.

Default and per-provider timeouts must be integer milliseconds between `1` and
`2_147_483_647`. Invalid values fail during setup before a provider check runs.

## Usage

Expand Down
7 changes: 6 additions & 1 deletion packages/diagnostics-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ export {
isDiagnosticCode,
} from "./libs/DiagnosticCodes";
export { ErrorHistoryRingBuffer } from "./libs/ErrorHistoryRingBuffer";
export { DuplicateDiagnosticsProviderProblem } from "./libs/problems/DiagnosticsProblems";
export {
DuplicateDiagnosticsProviderProblem,
InvalidDiagnosticsTimeoutProblem,
MAX_DIAGNOSTICS_TIMEOUT_MS,
} from "./libs/problems/DiagnosticsProblems";
export type {
CreateDiagnosticMessageOptions,
DiagnosticCategory,
Expand All @@ -31,3 +35,4 @@ export type {
ErrorRecord,
DiagnosticsReport,
} from "./libs/types";
export type { DiagnosticsTimeoutSource } from "./libs/problems/DiagnosticsProblems";
28 changes: 22 additions & 6 deletions packages/diagnostics-core/src/libs/DiagnosticsCollector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,23 @@ import type {
HealthStatus,
} from "./types";
import { ErrorHistoryRingBuffer } from "./ErrorHistoryRingBuffer";
import { DuplicateDiagnosticsProviderProblem } from "./problems/DiagnosticsProblems";
import {
DuplicateDiagnosticsProviderProblem,
InvalidDiagnosticsTimeoutProblem,
MAX_DIAGNOSTICS_TIMEOUT_MS,
} from "./problems/DiagnosticsProblems";

const DEFAULT_PROVIDER_TIMEOUT_MS = 5000;

function assertValidTimeout(timeout: number, source: "default" | "provider"): void {
if (!Number.isSafeInteger(timeout) || timeout <= 0 || timeout > MAX_DIAGNOSTICS_TIMEOUT_MS) {
throw new InvalidDiagnosticsTimeoutProblem(source, timeout);
}
}

type RegisteredDiagnosticsProvider = {
readonly provider: DiagnosticsProvider;
readonly options: DiagnosticsProviderOptions;
readonly timeout?: number;
};

class DiagnosticsProviderTimeoutError extends Error {
Expand Down Expand Up @@ -49,10 +59,16 @@ export class DiagnosticsCollector {
private readonly timeout: number;

constructor(options: DiagnosticsCollectorOptions = {}) {
this.timeout = options.timeout ?? DEFAULT_PROVIDER_TIMEOUT_MS;
const timeout = options.timeout ?? DEFAULT_PROVIDER_TIMEOUT_MS;
assertValidTimeout(timeout, "default");
this.timeout = timeout;
}

registerProvider(provider: DiagnosticsProvider, options: DiagnosticsProviderOptions = {}): void {
const timeout = options.timeout;
if (timeout !== undefined) {
assertValidTimeout(timeout, "provider");
}
const existingProvider = this.providers.get(provider.name);

if (existingProvider !== undefined) {
Expand All @@ -63,7 +79,7 @@ export class DiagnosticsCollector {
throw new DuplicateDiagnosticsProviderProblem(provider.name);
}

this.providers.set(provider.name, { provider, options });
this.providers.set(provider.name, { provider, timeout });
}

getProviders(): readonly DiagnosticsProvider[] {
Expand Down Expand Up @@ -113,9 +129,9 @@ export class DiagnosticsCollector {

private async getProviderHealth({
provider,
options,
timeout: timeoutOverride,
}: RegisteredDiagnosticsProvider): Promise<HealthStatus> {
const timeoutMs = options.timeout ?? this.timeout;
const timeoutMs = timeoutOverride ?? this.timeout;
const controller = new AbortController();
let timeoutId: ReturnType<typeof setTimeout> | undefined;

Expand Down
18 changes: 18 additions & 0 deletions packages/diagnostics-core/src/libs/problems/DiagnosticsProblems.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
import { Problem, ProblemCategory } from "@croco/problems-core";

export const MAX_DIAGNOSTICS_TIMEOUT_MS = 2_147_483_647;

export type DiagnosticsTimeoutSource = "default" | "provider";

/** Diagnostics timeout configuration cannot be represented safely by a Node.js timer. */
export class InvalidDiagnosticsTimeoutProblem extends Problem {
readonly code = "diagnostics-core/invalid-timeout";
readonly category = ProblemCategory.ValidationError;

constructor(source: DiagnosticsTimeoutSource, timeoutMs: number) {
super(
undefined,
undefined,
`Diagnostics ${source} timeout must be an integer between 1 and ${MAX_DIAGNOSTICS_TIMEOUT_MS} milliseconds; received ${timeoutMs}`,
);
}
}

/**
* 같은 이름으로 서로 다른 diagnostics provider가 등록될 때 발생하는 Problem입니다.
*/
Expand Down
8 changes: 8 additions & 0 deletions packages/diagnostics-core/src/libs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,18 @@ export interface DiagnosticsProvider {
}

export type DiagnosticsCollectorOptions = {
/**
* Default provider timeout in milliseconds. Must be an integer from 1 through 2,147,483,647.
* Invalid values throw an InvalidDiagnosticsTimeoutProblem during setup.
*/
readonly timeout?: number;
};

export type DiagnosticsProviderOptions = {
/**
* Provider timeout in milliseconds. Must be an integer from 1 through 2,147,483,647.
* Invalid values throw an InvalidDiagnosticsTimeoutProblem during registration.
*/
readonly timeout?: number;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { DiagnosticsCollector } from "../libs/DiagnosticsCollector";
import { DuplicateDiagnosticsProviderProblem } from "../libs/problems/DiagnosticsProblems";
import {
DuplicateDiagnosticsProviderProblem,
InvalidDiagnosticsTimeoutProblem,
MAX_DIAGNOSTICS_TIMEOUT_MS,
} from "../libs/problems/DiagnosticsProblems";
import type { DiagnosticsProvider, HealthStatus, ErrorRecord } from "../libs/types";

class MockDiagnosticsProvider implements DiagnosticsProvider {
Expand Down Expand Up @@ -46,6 +50,99 @@ describe("DiagnosticsCollector Integration", () => {
collector = new DiagnosticsCollector();
});

describe("timeout configuration", () => {
const invalidTimeouts = [Number.NaN, Number.POSITIVE_INFINITY, -1, 0, 0.5, 1.5, 2_147_483_648];

it.each(invalidTimeouts)("rejects invalid default timeout %s at construction", (timeout) => {
expect(() => new DiagnosticsCollector({ timeout })).toThrow(InvalidDiagnosticsTimeoutProblem);

try {
new DiagnosticsCollector({ timeout });
} catch (error) {
expect(error).toMatchObject({
code: "diagnostics-core/invalid-timeout",
message: expect.stringContaining(String(timeout)),
});
}
});

it.each(invalidTimeouts)("rejects invalid provider timeout %s at registration", (timeout) => {
const provider = new MockDiagnosticsProvider("db", {
status: "healthy",
component: "db",
lastChecked: new Date().toISOString(),
});

expect(() => collector.registerProvider(provider, { timeout })).toThrow(
InvalidDiagnosticsTimeoutProblem,
);
expect(collector.getProviders()).toEqual([]);
});

it.each([1, MAX_DIAGNOSTICS_TIMEOUT_MS])(
"preserves valid default timeout boundary %s",
async (timeout) => {
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
const boundaryCollector = new DiagnosticsCollector({ timeout });
boundaryCollector.registerProvider(
new MockDiagnosticsProvider("db", {
status: "healthy",
component: "db",
lastChecked: new Date().toISOString(),
}),
);

await boundaryCollector.getReport();

expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), timeout);
setTimeoutSpy.mockRestore();
},
);

it.each([1, MAX_DIAGNOSTICS_TIMEOUT_MS])(
"preserves valid provider timeout boundary %s",
async (timeout) => {
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
collector.registerProvider(
new MockDiagnosticsProvider("db", {
status: "healthy",
component: "db",
lastChecked: new Date().toISOString(),
}),
{ timeout },
);

await collector.getReport();

expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), timeout);
setTimeoutSpy.mockRestore();
},
);

it("snapshots a validated provider timeout before the caller mutates its options", async () => {
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
const options = { timeout: 100 };
collector.registerProvider(
new MockDiagnosticsProvider("db", {
status: "healthy",
component: "db",
lastChecked: new Date().toISOString(),
}),
options,
);

options.timeout = Number.POSITIVE_INFINITY;
await collector.getReport();

expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 100);
expect(setTimeoutSpy).not.toHaveBeenCalledWith(
expect.any(Function),
Number.POSITIVE_INFINITY,
);
setTimeoutSpy.mockRestore();
});
});

it("should return report with summary when no providers registered", async () => {
const report = await collector.getReport();

Expand Down
Loading