Skip to content

eslint-factory: broaden no-unsafe-catch-error-property to flag .status, .cause, .name - #42239

Merged
pelikhan merged 2 commits into
mainfrom
copilot/eslint-factory-broaden-no-unsafe-catch
Jun 29, 2026
Merged

eslint-factory: broaden no-unsafe-catch-error-property to flag .status, .cause, .name#42239
pelikhan merged 2 commits into
mainfrom
copilot/eslint-factory-broaden-no-unsafe-catch

Conversation

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

no-unsafe-catch-error-property (and its promise variant) only flagged .message, .stack, .code on unguarded caught error variables — missing .status accesses like if (err.status === 404) that are common in HTTP error handling.

Changes

  • Broadened UNSAFE_PROPERTIES in both no-unsafe-catch-error-property and no-unsafe-promise-catch-error-property to include status, cause, and name
  • Added typeof err === 'object' as an accepted guard (strict === only; alongside existing instanceof Error and getErrorMessage guards) — real call sites that check typeof err === 'object' before reading .status are no longer false positives
  • New RuleTester cases for all three new properties (invalid without guard) and typeof guard variants (valid)
// Now flagged:
try { ... } catch (err) { if (err.status === 404) { } }
//                              ^^^^^^^^^^

// Still suppressed — typeof guard recognized:
try { ... } catch (err) { if (typeof err === 'object') { doSomething(err.status); } }

…e/name with typeof guard

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add check for additional properties in no-unsafe-catch-error-property eslint-factory: broaden no-unsafe-catch-error-property to flag .status, .cause, .name Jun 29, 2026
Copilot AI requested a review from pelikhan June 29, 2026 12:31
@pelikhan
pelikhan marked this pull request as ready for review June 29, 2026 13:53
Copilot AI review requested due to automatic review settings June 29, 2026 13:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the eslint-factory “unsafe catch error property” rules to catch additional common error-property accesses (notably HTTP-style .status) and adds an additional guard pattern (typeof err === 'object') to reduce false positives at real call sites.

Changes:

  • Expanded both rules’ UNSAFE_PROPERTIES to include status, cause, and name.
  • Added recognition of typeof <errVar> === 'object' (and the reversed literal form) as an accepted guard in both rules.
  • Added RuleTester coverage for the new properties and the new typeof guard behavior.
Show a summary per file
File Description
eslint-factory/src/rules/no-unsafe-promise-catch-error-property.ts Expands unsafe properties and adds typeof … === 'object' guard detection for .catch() callbacks.
eslint-factory/src/rules/no-unsafe-promise-catch-error-property.test.ts Adds new valid/invalid cases for .status, .cause, .name, including typeof guard variants.
eslint-factory/src/rules/no-unsafe-catch-error-property.ts Expands unsafe properties and adds typeof … === 'object' guard detection for catch (err) blocks.
eslint-factory/src/rules/no-unsafe-catch-error-property.test.ts Adds new valid/invalid cases for .status, .cause, .name, and typeof guard variants (including suggestion expectations).

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 4/4 changed files
  • Comments generated: 5
  • Review effort level: Low

hasSuggestions: true,
docs: {
description: "Disallow direct access to .message, .stack, or .code on a caught error variable without a getErrorMessage guard",
description: "Disallow direct access to .message, .stack, .code, .status, .cause, or .name on a caught error variable without a getErrorMessage guard",
hasSuggestions: true,
docs: {
description: "Disallow direct access to .message, .stack, or .code on a promise .catch() callback parameter without a getErrorMessage guard",
description: "Disallow direct access to .message, .stack, .code, .status, .cause, or .name on a promise .catch() callback parameter without a getErrorMessage guard",
Comment on lines +104 to +108
// typeof varName === 'object' or 'object' === typeof varName
if (node.operator === "===") {
const { left, right } = node;
const isTypeofObject =
(left.type === AST_NODE_TYPES.UnaryExpression &&
Comment on lines +115 to +119
// typeof varName === 'object' or 'object' === typeof varName
if (node.operator === "===") {
const { left, right } = node;
const isTypeofObject =
(left.type === AST_NODE_TYPES.UnaryExpression &&
Comment on lines +368 to +370
messageId: "wrapWithInstanceof",
data: { errorVar: "err", prop: "status" },
output: `try { f(); } catch (err) { if ((err instanceof Error ? err.status : undefined) === 404) { } }`,
@github-actions

Copy link
Copy Markdown
Contributor

🤖 PR Triage — §28376613466

Field Value
Category chore (tooling)
Risk 🟢 Low
Priority 🟢 Low
Score 20 / 100
Action ⏸️ defer
Age 1.5h

Score breakdown: Impact 10 + Urgency 5 + Quality 5

Rationale: Broadens the no-unsafe-catch-error-property eslint rule to catch .status, .cause, .name accesses in addition to .message/.stack/.code. Good quality improvement (203+/9−, 4 files) but draft with no CI yet. Non-urgent tooling change — defer until promoted from draft and CI confirms no regressions.

Generated by 🔧 PR Triage Agent · 93.2 AIC · ⌖ 14.6 AIC · ⊞ 5.4K ·

@pelikhan
pelikhan merged commit 44d5a43 into main Jun 29, 2026
21 checks passed
@pelikhan
pelikhan deleted the copilot/eslint-factory-broaden-no-unsafe-catch branch June 29, 2026 14:48
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.82.0

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants