Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 21 additions & 0 deletions eslint-factory/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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`.
Expand Down
116 changes: 116 additions & 0 deletions eslint-factory/src/rules/require-fetch-try-catch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down
79 changes: 69 additions & 10 deletions eslint-factory/src/rules/require-fetch-try-catch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>([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;
Comment on lines +43 to +44

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit fix(require-fetch-try-catch): unwrap ChainExpression and validate rejection handler callability. The traversal loop now checks for AST_NODE_TYPES.ChainExpression and unwraps it to current.expression before continuing, so await fetch(url)?.then(ok) (and any other optional-chain form) is correctly walked back to the root fetch(...) call. Added await fetch(url)?.then(x => x.json()) as an invalid test case and await fetch(url)?.catch(handler) / await fetch(url)?.then(ok, onErr) as valid ones.


// 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({
Expand Down Expand Up @@ -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);
Expand Down