From c6ac74991ea00873c818d69189729109bb96b9eb Mon Sep 17 00:00:00 2001 From: Andy Pai Date: Tue, 23 Jun 2026 13:13:30 -0400 Subject: [PATCH] =?UTF-8?q?refactor(serve):=20simplify=20parsePositiveInt?= =?UTF-8?q?=20=E2=80=94=20drop=20type-blind=20catch=20+=20redundant=20trim?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings F4/F5 (no behavior change). parsePositiveInt routed a pre-trimmed string through parseBoundedInt inside a try/catch, then re-framed ANY thrown error as "must be a positive integer". Two smells: - F4: the bare `catch {}` was type-blind — a future non-validation throw from parseBoundedInt would be silently relabeled, hiding the real fault. - F5: parseBoundedInt re-trimmed the already-trimmed string. Resolve directly through the shared parseDecimalDigits primitive (trims once, caps at MAX_SAFE_INTEGER) and range-check `>= 1` inline. No exception is used for control flow, so there's no catch to mask anything, and the value is trimmed without a round-trip. Error code/message and all accept/reject behavior are identical (parseDecimalDigits already returns null past MAX_SAFE_INTEGER, so the old upper-bound check is preserved). Suite 499 pass / 0 fail, check clean. Claude-Session: https://claude.ai/code/session_01LQjR5poSV2M3nxt8h5hNad --- src/transport-input.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/transport-input.ts b/src/transport-input.ts index dab0a33..0bc752c 100644 --- a/src/transport-input.ts +++ b/src/transport-input.ts @@ -39,23 +39,24 @@ export function parseBoundedInt( /** * Parse an optional positive integer supplied through a transport boundary. - * Returns `undefined` when the value is absent, otherwise delegates to - * {@link parseBoundedInt} (min 1) and re-frames the error as "positive integer" - * so invalid limits don't leak into provider logic (SQL `LIMIT`, `slice`, PG). + * Returns `undefined` when the value is absent/blank, otherwise resolves via + * {@link parseDecimalDigits} (which trims and caps at `MAX_SAFE_INTEGER`) and + * enforces `>= 1`, throwing `KanbanError(INVALID_ARGUMENT)` framed as "positive + * integer" so invalid limits don't leak into provider logic (SQL `LIMIT`, + * `slice`, PG). */ export function parsePositiveInt( value: string | null | undefined, field = 'limit', ): number | undefined { if (value === null || value === undefined) return undefined - const trimmed = value.trim() - if (trimmed === '') return undefined - try { - return parseBoundedInt(trimmed, { min: 1, field }) - } catch { + if (value.trim() === '') return undefined + const parsed = parseDecimalDigits(value) + if (parsed === null || parsed < 1) { throw new KanbanError( ErrorCode.INVALID_ARGUMENT, `${field} must be a positive integer (received '${value}')`, ) } + return parsed }