Skip to content

eslint-factory: remove useless rethrow from require-json-parse-try-catch suggestion #42537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 6 commits into from
Jun 30, 2026
+41 −12
Merged

eslint-factory: remove useless rethrow from require-json-parse-try-catch suggestion #42537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
264bdeb
Initial plan
Copilot Jun 30, 2026
cfc8d13
fix(eslint-factory): avoid useless catch in JSON.parse suggestion
Copilot Jun 30, 2026
62aeaab
refactor(eslint-factory): make JSON.parse suggestion catch meaningful
Copilot Jun 30, 2026
2ab8d9a
chore: start PR finisher pass
Copilot Jun 30, 2026
d41e2b4
fix(eslint-factory): preserve JSON.parse error cause
Copilot Jun 30, 2026
7b79d80
Merge branch 'main' into copilot/eslint-factory-fix-require-json-pars…
pelikhan Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 26 additions & 11 deletions eslint-factory/src/rules/require-json-parse-try-catch.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try {\n const data = JSON.parse(rawInput);\n} catch (err) {\n throw err;\n}`,
output: `try {\n const data = JSON.parse(rawInput);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`,
},
],
},
Expand All @@ -59,7 +59,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try {\n JSON.parse(response.body);\n} catch (err) {\n throw err;\n}`,
output: `try {\n JSON.parse(response.body);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`,
},
],
},
Expand All @@ -82,7 +82,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try {\n const data = JSON.parse(rawInput);\n} catch (err) {\n throw err;\n}`,
output: `try {\n const data = JSON.parse(rawInput);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`,
},
],
},
Expand All @@ -104,7 +104,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try {\n const result = JSON.parse(text);\n} catch (err) {\n throw err;\n}`,
output: `try {\n const result = JSON.parse(text);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`,
},
],
},
Expand All @@ -127,7 +127,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try {\n const data = JSON["parse"](rawInput);\n} catch (err) {\n throw err;\n}`,
output: `try {\n const data = JSON["parse"](rawInput);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`,
},
],
},
Expand All @@ -142,7 +142,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try {\n JSON["parse"](response.body);\n} catch (err) {\n throw err;\n}`,
output: `try {\n JSON["parse"](response.body);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`,
},
],
},
Expand Down Expand Up @@ -181,7 +181,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try { emitter.on("data", chunk => { try {\n JSON.parse(chunk);\n} catch (err) {\n throw err;\n} }); } catch (e) {}`,
output: `try { emitter.on("data", chunk => { try {\n JSON.parse(chunk);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }); } catch (e) {}`,
},
],
},
Expand All @@ -197,7 +197,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try { promise.then(data => { try {\n const x = JSON.parse(data);\n} catch (err) {\n throw err;\n} }); } catch (e) {}`,
output: `try { promise.then(data => { try {\n const x = JSON.parse(data);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }); } catch (e) {}`,
},
],
},
Expand All @@ -213,7 +213,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try { setTimeout(() => { try {\n JSON.parse(raw);\n} catch (err) {\n throw err;\n} }, 100); } catch (e) {}`,
output: `try { setTimeout(() => { try {\n JSON.parse(raw);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }, 100); } catch (e) {}`,
},
],
},
Expand All @@ -229,7 +229,7 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try { new Promise(resolve => { try {\n JSON.parse(data);\n} catch (err) {\n throw err;\n} }); } catch (e) {}`,
output: `try { new Promise(resolve => { try {\n JSON.parse(data);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }); } catch (e) {}`,
},
],
},
Expand All @@ -245,7 +245,22 @@ describe("require-json-parse-try-catch", () => {
suggestions: [
{
messageId: "useHelper",
output: `try { process.nextTick(() => { try {\n JSON.parse(payload);\n} catch (err) {\n throw err;\n} }); } catch (e) {}`,
output: `try { process.nextTick(() => { try {\n JSON.parse(payload);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }); } catch (e) {}`,
},
],
},
],
},
{
code: `if (cond) {\n JSON.parse(raw);\n}`,
errors: [
{
messageId: "requireTryCatch",
data: { arg: "raw" },
suggestions: [
{
messageId: "useHelper",
output: `if (cond) {\n try {\n JSON.parse(raw);\n } catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n }\n}`,
},
],
},
Expand Down
16 changes: 15 additions & 1 deletion eslint-factory/src/rules/require-json-parse-try-catch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@ function isDeferredCallback(funcNode: TSESTree.Node): boolean {
return false;
}

function buildTryCatchSuggestion(stmtText: string, indent: string): string {
return [
"try {",
`${indent} ${stmtText}`,
`${indent}} catch (err) {`,
`${indent} // TODO: handle parse failure for this code path.`,
`${indent} throw new Error(`,

@github-actions github-actions Bot Jun 30, 2026

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.

The generated throw new Error(...) discards the original error's stack trace, which can make debugging parse failures harder.

Consider emitting { cause: err } to retain the original context:

`${indent}  throw new Error(`,
`${indent}    "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),`,
`${indent}    { cause: err },`,
`${indent}  );`,

This produces auto-generated scaffolding like:

catch (err) {
  // TODO: handle parse failure for this code path.
  throw new Error(
    "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),
    { cause: err },
  );
}

Error({ cause }) is supported in Node 16.9+ and is available in all environments this rule targets. Since the fix is auto-generated and applied broadly, preserving the original error as cause improves debuggability for all consumers who apply it. @copilot please address this.

`${indent} "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),`,

@github-actions github-actions Bot Jun 30, 2026

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.

Generated Error silently drops the original SyntaxError, making parse failures harder to debug.

💡 Suggested fix

The emitted catch body wraps err into a new Error but never passes it as cause, discarding the original SyntaxError stack trace. Anyone who applies this suggestion and later needs to diagnose why JSON failed to parse (e.g., unexpected token at position X) has no way to recover that information from the rethrown error.

Add { cause: err } as the second argument to new Error() in the generated string:

`${indent}  throw new Error(`,
`${indent}    "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),`,
`${indent}    { cause: err },`,   // <-- add this line in the generated output
`${indent}  );`,

This preserves the original SyntaxError as error.cause, which debuggers and structured logging can surface. Error.cause is available in Node ≥ 16.9 and all modern browsers.

`${indent} { cause: err },`,
`${indent} );`,
Comment on lines +64 to +69
`${indent}}`,
].join("\n");
}

export const requireJsonParseTryCatchRule = createRule({
name: "require-json-parse-try-catch",
meta: {
Expand Down Expand Up @@ -158,7 +172,7 @@ export const requireJsonParseTryCatchRule = createRule({
const startLine = stmt.loc?.start.line;
const stmtLine = startLine !== undefined ? (sourceCode.lines[startLine - 1] ?? "") : "";
const indent = stmtLine.match(/^(\s*)/)?.[1] ?? "";
return fixer.replaceText(stmt, `try {\n${indent} ${stmtText}\n${indent}} catch (err) {\n${indent} throw err;\n${indent}}`);
return fixer.replaceText(stmt, buildTryCatchSuggestion(stmtText, indent));
},
},
],
Expand Down
Loading