Skip to content

eslint-factory: require-error-cause-in-rethrow misses the const err = catchError normalization idiom (live FN in create_pull_r [Content truncated due to length] #43947

Description

@github-actions

Summary

require-error-cause-in-rethrow (the newly added 11th rule) tracks only the catch-clause parameter identifier as the "caught variable." It does not follow the extremely common normalization idiom where the catch binding is immediately aliased into a local, TS-friendly variable:

catch (deleteError) {
  /** `@type` {any} */
  const err = deleteError;   // <-- normalization alias
  ...
  throw new Error(`... ${message || String(err)}`);  // references `err`, NOT `deleteError`
}

Because expressionReferencesCatchVar searches for the catch param name (deleteError) and the rethrow message only references the alias (err), the rule never fires — the original error chain is silently dropped and the rule's whole reason for existing is defeated.

Grounding

The /** @type {any} */ const err = <catchError>; idiom is used in 5 corpus files:

file catch param alias
actions/setup/js/create_pull_request.cjs:573-575 deleteError const err = deleteError
actions/setup/js/push_signed_commits.cjs:671-673 createRefError const err = createRefError
actions/setup/js/dispatch_workflow.cjs:272-274 dispatchError const err = dispatchError
actions/setup/js/route_slash_command.cjs:341 dispatchError const err = dispatchError
actions/setup/js/mcp_server_core.cjs:830 error const err = error

Live true-negative (rule should fire, doesn't): actions/setup/js/create_pull_request.cjs:589:

} catch (deleteError) {
  const err = deleteError;
  ...
  throw new Error(`Failed to delete existing remote branch "${branchName}" for reuse with recreate-ref: ${message || String(err)}`);
}

This rethrows a brand-new Error derived from the caught value, with no { cause } — exactly the defect the rule targets — yet it escapes detection purely because of the alias.

This is a growing blindspot: as more catch blocks adopt the @type {any} normalization idiom for TypeScript-checked .cjs, every rethrow inside them becomes invisible to the rule.

Acceptance criteria

  1. When a catch body contains const <alias> = <catchParam>; (a simple, un-reassigned local initialized directly from the catch binding), treat references to <alias> in the rethrown new Error(...) as references to the caught variable.
    • Scope the alias resolution to the catch body and require the initializer be exactly the catch param identifier (no method calls / member access) to stay conservative.
    • The reported/suggested { cause: ... } should use whichever identifier is in scope and correct (prefer the alias if the catch param is shadowed/renamed, else the catch param).
  2. Do not introduce false positives: a const x = someUnrelatedValue; must not be treated as an alias, and a rethrow via bare throw err; (no new Error) must still be ignored.
  3. Add RuleTester coverage: (a) invalid — aliased catch var referenced in new Error message without cause, mirroring create_pull_request.cjs:589; (b) valid — same but with { cause: err }; (c) valid — alias initialized from an unrelated value; (d) valid — alias reassigned before the throw (too complex to track → do not flag).
  4. Re-verify against create_pull_request.cjs:589 after the change: it should now be flagged (or the site fixed to add { cause }).

Notes

Related but distinct precision gap (filed separately): expressionReferencesCatchVar does not traverse LogicalExpression/ConditionalExpression, which independently hides the same ${message || String(err)} site. Both gaps must be closed for line 589 to be detected.

Generated by 🤖 ESLint Refiner · 197.1 AIC · ⌖ 13.3 AIC · ⊞ 4.6K ·

  • expires on Jul 13, 2026, 10:39 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

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions