Skip to content

require-http-response-error-listener: misses response error listener FN when http/https module is bound via a ternary (protocol #54734

Description

@github-actions

Summary

require-http-response-error-listener (eslint-factory/src/rules/require-http-response-error-listener.ts) only recognizes an http/https module binding when the variable's initializer is directly a require("http")-style call (isRequireHttpModule inside isHttpModuleBinding). It does not resolve one level of indirection through a ConditionalExpression that selects between two already-resolved http-module bindings -- the common "pick http or https by URL scheme" idiom. As a result, the rule silently skips a live call site with the exact missing-response-error-listener bug it exists to catch.

Grounded false negative

actions/setup/js/runtime_import.cjs:

17: const https = require("https");
18: const http = require("http");
...
730:    const protocol = url.startsWith("https") ? https : http;
...
732:    protocol
733:      .get(url, res => {
734:        if (res.statusCode !== 200) {
735:          reject(new Error(`Failed to fetch URL ${url}: HTTP ${res.statusCode}`));
736:          return;
737:        }
738:        let data = "";
739:        res.on("data", chunk => { data += chunk; });
740:        res.on("end", () => { /* ... */ resolve(data); });
741:      })
742:      .on("error", err => {
743:        reject(new Error(`Failed to fetch URL ${url}: ${err.message}`));
744:      });

res here never gets an 'error' listener -- only the ClientRequest returned by .get() does (the .on("error", ...) at line 742 is chained onto the request, not the response). This is precisely the socket-level-response-error hazard the rule's own description calls out. It goes unreported because:

  1. isHttpRequestCall resolves protocol's binding via isHttpModuleBinding("protocol", ...).
  2. isHttpModuleBinding walks variable.defs/variable.references and requires the declarator's init (or every write) to itself be a require("http")-style CallExpression (isRequireHttpModule).
  3. Here init is url.startsWith("https") ? https : http -- a ConditionalExpression, not a CallExpression -- so isRequireHttpModule returns false and isHttpModuleBinding bails out, treating protocol as unresolved.

Acceptance criteria

  • Extend the module-binding resolution (isRequireHttpModule / isHttpModuleBinding) to also accept a ConditionalExpression initializer/write whose consequent and alternate both resolve (recursively, one level is enough) to an existing http/https module binding -- e.g. cond ? https : http where https/http are each bound via require("http"|"https"|"node:http"|"node:https").
  • Add a RuleTester invalid case mirroring runtime_import.cjs:730-744 (ternary-selected protocol, .get() callback with res.on("data"/"end") but no res.on("error", ...), only req.on("error", ...) chained on the return value) and confirm it now reports missingResponseErrorListener.
  • Re-verify the existing valid/invalid fixtures in require-http-response-error-listener.test.ts still pass unchanged.
  • Once the rule change lands, confirm runtime_import.cjs:733 is flagged, and add the missing res.on("error", ...) handler there as a follow-up fix (separate PR) so the live gap is closed, not just newly detected.

Scope

eslint-factory/src/rules/require-http-response-error-listener.ts (+ its .test.ts). Rule target file: actions/setup/js/runtime_import.cjs.

Generated by 🤖 ESLint Refiner · agent · 197.6 AIC · ⌖ 16 AIC · ⊞ 5.8K ·

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

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions