eslint-factory: add no-unsafe-catch-error-property rule#42057
Merged
pelikhan merged 2 commits intoJun 28, 2026
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add no-unsafe-catch-error-property rule for ESLint
eslint-factory: add no-unsafe-catch-error-property rule
Jun 28, 2026
pelikhan
deleted the
copilot/eslint-mineradd-no-unsafe-catch-error-property
branch
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new custom ESLint rule to the eslint-factory plugin to prevent unsafe direct property access (.message, .stack, .code) on caught exceptions when the thrown value might not be an Error, with a suggestion to replace .message usage with getErrorMessage(err).
Changes:
- Implements
no-unsafe-catch-error-propertywith a catch-stack approach that defers reporting untilCatchClause:exitonce guard detection is complete. - Adds a dedicated Vitest/RuleTester suite covering valid/invalid patterns, guard handling, nested catch independence, and suggestion output.
- Registers and enables the rule in the plugin index and the flat ESLint config at
warnlevel.
Show a summary per file
| File | Description |
|---|---|
| eslint-factory/src/rules/no-unsafe-catch-error-property.ts | New rule implementation for guarded vs unguarded caught-error property access, with suggestions for .message. |
| eslint-factory/src/rules/no-unsafe-catch-error-property.test.ts | Test suite for rule correctness, nested catch behavior, and suggestion outputs. |
| eslint-factory/src/index.ts | Registers the new rule in the exported plugin rules map. |
| eslint-factory/eslint.config.cjs | Enables the new rule for .cjs files at warning level. |
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
Comment on lines
+83
to
+92
| // Detect catchVar instanceof Error — also accepted as a safe guard | ||
| BinaryExpression(node) { | ||
| if (catchStack.length === 0) return; | ||
| const top = catchStack[catchStack.length - 1]; | ||
| if (!top || top.hasGuard || !top.varName) return; | ||
|
|
||
| if (node.operator === "instanceof" && node.left.type === AST_NODE_TYPES.Identifier && node.left.name === top.varName) { | ||
| top.hasGuard = true; | ||
| } | ||
| }, |
Comment on lines
+18
to
+20
| docs: { | ||
| description: "Disallow direct access to .message, .stack, or .code on a caught error variable without a getErrorMessage guard", | ||
| }, |
Comment on lines
+22
to
+25
| messages: { | ||
| unsafeProperty: "Direct access to .{{prop}} on caught error '{{errorVar}}' is unsafe — the thrown value may not be an Error instance. Use getErrorMessage({{errorVar}}) from error_helpers.cjs instead.", | ||
| useGetErrorMessage: "Replace with getErrorMessage({{errorVar}}) from error_helpers.cjs for safe error message extraction.", | ||
| }, |
Comment on lines
+38
to
+60
| it("valid: instanceof Error guard suppresses all warnings in the catch block", () => { | ||
| cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { | ||
| valid: [`try { f(); } catch (err) { core.setFailed(err instanceof Error ? err.message : String(err)); }`, `try { f(); } catch (err) { if (err instanceof Error) { console.log(err.stack); } }`], | ||
| invalid: [], | ||
| }); | ||
| }); | ||
|
|
||
| it("valid: property access on a different variable is not flagged", () => { | ||
| cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { | ||
| valid: [`try { f(); } catch (err) { console.log(otherObj.message); }`, `try { f(); } catch (err) { const e = new Error(); console.log(e.message); }`], | ||
| invalid: [], | ||
| }); | ||
| }); | ||
|
|
||
| it("valid: computed property access on catch variable is not flagged", () => { | ||
| cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { | ||
| valid: [`try { f(); } catch (err) { console.log(err["message"]); }`, `try { f(); } catch (err) { const prop = "message"; console.log(err[prop]); }`], | ||
| invalid: [], | ||
| }); | ||
| }); | ||
|
|
||
| it("invalid: err.message without guard is flagged", () => { | ||
| cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { |
| }); | ||
| }); | ||
|
|
||
| it("valid: nested try/catch — inner guard does not affect the outer catch block", () => { |
Contributor
|
🎉 This pull request is included in a new release. Release: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a new ESLint rule that flags direct access to
.message,.stack, or.codeon a caught error variable when no safe guard is present — catching the silent-undefinedbug that occurs when non-Errorvalues are thrown.Rule design
getErrorMessage(err)call (preferred) orerr instanceof Errorcheck anywhere in the same catch block.messageonly): auto-fix togetErrorMessage(err)fromerror_helpers.cjscatch {}and destructuring bindings are ignored; nested catch blocks are evaluated independentlyChanges
eslint-factory/src/rules/no-unsafe-catch-error-property.ts— rule implementation using aCatchClause-stack visitor pattern; collects unsafe nodes during traversal and reports atCatchClause:exitafter guard detection is completeeslint-factory/src/rules/no-unsafe-catch-error-property.test.ts— 14 tests covering valid/invalid patterns, both guard types, nested try/catch independence, and suggestion outputeslint-factory/src/index.ts— registers the rule in the plugineslint-factory/eslint.config.cjs— enables atwarnlevel