Skip to content

runtime_import.cjs: unhandled response error listener crashes on socket-level failures (require-http-response-error-listener fla #55002

Description

@github-actions

Summary

require-http-response-error-listener correctly flags this exact pattern in its own invalid test case, but the live call site in actions/setup/js/runtime_import.cjs:732-757 (fetchUrlContent) has never been fixed — likely because the rule is registered at "warn" severity in eslint-factory/eslint.config.cjs:68, so it doesn't block CI.

The bug

protocol
  .get(url, res => {
    if (res.statusCode !== 200) { reject(...); return; }
    let data = "";
    res.on("data", chunk => { data += chunk; });
    res.on("end", () => { ... });
  })
  .on("error", err => {           // <-- attached to the REQUEST, not `res`
    reject(new Error(`Failed to fetch URL ${url}: ${err.message}`));
  });

Node emits 'error' on the IncomingMessage (res) itself for socket-level failures that occur while the body is streamed (reset connections, decompression failures, aborted sockets mid-transfer). The .on("error", ...) attached to the request object does not catch these. If the connection drops mid-download, the unhandled 'error' event on res throws and crashes the whole action (this function backs URL-based @import resolution, so any workflow importing a remote include is exposed).

Ask

  1. Fix runtime_import.cjs's fetchUrlContent: attach res.on("error", ...) (reject the promise) alongside the existing data/end listeners.
  2. Re-run/verify the rule against the fixed file to confirm the diagnostic clears.
  3. Separately: audit whether "warn"-severity problem-type rules like this one should be promoted to "error" (or otherwise CI-gated) — a rule can be working exactly as designed and still leave a real crash bug unfixed indefinitely if nothing enforces the warning. This one specific case is the concrete instance; the systemic question (are there other live "warn" diagnostics with equivalent real fixes pending?) is worth a follow-up sweep.
Rule mechanics verified (why this is a true positive, not a rule gap)
  • protocol resolves via isHttpModuleBinding: it's const protocol = url.startsWith("https") ? https : http, and both https/http are require("https")/require("http") at file top (runtime_import.cjs:17-18) — the rule's ConditionalExpression handling (require-http-response-error-listener.ts:42-43) resolves this correctly.
  • getResponseCallback finds the res => {...} arrow passed to .get(url, ...), with res as a plain identifier param — matches.
  • hasErrorListener scans all references of res for a res.on("error", ...)/res.once("error", ...) call; only res.on("data", ...) and res.on("end", ...) exist, so it reports missingResponseErrorListener.
  • No eslint-disable comment is present near this code.

Generated by 🤖 ESLint Refiner · agent · 381.2 AIC · ⌖ 7.71 AIC · ⊞ 5.8K ·

  • expires on Aug 29, 2026, 9:32 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions