Skip to content

eslint-factory: 11 try/catch rules overclaim will-crash-the-action wording; precedent fix propagated incompletely #56288

Description

@github-actions

Summary

Eleven eslint-factory try/catch-requiring rules (including both rules reviewed today) still contain the phrase "...will crash the action if unhandled" in their meta.messages.requireTryCatch and/or docs.description strings. This wording overclaims the risk: every actions/setup/js entrypoint has a top-level try/catch (enforced separately by require-async-entrypoint-catch) that routes any uncaught throw to core.setFailed. An unhandled throw at one of these call sites therefore does not crash the action silently — it still surfaces as a controlled, actionable job failure. The real value lost by skipping the fix is the specific { cause } context and call-site-framed message; the generic entrypoint catch produces a less-useful engine-level stack instead.

This exact defect was already identified and fixed for require-fetch-response-body-try-catch in #52644 (closed) — the wording there was reworded to describe the actual benefit (preserved cause / specific message) rather than claiming a crash. That fix was scoped narrowly to one rule and never propagated to its many siblings, so the same overclaim persists verbatim elsewhere, including in require-mkdtempsync-try-catch and require-decodeuricomponent-try-catch, both added by eslint-miner after #52644 landed — meaning the defect has already recurred once since the "fix."

Grounded occurrences (verbatim "will crash the action" phrasing)

11 affected rule files
  • eslint-factory/src/rules/require-fs-io-try-catch.ts:23
  • eslint-factory/src/rules/require-fs-sync-try-catch.ts:25
  • eslint-factory/src/rules/require-mkdirsync-try-catch.ts:21
  • eslint-factory/src/rules/require-mkdtempsync-try-catch.ts:21
  • eslint-factory/src/rules/require-rmsync-try-catch.ts:21
  • eslint-factory/src/rules/require-decodeuricomponent-try-catch.ts:22
  • eslint-factory/src/rules/require-new-url-try-catch.ts:19
  • eslint-factory/src/rules/require-execsync-try-catch.ts:90
  • eslint-factory/src/rules/require-execfilesync-try-catch.ts:90
  • eslint-factory/src/rules/require-fetch-try-catch.ts:87 and :91

Ask

  1. Reword each occurrence to describe the actual benefit — preserves the original error as { cause } and produces a call-site-specific message, instead of a generic engine-level stack surfaced through the entrypoint's top-level catch — following the exact pattern already applied in eslint-factory: require-fetch-response-body-try-catch — "will crash the action" message overclaims given the codebase's entrypoi [Content truncated due to length] #52644 for require-fetch-response-body-try-catch.
  2. To stop this recurring a third time (it already recurred once via two new eslint-miner-authored rules after the first fix), add a small guard: either a shared message-string constant/template that all these rules import, or a meta-test that scans meta.messages/docs.description across all rules for the phrase "will crash the action" and fails if it's reintroduced without an explicit allowlist entry.
  3. No detection-logic changes needed anywhere — this is scoped entirely to message/docs wording.

Scope

eslint-factory/src/rules/{require-fs-io-try-catch,require-fs-sync-try-catch,require-mkdirsync-try-catch,require-mkdtempsync-try-catch,require-rmsync-try-catch,require-decodeuricomponent-try-catch,require-new-url-try-catch,require-execsync-try-catch,require-execfilesync-try-catch,require-fetch-try-catch}.ts (+ their .test.ts files if message-text assertions need updating).

Generated by 🤖 ESLint Refiner · claude · agent · 223.2 AIC · ⌖ 7.75 AIC · ⊞ 5.8K · ◷

  • expires on Sep 2, 2026, 11:56 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