Skip to content

Commit 984ac9c

Browse files
Amnish04nzakas
authored andcommitted
feat: add preserve-caught-error rule (#19913)
* Write documentation for `preserve-caught-error` rule * Remove unsupported headings from docs * Setup initial unit test cases * Implement `preserve-caught-error` rule * Update messageIds in tests * Fix incorrect message id in one of the tests * Handle multiple throw statements and mandate looking at the caught error * Allow passing an option for custom error types * Remove unnecessary comments * Add another invalid case to docs * Remove MDN link from `further_reading` in docs * Add "Options" section to docs * Fix incorrect formatting * Do not use bare urls in docs * Add `proposal-error-cause` to `further_reading` * Add further reading links metadata * Improve error message for when further links metadata is missing * Add type definition for `preserve-caught-error` * Auto-generate tsdoc comment * Mark `preserve-caught-error` as recommended * Add `preserve-caught-error` to recommended config * Update rule tsdoc * Only use file path in further reading links error message * Only enable the rule in `eslint-plugin-eslint` for now * Fix edge cases and add tests * Fix formatting * Pretty format test cases with consistent indents between `code` and `output` * Get rid of manual throw statement search * Add back lost comment * Remove `customErrorTypes` option for now * Fix issues caught by `preserve-caught-error` * Require function calls with throws to use the caught error * Fix `createExtraneousResultsError` to accept a `cause` error * Add error creating functions case to docs * Handle autofix case where there is no message argument * Fix formatting * Handle `AggregateError` arg positioning exception * Fix `AggregateError` suggestion * Fix AggregateError test case * Revert "Require function calls with throws to use the caught error" This reverts commit afbda4f. * Add rule configuration comments to examples * Do not report/analyze complex error options * Preserve existing options when adding cause using fixer * Add test cases to avoid false positives, and existing options preservation * Fix formatting * Remove error factory case from docs * Update docs based on reviews * Don't report cases where throw is not related to caught error * Add `disallowUncaughtErrors` option and autofix for discarded errors * Fix rule type * Rename `disallowUncaughtErrors` to `requireCatchParameter` * Update the docs and use triple-colon fences * Improve examples in docs * Fix custom error false positives and unrelated-error fixer * Update TODO comment to limitation * Start reporting partially lost caught error * Report cases where caught error is shadowed by local declarations * Do not provide a suggestion on partially lost error * Do not provide suggestion when caught error is not referenced * Highlight throw statements individually when catch param is missing * Ignore cases with `SpreadElement` constructor arguments * Make sure existing comments are preserved with suggested fixes * Cleanup and follow up fixes * Handle value and method shorthand edge cases * Fix comment typo
1 parent 4a5a02a commit 984ac9c

File tree

13 files changed

+1484
-7
lines changed

13 files changed

+1484
-7
lines changed

docs/.eleventy.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,9 @@ module.exports = function (eleventyConfig) {
307307
const urlData = this.ctx.further_reading_links[url];
308308

309309
if (!urlData) {
310-
throw new Error(`Data missing for ${url}`);
310+
throw new Error(
311+
`Data missing for "${url}". Did you forget to add the URL information to "/docs/src/_data/further_reading_links.json"?`,
312+
);
311313
}
312314

313315
const { domain, title, logo } = urlData;

docs/src/_data/further_reading_links.json

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -796,5 +796,40 @@
796796
"logo": "https://tc39.es/ecma262/2020/img/favicon.ico",
797797
"title": "ECMAScript® 2020 Language Specification",
798798
"description": null
799+
},
800+
"https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause": {
801+
"domain": "developer.mozilla.org",
802+
"url": "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause",
803+
"logo": "https://developer.mozilla.org/favicon-192x192.png",
804+
"title": "MDN Web Docs",
805+
"description": "MDN docs for the cause data property of an Error instance."
806+
},
807+
"https://nodejs.org/api/errors.html#errorcause": {
808+
"domain": "nodejs.org",
809+
"url": "https://nodejs.org/api/errors.html#errorcause",
810+
"logo": "https://nodejs.org/favicon.ico",
811+
"title": "Errors | Node.js v24.3.0 Documentation",
812+
"description": "Official Node.js documentation for the Error `cause` property."
813+
},
814+
"https://github.com/tc39/proposal-error-cause/blob/main/README.md": {
815+
"domain": "github.com",
816+
"url": "https://github.com/tc39/proposal-error-cause/blob/main/README.md",
817+
"logo": "https://github.githubassets.com/favicons/favicon.png",
818+
"title": "proposal-error-cause/README.md at main · tc39/proposal-error-cause",
819+
"description": "Proposal for Error Cause to the Ecma International Technical Committee."
820+
},
821+
"https://dev.to/amnish04/never-lose-valuable-error-context-in-javascript-3aco": {
822+
"domain": "dev.to",
823+
"url": "https://dev.to/amnish04/never-lose-valuable-error-context-in-javascript-3aco",
824+
"logo": "https://media2.dev.to/dynamic/image/quality=100/https://dev-to-uploads.s3.amazonaws.com/uploads/logos/resized_logo_UQww2soKuUsjaOGNB38o.png",
825+
"title": "Never lose valuable error context in JavaScript - DEV Community",
826+
"description": "My blog post discussing the importance of Error `cause` property and the ESLint rule to enforce proper usage of it."
827+
},
828+
"https://github.com/microsoft/TypeScript/blob/main/src/lib/es2022.error.d.ts": {
829+
"domain": "github.com",
830+
"url": "https://github.com/microsoft/TypeScript/blob/main/src/lib/es2022.error.d.ts",
831+
"logo": "https://github.githubassets.com/favicons/favicon.png",
832+
"title": "Error types that support `cause`",
833+
"description": "Interface declarations for all the Error types in JavaScript that support passing a `cause` property."
799834
}
800-
}
835+
}
Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
---
2+
title: preserve-caught-error
3+
rule_type: suggestion
4+
further_reading:
5+
- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause
6+
- https://nodejs.org/api/errors.html#errorcause
7+
- https://github.com/tc39/proposal-error-cause/blob/main/README.md
8+
- https://dev.to/amnish04/never-lose-valuable-error-context-in-javascript-3aco
9+
- https://github.com/microsoft/TypeScript/blob/main/src/lib/es2022.error.d.ts
10+
---
11+
12+
JavaScript developers often re-throw errors in `catch` blocks to add context but forget to preserve the original error, resulting in lost debugging information.
13+
14+
Using the `cause` option when throwing new errors helps retain the original error and maintain complete error chains, which improves debuggability and traceability.
15+
16+
```js
17+
try {
18+
await fetch("https://xyz.com/resource");
19+
} catch(error) {
20+
// Throw a more specific error without losing original context
21+
throw new Error("Failed to fetch resource", {
22+
cause: error
23+
});
24+
}
25+
```
26+
27+
## Rule Details
28+
29+
This rule enforces the use of the `cause` property when throwing a new error inside a `catch` block.
30+
31+
Checks for all built-in `error types` that support passing a `cause`.
32+
33+
Examples of **incorrect** code for this rule:
34+
35+
::: incorrect
36+
37+
```js
38+
/* eslint preserve-caught-error: "error" */
39+
40+
// Not using the `cause` option
41+
try {
42+
// ...
43+
} catch (error) {
44+
throw new Error("Something went wrong: " + error.message);
45+
}
46+
47+
// Throwing a new Error with unrelated cause
48+
try {
49+
doSomething();
50+
} catch (err) {
51+
const unrelated = new Error("other");
52+
throw new Error("Something failed", { cause: unrelated });
53+
}
54+
55+
// Caught error is being lost partially due to destructuring
56+
try {
57+
doSomething();
58+
} catch ({ message, ...rest }) {
59+
throw new Error(message);
60+
}
61+
62+
// Cause error is being shadowed by a closer scoped redeclaration.
63+
try {
64+
doSomething();
65+
} catch (error) {
66+
if (whatever) {
67+
const error = anotherError; // This declaration is the problem.
68+
throw new Error("Something went wrong", { cause: error });
69+
}
70+
}
71+
```
72+
73+
:::
74+
75+
Examples of **correct** code for this rule:
76+
77+
::: correct
78+
79+
```js
80+
/* eslint preserve-caught-error: "error" */
81+
82+
try {
83+
// ...
84+
} catch (error) {
85+
throw new Error("Something went wrong", { cause: error });
86+
}
87+
88+
// When the thrown error is not directly related to the caught error.
89+
try {
90+
} catch (error) {
91+
foo = {
92+
bar() {
93+
// This throw is not directly related to the caught error.
94+
throw new Error("Something went wrong");
95+
}
96+
};
97+
}
98+
99+
// No throw inside catch
100+
try {
101+
doSomething();
102+
} catch (e) {
103+
console.error(e);
104+
}
105+
106+
// Ignoring the caught error at the parameter level
107+
// This is valid by default, but this behavior can be changed
108+
// by using the `requireCatchParameter` option discussed below.
109+
try {
110+
doSomething();
111+
} catch {
112+
throw new TypeError("Something went wrong");
113+
}
114+
```
115+
116+
:::
117+
118+
## Options
119+
120+
This rule takes a single option — an object with the following optional property:
121+
122+
- `requireCatchParameter`: Requires the catch blocks to always have the caught error parameter when set to `true`. By default, this is `false`.
123+
124+
### requireCatchParameter
125+
126+
Enabling this option mandates for all the catch blocks to have a caught error parameter. This makes sure that the caught error is not discarded at the parameter level.
127+
128+
```js
129+
"preserve-caught-error": ["error", {
130+
"requireCatchParameter": true
131+
}]
132+
```
133+
134+
Example of **incorrect** code for the `{ "requireCatchParameter": true }` option:
135+
136+
::: incorrect
137+
138+
```js
139+
/* eslint preserve-caught-error: ["error", { "requireCatchParameter": true }] */
140+
141+
try {
142+
doSomething();
143+
} catch { // Can't discard the error ❌
144+
throw new Error("Something went wrong");
145+
}
146+
```
147+
148+
:::
149+
150+
Example of **correct** code for the `{ "requireCatchParameter": true }` option:
151+
152+
::: correct
153+
154+
```js
155+
/* eslint preserve-caught-error: ["error", { "requireCatchParameter": true }] */
156+
157+
try {
158+
doSomething();
159+
} catch(error) { // Error is being referenced ✅
160+
// Handling and re-throw logic
161+
}
162+
```
163+
164+
:::
165+
166+
## When Not To Use It
167+
168+
You might not want to enable this rule if:
169+
170+
- You follow a custom error-handling approach where the original error is intentionally omitted from re-thrown errors (e.g., to avoid exposing internal details or to log the original error separately).
171+
172+
- You use a third-party or internal error-handling library that preserves error context using non-standard properties (e.g., [verror](https://www.npmjs.com/package/verror)) instead of the cause option.
173+
174+
- (In rare cases) you are targeting legacy environments where the cause option in `Error` constructors is not supported.

lib/eslint/eslint.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,11 +263,15 @@ async function locateConfigFileToUse({ configFile, cwd }) {
263263

264264
/**
265265
* Creates an error to be thrown when an array of results passed to `getRulesMetaForResults` was not created by the current engine.
266+
* @param {Error|undefined} cause The original error that led to this symptom error being thrown. Might not always be available.
266267
* @returns {TypeError} An error object.
267268
*/
268-
function createExtraneousResultsError() {
269+
function createExtraneousResultsError(cause) {
269270
return new TypeError(
270271
"Results object was not created from this ESLint instance.",
272+
{
273+
cause,
274+
},
271275
);
272276
}
273277

@@ -773,8 +777,8 @@ class ESLint {
773777
try {
774778
configs =
775779
configLoader.getCachedConfigArrayForFile(filePath);
776-
} catch {
777-
throw createExtraneousResultsError();
780+
} catch (err) {
781+
throw createExtraneousResultsError(err);
778782
}
779783

780784
const config = configs.getConfig(filePath);

lib/linter/esquery.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,9 @@ function tryParseSelector(selector) {
271271
) {
272272
throw new SyntaxError(
273273
`Syntax error in selector "${selector}" at position ${err.location.start.offset}: ${err.message}`,
274+
{
275+
cause: err,
276+
},
274277
);
275278
}
276279
throw err;

lib/rule-tester/rule-tester.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,9 @@ class RuleTester {
851851
} catch (err) {
852852
throw new Error(
853853
`Schema for rule ${ruleName} is invalid: ${err.message}`,
854+
{
855+
cause: err,
856+
},
854857
);
855858
}
856859
}

lib/rules/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ module.exports = new LazyLoadingRuleMap(
293293
"prefer-rest-params": () => require("./prefer-rest-params"),
294294
"prefer-spread": () => require("./prefer-spread"),
295295
"prefer-template": () => require("./prefer-template"),
296+
"preserve-caught-error": () => require("./preserve-caught-error"),
296297
"quote-props": () => require("./quote-props"),
297298
quotes: () => require("./quotes"),
298299
radix: () => require("./radix"),

0 commit comments

Comments
 (0)