Summary
In require-error-cause-in-rethrow, expressionReferencesCatchVar (src/rules/require-error-cause-in-rethrow.ts:27-56) decides whether the rethrown new Error(...) message references the caught variable. It recurses through Identifier, CallExpression, TemplateLiteral, BinaryExpression, and MemberExpression — but it does not handle several node types that routinely wrap error values in defensive message-building:
LogicalExpression (err.message || "unknown", err ?? fallback, err.message && ...) — note ESTree models ||/&&/?? as LogicalExpression, not BinaryExpression, so the existing BinaryExpression branch does not cover them.ConditionalExpression (cond ? err.message : "n/a").SequenceExpression and array/object literals holding the catch var (new Error([err].join('')), less common).
Any rethrow whose reference to the caught variable lives only inside one of these unhandled nodes returns false → the rule does not fire → the cause chain is silently dropped.
Grounding
actions/setup/js/create_pull_request.cjs:589 builds its message with a LogicalExpression fallback:
throw new Error(`Failed to delete existing remote branch "${branchName}" for reuse with recreate-ref: ${message || String(err)}`);The ${message || String(err)} template expression is a LogicalExpression; expressionReferencesCatchVar bails out at the unhandled node type and never reaches the String(err) reference. (This site is also affected by the alias blindspot filed separately — both gaps independently hide it; fixing only one is insufficient.)
The x.message || "fallback" and cond ? x.message : default idioms are standard defensive error-formatting in JS, so this is a broad precision gap even where no aliasing is involved.
Acceptance criteria
- Extend
expressionReferencesCatchVar to traverse LogicalExpression (both operands), ConditionalExpression (test/consequent/alternate), and SequenceExpression (all expressions). Consider array/object literals for completeness if cheap.
- Add RuleTester coverage:
- invalid:
catch (err) { throw new Error(\x: ${err.message || 'unknown'}`); }`
- invalid:
catch (err) { throw new Error(cond ? err.message : 'n/a'); }
- valid: same shapes but with
{ cause: err } present.
- valid (regression guard): a logical expression that does NOT reference the catch var, e.g.
throw new Error(a || b);, must remain un-flagged.
- Confirm no new false positives on the existing valid fixtures (the
"Something went wrong" / unrelated-message cases).
Notes
Keep the traversal purely syntactic (no type info / no cross-statement dataflow), consistent with the rest of the rule. Recursion is already bounded by AST depth.
Generated by 🤖 ESLint Refiner · 197.1 AIC · ⌖ 13.3 AIC · ⊞ 4.6K · ◷
Summary
In
require-error-cause-in-rethrow,expressionReferencesCatchVar(src/rules/require-error-cause-in-rethrow.ts:27-56) decides whether the rethrownnew Error(...)message references the caught variable. It recurses throughIdentifier,CallExpression,TemplateLiteral,BinaryExpression, andMemberExpression— but it does not handle several node types that routinely wrap error values in defensive message-building:LogicalExpression(err.message || "unknown",err ?? fallback,err.message && ...) — note ESTree models||/&&/??asLogicalExpression, notBinaryExpression, so the existingBinaryExpressionbranch does not cover them.ConditionalExpression(cond ? err.message : "n/a").SequenceExpressionand array/object literals holding the catch var (new Error([err].join('')), less common).Any rethrow whose reference to the caught variable lives only inside one of these unhandled nodes returns
false→ the rule does not fire → the cause chain is silently dropped.Grounding
actions/setup/js/create_pull_request.cjs:589builds its message with aLogicalExpressionfallback:The
${message || String(err)}template expression is aLogicalExpression;expressionReferencesCatchVarbails out at the unhandled node type and never reaches theString(err)reference. (This site is also affected by the alias blindspot filed separately — both gaps independently hide it; fixing only one is insufficient.)The
x.message || "fallback"andcond ? x.message : defaultidioms are standard defensive error-formatting in JS, so this is a broad precision gap even where no aliasing is involved.Acceptance criteria
expressionReferencesCatchVarto traverseLogicalExpression(both operands),ConditionalExpression(test/consequent/alternate), andSequenceExpression(all expressions). Consider array/object literals for completeness if cheap.catch (err) { throw new Error(\x: ${err.message || 'unknown'}`); }`catch (err) { throw new Error(cond ? err.message : 'n/a'); }{ cause: err }present.throw new Error(a || b);, must remain un-flagged."Something went wrong"/ unrelated-message cases).Notes
Keep the traversal purely syntactic (no type info / no cross-statement dataflow), consistent with the rest of the rule. Recursion is already bounded by AST depth.