Skip to content

Commit 6a9d0e3

Browse files
committed
fix(cli): prevent resource leaks in HTTP timeout handling
Address code review feedback: - Add settled flag to prevent multiple resolve/reject calls - Call req.destroy() as backup when timeout fires - Reject promise directly in timeout handler (don't rely only on abort signal) - Use wrapper functions settledReject/settledResolve for safe promise settlement
1 parent 8661b18 commit 6a9d0e3

1 file changed

Lines changed: 34 additions & 14 deletions

File tree

cli/lib/checkup-api.ts

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,24 @@ async function postRpc<T>(params: {
159159

160160
// Use AbortController for clean timeout handling
161161
const controller = new AbortController();
162-
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
162+
let timeoutId: ReturnType<typeof setTimeout> | null = null;
163+
let settled = false;
163164

164165
return new Promise((resolve, reject) => {
166+
const settledReject = (err: Error) => {
167+
if (settled) return;
168+
settled = true;
169+
if (timeoutId) clearTimeout(timeoutId);
170+
reject(err);
171+
};
172+
173+
const settledResolve = (value: T) => {
174+
if (settled) return;
175+
settled = true;
176+
if (timeoutId) clearTimeout(timeoutId);
177+
resolve(value);
178+
};
179+
165180
const req = https.request(
166181
url,
167182
{
@@ -173,13 +188,12 @@ async function postRpc<T>(params: {
173188
let data = "";
174189
res.on("data", (chunk) => (data += chunk));
175190
res.on("end", () => {
176-
clearTimeout(timeoutId);
177191
if (res.statusCode && res.statusCode >= 200 && res.statusCode < 300) {
178192
try {
179193
const parsed = JSON.parse(data);
180-
resolve(unwrapRpcResponse(parsed) as T);
194+
settledResolve(unwrapRpcResponse(parsed) as T);
181195
} catch {
182-
reject(new Error(`Failed to parse RPC response: ${data}`));
196+
settledReject(new Error(`Failed to parse RPC response: ${data}`));
183197
}
184198
} else {
185199
const statusCode = res.statusCode || 0;
@@ -191,31 +205,37 @@ async function postRpc<T>(params: {
191205
payloadJson = null;
192206
}
193207
}
194-
reject(new RpcError({ rpcName, statusCode, payloadText: data, payloadJson }));
208+
settledReject(new RpcError({ rpcName, statusCode, payloadText: data, payloadJson }));
195209
}
196210
});
197-
res.on("error", () => {
198-
clearTimeout(timeoutId);
211+
res.on("error", (err) => {
212+
settledReject(err);
199213
});
200214
}
201215
);
216+
217+
// Set up timeout with both abort signal AND req.destroy() as backup
218+
timeoutId = setTimeout(() => {
219+
controller.abort();
220+
req.destroy(); // Backup: ensure request is terminated
221+
settledReject(new Error(`RPC ${rpcName} timed out after ${timeoutMs}ms`));
222+
}, timeoutMs);
202223

203224
req.on("error", (err: Error) => {
204-
clearTimeout(timeoutId);
205-
// Handle abort as timeout
225+
// Handle abort as timeout (may already be rejected by timeout handler)
206226
if (err.name === "AbortError" || (err as any).code === "ABORT_ERR") {
207-
reject(new Error(`RPC ${rpcName} timed out after ${timeoutMs}ms`));
227+
settledReject(new Error(`RPC ${rpcName} timed out after ${timeoutMs}ms`));
208228
return;
209229
}
210230
// Provide clearer error for common network issues
211231
if ((err as any).code === "ECONNREFUSED") {
212-
reject(new Error(`RPC ${rpcName} failed: connection refused to ${url.host}`));
232+
settledReject(new Error(`RPC ${rpcName} failed: connection refused to ${url.host}`));
213233
} else if ((err as any).code === "ENOTFOUND") {
214-
reject(new Error(`RPC ${rpcName} failed: DNS lookup failed for ${url.host}`));
234+
settledReject(new Error(`RPC ${rpcName} failed: DNS lookup failed for ${url.host}`));
215235
} else if ((err as any).code === "ECONNRESET") {
216-
reject(new Error(`RPC ${rpcName} failed: connection reset by server`));
236+
settledReject(new Error(`RPC ${rpcName} failed: connection reset by server`));
217237
} else {
218-
reject(err);
238+
settledReject(err);
219239
}
220240
});
221241

0 commit comments

Comments
 (0)