diff --git a/eslint-factory/README.md b/eslint-factory/README.md index b7c27f069e6..62a74d6a65f 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -32,6 +32,7 @@ This project hosts custom ESLint linters for `/actions/setup/js`. | [`require-async-entrypoint-catch`](#require-async-entrypoint-catch) | Require `.catch(...)` on bare async entrypoint calls | | [`require-await-core-summary-write`](#require-await-core-summary-write) | Require `await` on `core.summary.write()` calls | | [`require-error-cause-in-rethrow`](#require-error-cause-in-rethrow) | Require `{ cause: err }` when rethrowing inside a `catch` block | +| [`require-fetch-try-catch`](#require-fetch-try-catch) | Require try/catch around awaited `fetch(...)` calls, including chained promise forms without rejection handlers | | [`require-fs-io-try-catch`](#require-fs-io-try-catch) | Require try/catch around `fs.statSync`, `readdirSync`, `copyFileSync`, `unlinkSync`, and `renameSync` | | [`require-fs-sync-try-catch`](#require-fs-sync-try-catch) | Require try/catch around `fs.readFileSync`, `writeFileSync`, and `appendFileSync` | | [`require-json-parse-try-catch`](#require-json-parse-try-catch) | Require try/catch around `JSON.parse(...)` calls | @@ -219,6 +220,26 @@ Flagged form: Safe alternative: - `throw new Error(\`failed: ${getErrorMessage(err)}\`, { cause: err });` +### `require-fetch-try-catch` + +Require awaited `fetch(...)` calls to be wrapped in `try/catch`, including member-chained promise forms rooted in `fetch(...)`. + +Why: `fetch` rejects with `TypeError` on network failures (DNS errors, connection refused, timeouts surfaced as aborts, etc.). Without either an enclosing `try/catch` or an explicit promise rejection handler, the action crashes with an unhelpful uncaught exception. + +**Flagged forms:** +- `await fetch(url);` +- `await fetch(url).then(res => res.json());` +- `await fetch(url).then(ok).finally(cleanup);` + +**Not flagged:** +- `try { await fetch(url).then(res => res.json()); } catch (err) {}` +- `await fetch(url).catch(handleFetchError);` +- `await fetch(url).then(onFulfilled, onRejected);` + +**Out of scope:** +- locally shadowed `fetch` bindings such as `async function f(fetch) { await fetch(url); }` +- named-reference rejection handlers are not inspected for correctness; the rule only checks that `.catch(handler)` or `.then(ok, onErr)` is present on the awaited fetch chain + ### `require-fs-io-try-catch` Require `fs.statSync`, `fs.readdirSync`, `fs.copyFileSync`, `fs.unlinkSync`, and `fs.renameSync` calls to be wrapped in `try/catch`. diff --git a/eslint-factory/src/rules/require-fetch-try-catch.test.ts b/eslint-factory/src/rules/require-fetch-try-catch.test.ts index f33c32e594d..9fed9d6c6a5 100644 --- a/eslint-factory/src/rules/require-fetch-try-catch.test.ts +++ b/eslint-factory/src/rules/require-fetch-try-catch.test.ts @@ -101,6 +101,122 @@ describe("require-fetch-try-catch", () => { }); }); + it("valid: chained fetch calls with rejection handlers are not flagged", () => { + cjsRuleTester.run("require-fetch-try-catch", requireFetchTryCatchRule, { + valid: [ + `async function f() { await fetch(url).catch(handler); }`, + `async function f() { await fetch(url).then(ok, onErr); }`, + `async function f() { + const res = await fetch(url, { signal }).catch(rethrowAbortError); + return res.text(); + }`, + `async function f() { + try { + await fetch(url).then(x => x.json()); + } catch (e) {} + }`, + // Optional chain with callable rejection handler + `async function f() { await fetch(url)?.catch(handler); }`, + `async function f() { await fetch(url)?.then(ok, onErr); }`, + ], + invalid: [], + }); + }); + + it("invalid: chained fetch calls without rejection handlers are flagged", () => { + cjsRuleTester.run("require-fetch-try-catch", requireFetchTryCatchRule, { + valid: [], + invalid: [ + { + code: `async function f() { await fetch(url).then(x => x.json()); }`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `async function f() { try {\n await fetch(url).then(x => x.json());\n} catch (err) {\n // TODO: handle fetch network failure (TypeError on DNS/connection errors).\n throw new Error(\n "fetch failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }`, + }, + ], + }, + ], + }, + // Optional chain without rejection handler is flagged + { + code: `async function f() { await fetch(url)?.then(x => x.json()); }`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `async function f() { try {\n await fetch(url)?.then(x => x.json());\n} catch (err) {\n // TODO: handle fetch network failure (TypeError on DNS/connection errors).\n throw new Error(\n "fetch failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }`, + }, + ], + }, + ], + }, + // Non-callable rejection arguments do not suppress rejection + { + code: `async function f() { await fetch(url).catch(null); }`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `async function f() { try {\n await fetch(url).catch(null);\n} catch (err) {\n // TODO: handle fetch network failure (TypeError on DNS/connection errors).\n throw new Error(\n "fetch failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }`, + }, + ], + }, + ], + }, + { + code: `async function f() { await fetch(url).then(ok, null); }`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `async function f() { try {\n await fetch(url).then(ok, null);\n} catch (err) {\n // TODO: handle fetch network failure (TypeError on DNS/connection errors).\n throw new Error(\n "fetch failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }`, + }, + ], + }, + ], + }, + { + code: `async function f() { await fetch(url).then(ok, undefined); }`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `async function f() { try {\n await fetch(url).then(ok, undefined);\n} catch (err) {\n // TODO: handle fetch network failure (TypeError on DNS/connection errors).\n throw new Error(\n "fetch failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }`, + }, + ], + }, + ], + }, + { + code: `async function f() { await fetch(url).then(ok, 0); }`, + errors: [ + { + messageId: "requireTryCatch", + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `async function f() { try {\n await fetch(url).then(ok, 0);\n} catch (err) {\n // TODO: handle fetch network failure (TypeError on DNS/connection errors).\n throw new Error(\n "fetch failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} }`, + }, + ], + }, + ], + }, + ], + }); + }); + it("invalid: await fetch in loop outside try block", () => { cjsRuleTester.run("require-fetch-try-catch", requireFetchTryCatchRule, { valid: [], diff --git a/eslint-factory/src/rules/require-fetch-try-catch.ts b/eslint-factory/src/rules/require-fetch-try-catch.ts index 1d334960fe7..788efebd215 100644 --- a/eslint-factory/src/rules/require-fetch-try-catch.ts +++ b/eslint-factory/src/rules/require-fetch-try-catch.ts @@ -6,16 +6,73 @@ const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh /** Function node types that form an async boundary. */ const FUNCTION_BOUNDARY_TYPES = new Set([AST_NODE_TYPES.FunctionDeclaration, AST_NODE_TYPES.FunctionExpression, AST_NODE_TYPES.ArrowFunctionExpression]); +function getMemberPropertyName(node: TSESTree.MemberExpression): string | null { + const property = node.property; + if (!node.computed && property.type === AST_NODE_TYPES.Identifier) return property.name; + if (node.computed && property.type === AST_NODE_TYPES.Literal && typeof property.value === "string") return property.value; + return null; +} + /** - * Returns true when the node is an `await fetch(...)` expression (AwaitExpression wrapping - * a CallExpression whose callee is the global `fetch` identifier). + * Returns true when a call argument is statically non-callable. + * Promise rejection callbacks of `null`, `undefined`, any literal value, or spread elements + * are replaced by the default thrower and do NOT suppress rejection. */ -function isAwaitFetchCall(node: TSESTree.Node): node is TSESTree.AwaitExpression { - if (node.type !== AST_NODE_TYPES.AwaitExpression) return false; - const argument = node.argument; - if (argument.type !== AST_NODE_TYPES.CallExpression) return false; - const callee = argument.callee; - return callee.type === AST_NODE_TYPES.Identifier && callee.name === "fetch"; +function isStaticallyNonCallable(node: TSESTree.Expression | TSESTree.SpreadElement): boolean { + if (node.type === AST_NODE_TYPES.SpreadElement) return true; + if (node.type === AST_NODE_TYPES.Literal) return true; + if (node.type === AST_NODE_TYPES.Identifier && node.name === "undefined") return true; + return false; +} + +interface AwaitedFetchInfo { + fetchCall: TSESTree.CallExpression; + hasRejectionHandler: boolean; +} + +/** + * Returns info when the node is an awaited fetch call, including member-chained forms like + * `await fetch(url).then(...)` and whether the chain already carries a rejection handler. + */ +function getAwaitedFetchInfo(node: TSESTree.Node): AwaitedFetchInfo | null { + if (node.type !== AST_NODE_TYPES.AwaitExpression) return null; + + let current: TSESTree.Expression | TSESTree.Super = node.argument; + let hasRejectionHandler = false; + + while (true) { + if (current.type === AST_NODE_TYPES.Super) return null; + + // Unwrap optional chains: `fetch(url)?.then(ok)` is wrapped in a ChainExpression by Espree. + if (current.type === AST_NODE_TYPES.ChainExpression) { + current = current.expression; + continue; + } + + if (current.type === AST_NODE_TYPES.CallExpression) { + const callee = current.callee; + + if (callee.type === AST_NODE_TYPES.Identifier && callee.name === "fetch") { + return { fetchCall: current, hasRejectionHandler }; + } + + if (callee.type !== AST_NODE_TYPES.MemberExpression) return null; + + const methodName = getMemberPropertyName(callee); + if (methodName === "catch" && current.arguments.length >= 1 && !isStaticallyNonCallable(current.arguments[0])) hasRejectionHandler = true; + if (methodName === "then" && current.arguments.length >= 2 && !isStaticallyNonCallable(current.arguments[1])) hasRejectionHandler = true; + + current = callee.object; + continue; + } + + if (current.type === AST_NODE_TYPES.MemberExpression) { + current = current.object; + continue; + } + + return null; + } } export const requireFetchTryCatchRule = createRule({ @@ -83,12 +140,14 @@ export const requireFetchTryCatchRule = createRule({ return { AwaitExpression(node) { - if (!isAwaitFetchCall(node)) return; + const fetchInfo = getAwaitedFetchInfo(node); + if (!fetchInfo) return; // Skip when fetch is shadowed by a local binding (e.g. a parameter or import named fetch). if (hasLocalBinding(node, "fetch")) return; + if (fetchInfo.hasRejectionHandler) return; if (isInsideTryBlock(node)) return; - const fetchCall = node.argument as TSESTree.CallExpression; + const { fetchCall } = fetchInfo; const firstArg = fetchCall.arguments[0]; const urlText = firstArg !== undefined ? sourceCode.getText(firstArg as TSESTree.Node) : ""; const stmt = findEnclosingStatement(sourceCode, node);