From 18ae5ea18c318286ff482d53d13434aab9f5a728 Mon Sep 17 00:00:00 2001 From: Pavel Fadeev Date: Sun, 16 Aug 2026 02:20:20 +0200 Subject: [PATCH 1/2] fix(transport): bound retries and validate upstream responses --- src/api/client.ts | 96 ++++++++---- src/api/metainfo.ts | 122 +++++++++++---- src/api/search.ts | 81 ++++++---- src/api/transport.ts | 141 +++++++++++++++++ src/api/types.ts | 2 +- src/cli.ts | 19 +-- src/config.ts | 47 ++++++ src/index.ts | 20 +-- src/tests/cli.test.ts | 13 +- src/tests/transport.test.ts | 304 ++++++++++++++++++++++++++++++++++++ 10 files changed, 738 insertions(+), 107 deletions(-) create mode 100644 src/api/transport.ts create mode 100644 src/config.ts create mode 100644 src/tests/transport.test.ts diff --git a/src/api/client.ts b/src/api/client.ts index afb98f2..8c58c0e 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -2,49 +2,91 @@ * TradingView API Client */ -import fetch from "node-fetch"; import { createRequire } from "module"; import type { ScreenerRequest, ScreenerResponse } from "./types.js"; +import { + requestJson, + type TransportOptions, + UpstreamError, +} from "./transport.js"; const require = createRequire(import.meta.url); const pkg = require("../../package.json"); const API_BASE = "https://scanner.tradingview.com"; -const API_TIMEOUT = 10000; // 10 seconds + +function isScreenerCell(value: unknown): value is number | string | boolean | null { + return value === null || typeof value === "number" || typeof value === "string" || typeof value === "boolean"; +} + +function isScreenerRow( + value: unknown, +): value is ScreenerResponse["data"][number] { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + if (!("s" in value) || !("d" in value)) return false; + return typeof value.s === "string" + && Array.isArray(value.d) + && value.d.every(isScreenerCell); +} + +function isScreenerResponse(value: unknown): value is ScreenerResponse { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + if (!("totalCount" in value) || !("data" in value)) return false; + const totalCount = value.totalCount; + const data = value.data; + return typeof totalCount === "number" + && Number.isInteger(totalCount) + && totalCount >= 0 + && Array.isArray(data) + && data.every(isScreenerRow); +} + +export function validateScreenerResponse( + value: unknown, + endpoint: string, +): ScreenerResponse { + if (!isScreenerResponse(value)) { + throw new Error(`TradingView returned malformed screener response for ${endpoint}`); + } + return value; +} export class TradingViewClient { + constructor(private readonly transportOptions: TransportOptions = {}) {} + private async makeRequest( endpoint: string, - payload: ScreenerRequest + payload: ScreenerRequest, ): Promise { - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), API_TIMEOUT); - try { - const response = await fetch(`${API_BASE}${endpoint}`, { - method: "POST", - headers: { - "Content-Type": "application/json", - "User-Agent": `tradingview-mcp-server/${pkg.version}`, + const data = await requestJson( + `${API_BASE}${endpoint}`, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "User-Agent": `tradingview-mcp-server/${pkg.version}`, + }, + body: JSON.stringify(payload), }, - body: JSON.stringify(payload), - signal: controller.signal, - }); - - clearTimeout(timeout); - - if (!response.ok) { - throw new Error( - `TradingView API error: ${response.status} ${response.statusText}` - ); - } - - return (await response.json()) as ScreenerResponse; + this.transportOptions, + ); + return validateScreenerResponse(data, endpoint); } catch (error) { - clearTimeout(timeout); - if ((error as Error).name === "AbortError") { - throw new Error("Request timeout"); + if (!(error instanceof UpstreamError)) throw error; + if (error.kind === "timeout") { + throw new Error("Request timeout", { cause: error }); + } + if (error.kind === "http") { + throw new Error(`TradingView API error: ${error.status} ${error.statusText ?? ""}`.trimEnd(), { + cause: error, + }); } + if (error.cause instanceof Error) throw error.cause; throw error; } } diff --git a/src/api/metainfo.ts b/src/api/metainfo.ts index cb4b8f0..5e19db4 100644 --- a/src/api/metainfo.ts +++ b/src/api/metainfo.ts @@ -5,14 +5,17 @@ * Endpoint: POST https://scanner.tradingview.com/{market}/metainfo */ -import fetch from "node-fetch"; import { createRequire } from "module"; +import { + requestJson, + type TransportOptions, + UpstreamError, +} from "./transport.js"; const require = createRequire(import.meta.url); const pkg = require("../../package.json"); const API_BASE = "https://scanner.tradingview.com"; -const METAINFO_TIMEOUT = 10000; // 10 seconds export interface MetainfoField { name: string; @@ -36,59 +39,114 @@ export interface MetainfoSummaryResponse { fields: MetainfoField[]; }; } +export interface MetainfoRawResponse { + market: string; + raw: unknown; +} + +type MetainfoPayload = Record | unknown[]; + +function validateMetainfoResponse(value: unknown): MetainfoPayload { + if (Array.isArray(value)) { + if ( + value.length === 0 + || !value.every((field) => + typeof field === "string" + || (typeof field === "object" && field !== null && !Array.isArray(field)) + ) + ) { + throw new Error("TradingView returned malformed metainfo response"); + } + return value; + } + + if (typeof value !== "object" || value === null) { + throw new Error("TradingView returned malformed metainfo response"); + } + + if ("fields" in value || "columns" in value) { + const fields = "fields" in value ? value.fields : value.columns; + if (typeof fields !== "object" || fields === null) { + throw new Error("TradingView returned malformed metainfo response"); + } + } else { + const values = Object.values(value); + if (!values.every((field) => typeof field === "object" && field !== null && !Array.isArray(field))) { + throw new Error("TradingView returned malformed metainfo response"); + } + } + + return value as Record; +} export class MetainfoClient { + constructor(private readonly transportOptions: TransportOptions = {}) {} + /** * Fetch metainfo for a market. */ - async getMetainfo(input: MetainfoInput): Promise { + async getMetainfo( + input: MetainfoInput & { mode: "raw" }, + ): Promise; + async getMetainfo( + input: MetainfoInput & { mode?: "summary" }, + ): Promise; + async getMetainfo(input: MetainfoInput): Promise; + async getMetainfo( + input: MetainfoInput, + ): Promise { const { market, fields, mode = "summary" } = input; if (!market || market.trim().length < 1) { throw new Error("Market is required (e.g., 'america', 'uk', 'germany')"); } - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), METAINFO_TIMEOUT); + const trimmedMarket = market.trim(); + const url = `${API_BASE}/${encodeURIComponent(trimmedMarket)}/metainfo`; try { - const url = `${API_BASE}/${encodeURIComponent(market.trim())}/metainfo`; - - const response = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/json", - "User-Agent": `tradingview-mcp-server/${pkg.version}`, + const data = await requestJson( + url, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "User-Agent": `tradingview-mcp-server/${pkg.version}`, + }, + body: fields ? JSON.stringify({ fields }) : undefined, }, - body: fields ? JSON.stringify({ fields }) : undefined, - signal: controller.signal, - }); - - clearTimeout(timeout); - - if (!response.ok) { - if (response.status === 404) { - throw new Error(`Invalid market: '${market}'. Use markets like 'america', 'uk', 'germany', etc.`); - } - throw new Error(`Metainfo request failed: ${response.status} ${response.statusText}`); - } - - const data = await response.json() as any; + this.transportOptions, + ); if (mode === "raw") { return { - market: market.trim(), + market: trimmedMarket, raw: data, }; } - // Summary mode: normalize the response - return this.normalizeMetainfo(market.trim(), fields, data); + // Summary mode: validate and normalize the response. + return this.normalizeMetainfo( + trimmedMarket, + fields, + validateMetainfoResponse(data), + ); } catch (error) { - clearTimeout(timeout); - if ((error as Error).name === "AbortError") { - throw new Error("Metainfo request timeout"); + if (!(error instanceof UpstreamError)) throw error; + if (error.kind === "http" && error.status === 404) { + throw new Error(`Invalid market: '${market}'. Use markets like 'america', 'uk', 'germany', etc.`, { + cause: error, + }); + } + if (error.kind === "timeout") { + throw new Error("Metainfo request timeout", { cause: error }); + } + if (error.kind === "http") { + throw new Error(`Metainfo request failed: ${error.status} ${error.statusText ?? ""}`.trimEnd(), { + cause: error, + }); } + if (error.cause instanceof Error) throw error.cause; throw error; } } diff --git a/src/api/search.ts b/src/api/search.ts index 98823ed..8654c5f 100644 --- a/src/api/search.ts +++ b/src/api/search.ts @@ -5,14 +5,17 @@ * Endpoint: GET https://symbol-search.tradingview.com/symbol_search/v3 */ -import fetch from "node-fetch"; import { createRequire } from "module"; +import { + requestJson, + type TransportOptions, + UpstreamError, +} from "./transport.js"; const require = createRequire(import.meta.url); const pkg = require("../../package.json"); const SEARCH_BASE = "https://symbol-search.tradingview.com"; -const SEARCH_TIMEOUT = 10000; // 10 seconds export interface SearchSymbolResult { symbol: string; @@ -117,7 +120,35 @@ export function normalizeSearchResults( }; } +function isSearchResult(value: unknown): value is Record { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + const symbol = "symbol" in value ? value.symbol : undefined; + const ticker = "ticker" in value ? value.ticker : undefined; + return typeof symbol === "string" || typeof ticker === "string"; +} + +function validateSearchResponse(value: unknown): Record[] { + const rawResults = Array.isArray(value) + ? value + : typeof value === "object" && value !== null && !Array.isArray(value) + ? "symbols" in value + ? value.symbols + : "results" in value + ? value.results + : undefined + : undefined; + + if (!Array.isArray(rawResults) || !rawResults.every(isSearchResult)) { + throw new Error("TradingView returned malformed symbol search response"); + } + return rawResults; +} + export class SearchClient { + constructor(private readonly transportOptions: TransportOptions = {}) {} + /** * Search for symbols on TradingView. */ @@ -141,31 +172,21 @@ export class SearchClient { const url = `${SEARCH_BASE}/symbol_search/v3/?${params.toString()}`; - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), SEARCH_TIMEOUT); - try { - const response = await fetch(url, { - method: "GET", - headers: { - "User-Agent": `tradingview-mcp-server/${pkg.version}`, - "Origin": "https://www.tradingview.com", - "Referer": "https://www.tradingview.com/", + const data = await requestJson( + url, + { + method: "GET", + headers: { + "User-Agent": `tradingview-mcp-server/${pkg.version}`, + "Origin": "https://www.tradingview.com", + "Referer": "https://www.tradingview.com/", + }, }, - signal: controller.signal, - }); - - clearTimeout(timeout); - - if (!response.ok) { - throw new Error(`Symbol search failed: ${response.status} ${response.statusText}`); - } - - const data = await response.json() as any; + this.transportOptions, + ); - // TradingView symbol search v3 returns an array of results - // Each result has: symbol, type, exchange, description, currency, etc. - const rawResults: any[] = Array.isArray(data) ? data : (data.symbols || data.results || []); + const rawResults = validateSearchResponse(data); const filteredResults = filterSearchResults(rawResults, asset_type); const normalized = normalizeSearchResults(filteredResults, start, clampedLimit); @@ -175,10 +196,16 @@ export class SearchClient { symbols: normalized.symbols, }; } catch (error) { - clearTimeout(timeout); - if ((error as Error).name === "AbortError") { - throw new Error("Symbol search request timeout"); + if (!(error instanceof UpstreamError)) throw error; + if (error.kind === "timeout") { + throw new Error("Symbol search request timeout", { cause: error }); + } + if (error.kind === "http") { + throw new Error(`Symbol search failed: ${error.status} ${error.statusText ?? ""}`.trimEnd(), { + cause: error, + }); } + if (error.cause instanceof Error) throw error.cause; throw error; } } diff --git a/src/api/transport.ts b/src/api/transport.ts new file mode 100644 index 0000000..f45f1b3 --- /dev/null +++ b/src/api/transport.ts @@ -0,0 +1,141 @@ +import fetch, { type RequestInit } from "node-fetch"; +import { setTimeout as delay } from "node:timers/promises"; +import type { RateLimiter } from "../utils/rateLimit.js"; + +export interface FetchResponse { + ok: boolean; + status: number; + statusText: string; + json(): Promise; +} + +export type FetchLike = (url: string, init: RequestInit) => Promise; + +export interface TransportOptions { + fetchImpl?: FetchLike; + timeoutMs?: number; + maxRetries?: number; + retryDelayMs?: number; + rateLimiter?: RateLimiter; +} + +export type UpstreamErrorKind = "timeout" | "http" | "network" | "parse" | "response"; + +export class UpstreamError extends Error { + constructor( + message: string, + public readonly kind: UpstreamErrorKind, + public readonly status?: number, + public readonly retryable = false, + public readonly statusText?: string, + public readonly cause?: unknown, + ) { + super(message, cause === undefined ? undefined : { cause }); + this.name = "UpstreamError"; + } +} + +const DEFAULT_TIMEOUT_MS = 10_000; +const DEFAULT_MAX_RETRIES = 2; +const DEFAULT_RETRY_DELAY_MS = 100; + +function isRetryableStatus(status: number): boolean { + return status === 408 || status === 425 || status === 429 || status >= 500; +} + +function isResponse(value: FetchResponse): boolean { + return typeof value === "object" + && value !== null + && typeof value.ok === "boolean" + && Number.isInteger(value.status) + && typeof value.statusText === "string" + && typeof value.json === "function"; +} + +function asError(value: unknown): Error { + return value instanceof Error ? value : new Error(String(value)); +} + +async function sleep(delayMs: number): Promise { + if (delayMs > 0) await delay(delayMs); +} + +/** + * Execute one JSON request with a bounded retry budget for transient failures. + * Parse and response-shape failures are deliberately not retried. + */ +export async function requestJson( + url: string, + init: RequestInit, + options: TransportOptions = {}, +): Promise { + const fetchImpl = options.fetchImpl ?? fetch; + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + const maxRetries = options.maxRetries ?? DEFAULT_MAX_RETRIES; + const retryDelayMs = options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS; + + for (let attempt = 0; attempt <= maxRetries; attempt += 1) { + let timedOut = false; + const controller = new AbortController(); + const timeout = setTimeout(() => { + timedOut = true; + controller.abort(); + }, timeoutMs); + + try { + const response = await fetchImpl(url, { ...init, signal: controller.signal }); + if (!isResponse(response)) { + throw new UpstreamError("Upstream transport returned a malformed response", "response"); + } + if (!response.ok) { + throw new UpstreamError( + `Upstream request failed with HTTP ${response.status}${response.statusText ? `: ${response.statusText}` : ""}`, + "http", + response.status, + isRetryableStatus(response.status), + response.statusText, + ); + } + + try { + return await response.json(); + } catch (error) { + if (!(error instanceof SyntaxError)) throw error; + throw new UpstreamError( + `Upstream returned invalid JSON: ${asError(error).message}`, + "parse", + undefined, + false, + undefined, + error, + ); + } + } catch (error) { + const normalized = error instanceof UpstreamError + ? error + : timedOut || asError(error).name === "AbortError" + ? new UpstreamError("Upstream request timed out", "timeout", undefined, true, undefined, error) + : new UpstreamError( + `Upstream request failed: ${asError(error).message}`, + "network", + undefined, + true, + undefined, + error, + ); + + if (!normalized.retryable || attempt >= maxRetries) { + throw normalized; + } + + if (options.rateLimiter) { + await options.rateLimiter.acquire(); + } + await sleep(retryDelayMs * 2 ** attempt); + } finally { + clearTimeout(timeout); + } + } + + throw new UpstreamError("Upstream request retry loop exhausted", "network"); +} diff --git a/src/api/types.ts b/src/api/types.ts index 8801869..1dfaef2 100644 --- a/src/api/types.ts +++ b/src/api/types.ts @@ -52,7 +52,7 @@ export interface ScreenerResponse { totalCount: number; data: Array<{ s: string; // symbol - d: (number | string | null)[]; // data array + d: (number | string | boolean | null)[]; // data array }>; } diff --git a/src/cli.ts b/src/cli.ts index 9d5d160..8e06549 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -17,6 +17,7 @@ import { FieldsTool } from "./tools/fields.js"; import { PresetsTool } from "./resources/presets.js"; import { Cache } from "./utils/cache.js"; import { RateLimiter } from "./utils/rateLimit.js"; +import { loadRuntimeConfig } from "./config.js"; import { formatOutput, type OutputFormat } from "./cli/formatters.js"; import { loadPresetFile } from "./cli/presetFile.js"; import { @@ -54,15 +55,15 @@ const require = createRequire(import.meta.url); const pkg = require("../package.json"); // Configuration from environment -const CACHE_TTL = parseInt(process.env.CACHE_TTL_SECONDS || "300"); -const RATE_LIMIT_RPM = parseInt(process.env.RATE_LIMIT_RPM || "10"); - -// Initialize components (no cache.startCleanup — CLI is short-lived) -const client = new TradingViewClient(); -const searchClient = new SearchClient(); -const metainfoClient = new MetainfoClient(); -const cache = new Cache(CACHE_TTL); -const rateLimiter = new RateLimiter(RATE_LIMIT_RPM); +const { cacheTtlSeconds, rateLimitRpm } = loadRuntimeConfig(); + +// Initialize shared infrastructure before clients so retries consume rate-limit tokens. +// No cache.startCleanup — the CLI is short-lived. +const cache = new Cache(cacheTtlSeconds); +const rateLimiter = new RateLimiter(rateLimitRpm); +const client = new TradingViewClient({ rateLimiter }); +const searchClient = new SearchClient({ rateLimiter }); +const metainfoClient = new MetainfoClient({ rateLimiter }); const screenTool = new ScreenTool(client, cache, rateLimiter); const searchTool = new SearchTool(searchClient, cache, rateLimiter); const metainfoTool = new MetainfoTool(metainfoClient, cache, rateLimiter); diff --git a/src/config.ts b/src/config.ts new file mode 100644 index 0000000..7f100f9 --- /dev/null +++ b/src/config.ts @@ -0,0 +1,47 @@ +export interface RuntimeConfig { + cacheTtlSeconds: number; + rateLimitRpm: number; +} + +const DEFAULT_CACHE_TTL_SECONDS = 300; +const DEFAULT_RATE_LIMIT_RPM = 10; + +function parseBoundedInteger( + name: string, + rawValue: string | undefined, + defaultValue: number, + minimum: number, + maximum: number, +): number { + if (rawValue === undefined) return defaultValue; + if (!/^\d+$/.test(rawValue)) { + throw new Error(`${name} must be an integer between ${minimum} and ${maximum}; received '${rawValue}'`); + } + + const value = Number(rawValue); + if (!Number.isSafeInteger(value) || value < minimum || value > maximum) { + throw new Error(`${name} must be an integer between ${minimum} and ${maximum}; received '${rawValue}'`); + } + return value; +} + +export function loadRuntimeConfig( + environment: Record = process.env, +): RuntimeConfig { + return { + cacheTtlSeconds: parseBoundedInteger( + "CACHE_TTL_SECONDS", + environment.CACHE_TTL_SECONDS, + DEFAULT_CACHE_TTL_SECONDS, + 0, + 3600, + ), + rateLimitRpm: parseBoundedInteger( + "RATE_LIMIT_RPM", + environment.RATE_LIMIT_RPM, + DEFAULT_RATE_LIMIT_RPM, + 1, + 60, + ), + }; +} diff --git a/src/index.ts b/src/index.ts index 4a1d82f..693e7a1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -37,21 +37,21 @@ import { validateSearchInput, validateTASummaryInput, } from "./api/validation.js"; +import { loadRuntimeConfig } from "./config.js"; import { createRequire } from "module"; const require = createRequire(import.meta.url); const pkg = require("../package.json"); // Configuration from environment -const CACHE_TTL = parseInt(process.env.CACHE_TTL_SECONDS || "300"); -const RATE_LIMIT_RPM = parseInt(process.env.RATE_LIMIT_RPM || "10"); - -// Initialize components -const client = new TradingViewClient(); -const searchClient = new SearchClient(); -const metainfoClient = new MetainfoClient(); -const cache = new Cache(CACHE_TTL); -const rateLimiter = new RateLimiter(RATE_LIMIT_RPM); +const { cacheTtlSeconds, rateLimitRpm } = loadRuntimeConfig(); + +// Initialize shared infrastructure before clients so retries consume rate-limit tokens. +const cache = new Cache(cacheTtlSeconds); +const rateLimiter = new RateLimiter(rateLimitRpm); +const client = new TradingViewClient({ rateLimiter }); +const searchClient = new SearchClient({ rateLimiter }); +const metainfoClient = new MetainfoClient({ rateLimiter }); const screenTool = new ScreenTool(client, cache, rateLimiter); const searchTool = new SearchTool(searchClient, cache, rateLimiter); const metainfoTool = new MetainfoTool(metainfoClient, cache, rateLimiter); @@ -765,7 +765,7 @@ async function main() { await server.connect(transport); console.error("TradingView MCP Server running on stdio"); - console.error(`Cache TTL: ${CACHE_TTL}s | Rate Limit: ${RATE_LIMIT_RPM} req/min`); + console.error(`Cache TTL: ${cacheTtlSeconds}s | Rate Limit: ${rateLimitRpm} req/min`); } main().catch((error) => { diff --git a/src/tests/cli.test.ts b/src/tests/cli.test.ts index eaa0cc7..e894eb5 100644 --- a/src/tests/cli.test.ts +++ b/src/tests/cli.test.ts @@ -541,9 +541,10 @@ describe("CLI - Integration", () => { }); describe("CLI - End-to-End (child process)", () => { - const cli = (args: string[]) => + const cli = (args: string[], env: Record = {}) => execFileAsync("npx", ["tsx", "src/cli.ts", ...args], { timeout: 10000, + env: { ...process.env, ...env }, }); it("should show help with --help", async () => { @@ -563,6 +564,16 @@ describe("CLI - End-to-End (child process)", () => { assert.ok(stdout.startsWith("tradingview-cli v")); }); + it("rejects invalid runtime configuration before handling commands", async () => { + await assert.rejects( + () => cli(["--version"], { RATE_LIMIT_RPM: "fast" }), + (err: any) => { + assert.ok(err.stderr.includes("RATE_LIMIT_RPM must be an integer between 1 and 60")); + return true; + }, + ); + }); + it("should list presets as JSON", async () => { const { stdout } = await cli(["presets"]); const result = JSON.parse(stdout); diff --git a/src/tests/transport.test.ts b/src/tests/transport.test.ts new file mode 100644 index 0000000..968284d --- /dev/null +++ b/src/tests/transport.test.ts @@ -0,0 +1,304 @@ +import { describe, it, mock } from "node:test"; +import assert from "node:assert/strict"; + +import { + requestJson, + UpstreamError, + type FetchLike, +} from "../api/transport.js"; +import { TradingViewClient } from "../api/client.js"; +import { SearchClient } from "../api/search.js"; +import { MetainfoClient } from "../api/metainfo.js"; +import type { RateLimiter } from "../utils/rateLimit.js"; +import { loadRuntimeConfig } from "../config.js"; + +function response(status: number, payload: unknown) { + return { + ok: status >= 200 && status < 300, + status, + statusText: status === 503 ? "Service Unavailable" : "OK", + json: async () => payload, + }; +} + +describe("bounded upstream transport", () => { + it("retries transient HTTP failures and returns the first successful payload", async () => { + let attempts = 0; + const fetchImpl = mock.fn(async () => { + attempts += 1; + return attempts === 1 ? response(503, { error: "busy" }) : response(200, { ok: true }); + }) as unknown as FetchLike; + + const result = await requestJson( + "https://example.test/scan", + { method: "GET" }, + { fetchImpl, retryDelayMs: 0 }, + ); + + assert.deepEqual(result, { ok: true }); + assert.equal(attempts, 2); + }); + + it("stops after the bounded retry budget", async () => { + let attempts = 0; + const fetchImpl = mock.fn(async () => { + attempts += 1; + return response(503, { error: "busy" }); + }) as unknown as FetchLike; + + await assert.rejects( + () => requestJson("https://example.test/scan", { method: "GET" }, { fetchImpl, retryDelayMs: 0 }), + (error: unknown) => error instanceof UpstreamError + && error.kind === "http" + && error.status === 503, + ); + assert.equal(attempts, 3); + }); + + it("charges the shared rate limiter for each retry without retrying input validation errors", async () => { + const rateLimiter = { acquire: mock.fn(async () => {}) }; + let attempts = 0; + const fetchImpl = mock.fn(async () => { + attempts += 1; + return attempts === 1 ? response(500, {}) : response(200, { ok: true }); + }) as unknown as FetchLike; + + await requestJson( + "https://example.test/scan", + { method: "GET" }, + { fetchImpl, rateLimiter: rateLimiter as unknown as RateLimiter, retryDelayMs: 0 }, + ); + assert.equal(rateLimiter.acquire.mock.calls.length, 1); + + const search = new SearchClient({ fetchImpl, maxRetries: 2, retryDelayMs: 0 }); + await assert.rejects(() => search.searchSymbols({ query: " " }), /at least 1 character/); + assert.equal(attempts, 2); + }); + + it("does not retry malformed JSON or caller-independent response parsing failures", async () => { + let attempts = 0; + const fetchImpl = mock.fn(async () => { + attempts += 1; + return { + ok: true, + status: 200, + statusText: "OK", + json: async () => { + throw new SyntaxError("invalid JSON"); + }, + }; + }) as unknown as FetchLike; + + await assert.rejects( + () => requestJson("https://example.test/scan", { method: "GET" }, { fetchImpl, retryDelayMs: 0 }), + (error: unknown) => error instanceof UpstreamError && error.kind === "parse", + ); + assert.equal(attempts, 1); + }); + + it("normalizes timeout failures without an unbounded retry loop", async () => { + const abortError = Object.assign(new Error("aborted"), { name: "AbortError" }); + const fetchImpl = mock.fn(async () => { + throw abortError; + }) as unknown as FetchLike; + + await assert.rejects( + () => requestJson( + "https://example.test/scan", + { method: "GET" }, + { fetchImpl, maxRetries: 0, retryDelayMs: 0 }, + ), + (error: unknown) => error instanceof UpstreamError + && error.kind === "timeout" + && /timed out/.test(error.message), + ); + }); + it("retries a transient response-body timeout", async () => { + let attempts = 0; + const abortError = Object.assign(new Error("body aborted"), { name: "AbortError" }); + const fetchImpl = mock.fn(async () => { + attempts += 1; + if (attempts === 1) { + return { + ok: true, + status: 200, + statusText: "OK", + json: async () => { + throw abortError; + }, + }; + } + return response(200, { ok: true }); + }) as unknown as FetchLike; + + const result = await requestJson( + "https://example.test/scan", + { method: "GET" }, + { fetchImpl, retryDelayMs: 0 }, + ); + + assert.deepEqual(result, { ok: true }); + assert.equal(attempts, 2); + }); +}); + +describe("upstream response-shape validation", () => { + it("rejects malformed screener payloads instead of treating them as empty data", async () => { + const fetchImpl = mock.fn(async () => response(200, { totalCount: 1, data: [{ s: "NASDAQ:AAPL" }] })) as unknown as FetchLike; + const client = new TradingViewClient({ fetchImpl, maxRetries: 0 }); + + await assert.rejects( + () => client.scanStocks({ + filter: [], + columns: ["name"], + sort: { sortBy: "name", sortOrder: "asc" }, + range: [0, 1], + }), + /malformed screener response/, + ); + }); + + it("accepts boolean screener cells used by metadata fields", async () => { + const fetchImpl = mock.fn(async () => response(200, { + totalCount: 1, + data: [{ s: "NASDAQ:AAPL", d: ["Apple", true] }], + })) as unknown as FetchLike; + const client = new TradingViewClient({ fetchImpl, maxRetries: 0 }); + + const result = await client.scanStocks({ + filter: [], + columns: ["name", "is_primary"], + sort: { sortBy: "name", sortOrder: "asc" }, + range: [0, 1], + }); + + assert.deepEqual(result.data[0].d, ["Apple", true]); + }); + + it("rejects malformed symbol-search payloads", async () => { + const fetchImpl = mock.fn(async () => response(200, { error: "bad payload" })) as unknown as FetchLike; + const client = new SearchClient({ fetchImpl, maxRetries: 0 }); + + await assert.rejects( + () => client.searchSymbols({ query: "apple" }), + /malformed symbol search response/, + ); + }); + + it("rejects malformed metainfo payloads in summary mode", async () => { + const fetchImpl = mock.fn(async () => response(200, [])) as unknown as FetchLike; + const client = new MetainfoClient({ fetchImpl, maxRetries: 0 }); + + await assert.rejects( + () => client.getMetainfo({ market: "america" }), + /malformed metainfo response/, + ); + }); + + it("normalizes supported root-array metainfo payloads", async () => { + const fetchImpl = mock.fn(async () => response(200, [ + { propName: "close", title: "Close", kind: "number" }, + { propName: "name", title: "Name", kind: "string" }, + ])) as unknown as FetchLike; + const client = new MetainfoClient({ fetchImpl, maxRetries: 0 }); + + const result = await client.getMetainfo({ market: "america" }); + + assert.equal(result.metainfo.field_count, 2); + assert.deepEqual(result.metainfo.fields.map((field) => field.name), ["close", "name"]); + }); +}); + +describe("client error compatibility", () => { + const badRequest = () => ({ + ok: false, + status: 400, + statusText: "Bad Request", + json: async () => ({}), + }); + + it("preserves HTTP error messages for all upstream clients", async () => { + const fetchImpl = mock.fn(async () => badRequest()) as unknown as FetchLike; + const request = { + filter: [], + columns: ["name"], + sort: { sortBy: "name", sortOrder: "asc" as const }, + range: [0, 1] as [number, number], + }; + + await assert.rejects( + () => new TradingViewClient({ fetchImpl, maxRetries: 0 }).scanStocks(request), + { message: "TradingView API error: 400 Bad Request" }, + ); + await assert.rejects( + () => new SearchClient({ fetchImpl, maxRetries: 0 }).searchSymbols({ query: "apple" }), + { message: "Symbol search failed: 400 Bad Request" }, + ); + await assert.rejects( + () => new MetainfoClient({ fetchImpl, maxRetries: 0 }).getMetainfo({ market: "america" }), + { message: "Metainfo request failed: 400 Bad Request" }, + ); + }); + + it("preserves timeout error messages for all upstream clients", async () => { + const abortError = Object.assign(new Error("aborted"), { name: "AbortError" }); + const fetchImpl = mock.fn(async () => { + throw abortError; + }) as unknown as FetchLike; + const request = { + filter: [], + columns: ["name"], + sort: { sortBy: "name", sortOrder: "asc" as const }, + range: [0, 1] as [number, number], + }; + + await assert.rejects( + () => new TradingViewClient({ fetchImpl, maxRetries: 0 }).scanStocks(request), + { message: "Request timeout" }, + ); + await assert.rejects( + () => new SearchClient({ fetchImpl, maxRetries: 0 }).searchSymbols({ query: "apple" }), + { message: "Symbol search request timeout" }, + ); + await assert.rejects( + () => new MetainfoClient({ fetchImpl, maxRetries: 0 }).getMetainfo({ market: "america" }), + { message: "Metainfo request timeout" }, + ); + }); +}); + +describe("runtime configuration validation", () => { + it("uses documented defaults and accepts the cache disable value", () => { + assert.deepEqual(loadRuntimeConfig({}), { + cacheTtlSeconds: 300, + rateLimitRpm: 10, + }); + assert.deepEqual(loadRuntimeConfig({ CACHE_TTL_SECONDS: "0", RATE_LIMIT_RPM: "60" }), { + cacheTtlSeconds: 0, + rateLimitRpm: 60, + }); + assert.throws( + () => loadRuntimeConfig({ RATE_LIMIT_RPM: "" }), + /RATE_LIMIT_RPM must be an integer between 1 and 60/, + ); + }); + + it("rejects invalid cache TTL and rate-limit settings before startup", () => { + assert.throws( + () => loadRuntimeConfig({ CACHE_TTL_SECONDS: "-1" }), + /CACHE_TTL_SECONDS must be an integer between 0 and 3600/, + ); + assert.throws( + () => loadRuntimeConfig({ CACHE_TTL_SECONDS: "3.5" }), + /CACHE_TTL_SECONDS must be an integer between 0 and 3600/, + ); + assert.throws( + () => loadRuntimeConfig({ RATE_LIMIT_RPM: "0" }), + /RATE_LIMIT_RPM must be an integer between 1 and 60/, + ); + assert.throws( + () => loadRuntimeConfig({ RATE_LIMIT_RPM: "fast" }), + /RATE_LIMIT_RPM must be an integer between 1 and 60/, + ); + }); +}); From fcd36c9dfc21ff84e59627bc8fa5222465f35c34 Mon Sep 17 00:00:00 2001 From: Pavel Fadeev Date: Sun, 16 Aug 2026 02:58:16 +0200 Subject: [PATCH 2/2] fix(transport): accept structured cells and dispose failed bodies --- src/api/client.ts | 20 +++++++++++++++--- src/api/transport.ts | 19 +++++++++++++++++ src/api/types.ts | 10 ++++++++- src/tests/transport.test.ts | 42 +++++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/src/api/client.ts b/src/api/client.ts index 8c58c0e..4088ae6 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -3,7 +3,7 @@ */ import { createRequire } from "module"; -import type { ScreenerRequest, ScreenerResponse } from "./types.js"; +import type { ScreenerCell, ScreenerRequest, ScreenerResponse } from "./types.js"; import { requestJson, type TransportOptions, @@ -15,8 +15,22 @@ const pkg = require("../../package.json"); const API_BASE = "https://scanner.tradingview.com"; -function isScreenerCell(value: unknown): value is number | string | boolean | null { - return value === null || typeof value === "number" || typeof value === "string" || typeof value === "boolean"; +function isScreenerCell(value: unknown): value is ScreenerCell { + if ( + value === null + || typeof value === "number" + || typeof value === "string" + || typeof value === "boolean" + ) { + return true; + } + if (Array.isArray(value)) { + return value.every(isScreenerCell); + } + if (typeof value === "object") { + return Object.values(value).every(isScreenerCell); + } + return false; } function isScreenerRow( diff --git a/src/api/transport.ts b/src/api/transport.ts index f45f1b3..52bf724 100644 --- a/src/api/transport.ts +++ b/src/api/transport.ts @@ -6,6 +6,7 @@ export interface FetchResponse { ok: boolean; status: number; statusText: string; + body?: unknown; json(): Promise; } @@ -60,6 +61,23 @@ async function sleep(delayMs: number): Promise { if (delayMs > 0) await delay(delayMs); } +function disposeResponse(response: FetchResponse): void { + const body = response.body; + if (typeof body !== "object" || body === null) return; + + const candidate = body as { + destroy?: () => void; + cancel?: () => Promise | void; + }; + if (typeof candidate.destroy === "function") { + candidate.destroy(); + return; + } + if (typeof candidate.cancel === "function") { + void Promise.resolve(candidate.cancel()).catch(() => {}); + } +} + /** * Execute one JSON request with a bounded retry budget for transient failures. * Parse and response-shape failures are deliberately not retried. @@ -88,6 +106,7 @@ export async function requestJson( throw new UpstreamError("Upstream transport returned a malformed response", "response"); } if (!response.ok) { + disposeResponse(response); throw new UpstreamError( `Upstream request failed with HTTP ${response.status}${response.statusText ? `: ${response.statusText}` : ""}`, "http", diff --git a/src/api/types.ts b/src/api/types.ts index 1dfaef2..537fa3f 100644 --- a/src/api/types.ts +++ b/src/api/types.ts @@ -48,11 +48,19 @@ export interface ScreenerRequest { markets?: string[]; } +export type ScreenerCell = + | number + | string + | boolean + | null + | ScreenerCell[] + | { [key: string]: ScreenerCell }; + export interface ScreenerResponse { totalCount: number; data: Array<{ s: string; // symbol - d: (number | string | boolean | null)[]; // data array + d: ScreenerCell[]; // data array }>; } diff --git a/src/tests/transport.test.ts b/src/tests/transport.test.ts index 968284d..fd54fa0 100644 --- a/src/tests/transport.test.ts +++ b/src/tests/transport.test.ts @@ -39,6 +39,31 @@ describe("bounded upstream transport", () => { assert.equal(attempts, 2); }); + it("disposes retryable response bodies before retrying", async () => { + const destroy = mock.fn(); + let attempts = 0; + const fetchImpl = mock.fn(async () => { + attempts += 1; + if (attempts === 1) { + return { + ...response(503, { error: "busy" }), + body: { destroy }, + }; + } + return response(200, { ok: true }); + }) as unknown as FetchLike; + + const result = await requestJson( + "https://example.test/scan", + { method: "GET" }, + { fetchImpl, retryDelayMs: 0 }, + ); + + assert.deepEqual(result, { ok: true }); + assert.equal(attempts, 2); + assert.equal(destroy.mock.callCount(), 1); + }); + it("stops after the bounded retry budget", async () => { let attempts = 0; const fetchImpl = mock.fn(async () => { @@ -175,6 +200,23 @@ describe("upstream response-shape validation", () => { assert.deepEqual(result.data[0].d, ["Apple", true]); }); + it("accepts JSON arrays and objects in screener cells", async () => { + const fetchImpl = mock.fn(async () => response(200, { + totalCount: 1, + data: [{ s: "NASDAQ:AAPL", d: [["common"], { type: "map", columns: ["close"] }] }], + })) as unknown as FetchLike; + const client = new TradingViewClient({ fetchImpl, maxRetries: 0 }); + + const result = await client.scanStocks({ + filter: [], + columns: ["typespecs", "map"], + sort: { sortBy: "name", sortOrder: "asc" }, + range: [0, 1], + }); + + assert.deepEqual(result.data[0].d, [["common"], { type: "map", columns: ["close"] }]); + }); + it("rejects malformed symbol-search payloads", async () => { const fetchImpl = mock.fn(async () => response(200, { error: "bad payload" })) as unknown as FetchLike; const client = new SearchClient({ fetchImpl, maxRetries: 0 });