Skip to content

Handle chained awaited fetch calls in require-fetch-try-catch - #47963

Merged
pelikhan merged 3 commits into
mainfrom
copilot/require-fetch-try-catch-fix
Jul 25, 2026
Merged

Handle chained awaited fetch calls in require-fetch-try-catch#47963
pelikhan merged 3 commits into
mainfrom
copilot/require-fetch-try-catch-fix

Conversation

Copilot AI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

require-fetch-try-catch only recognized await fetch(...) when the awaited call target was the bare fetch identifier, so chained forms like await fetch(url).then(...) escaped detection. This change extends the rule to follow awaited fetch member chains while preserving the existing exemption for chains that already handle rejection.

  • Rule logic

    • Walk the awaited expression outward/inward through CallExpression / MemberExpression chains to recover the root fetch(...) call.
    • Treat chained promise forms rooted in fetch(...) as in scope, not just bare await fetch(...).
    • Skip reporting when the awaited chain already includes:
      • .catch(handler)
      • .then(onFulfilled, onRejected)
    • Preserve the existing try/catch exemption and local fetch shadowing behavior.
  • Coverage added

    • Invalid: await fetch(url).then(x => x.json())
    • Valid: await fetch(url).catch(handler)
    • Valid: await fetch(url).then(ok, onErr)
    • Valid: try { await fetch(url).then(...) } catch {}
  • Docs

    • Add require-fetch-try-catch to the eslint-factory rule index.
    • Document the chained-fetch scope, handled rejection cases, and explicit out-of-scope behavior.
// now flagged
async function f() {
  await fetch(url).then(res => res.json());
}

// still allowed
async function f() {
  await fetch(url).catch(handleFetchError);
  await fetch(url).then(onFulfilled, onRejected);
}

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix detection of chained await fetch calls Handle chained awaited fetch calls in require-fetch-try-catch Jul 25, 2026
Copilot AI requested a review from pelikhan July 25, 2026 09:08
@github-actions

Copy link
Copy Markdown
Contributor

Hey @Copilot 👋 — thanks for fixing the require-fetch-try-catch detection gap! This PR looks great:

Single, focused change — extends the rule to detect member-chained awaited fetch calls (await fetch(url).then(...)) while preserving the existing exemptions for .catch() and 2-arg .then(onF, onR) patterns.

Comprehensive test coverage — new test cases cover the flagged invalid pattern and the valid rejection-handled cases.

Clear documentation — the PR body and README updates explain the rule logic, scope, and acceptance criteria.

This PR is ready for review and merge. Well done! 🎉

Generated by ✅ Contribution Check · sonnet46 · 63.8 AIC · ⌖ 12.3 AIC · ⊞ 6.2K ·

@pelikhan
pelikhan marked this pull request as ready for review July 25, 2026 09:47
Copilot AI review requested due to automatic review settings July 25, 2026 09:47

Copilot AI left a comment

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.

Pull request overview

Extends require-fetch-try-catch to detect chained awaited fetch calls while exempting handled rejections.

Changes:

  • Traverses fetch member/call chains and detects rejection handlers.
  • Adds chained-fetch rule tests.
  • Documents the rule and supported forms.
Show a summary per file
File Description
eslint-factory/src/rules/require-fetch-try-catch.ts Adds chained-fetch detection.
eslint-factory/src/rules/require-fetch-try-catch.test.ts Tests chained promise behavior.
eslint-factory/README.md Documents rule scope and examples.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment on lines +31 to +32
while (true) {
if (current.type === AST_NODE_TYPES.Super) return null;

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.

Comment on lines +44 to +45
if (methodName === "catch" && current.arguments.length >= 1) hasRejectionHandler = true;
if (methodName === "then" && current.arguments.length >= 2) hasRejectionHandler = true;

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 the same commit. Added an isStaticallyNonCallable helper that returns true for null, undefined (identifier), any other literal, and spread elements. Both the .catch(arg) and .then(onFulfilled, onRejected) checks now gate on !isStaticallyNonCallable(arg), so .catch(null), .then(ok, null), .then(ok, undefined), and .then(ok, 0) are all flagged. Regression tests for each case added.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher skill

…ection handler callability

- Unwrap ChainExpression nodes in the traversal loop so optional chains
  like `await fetch(url)?.then(ok)` are correctly detected
- Add isStaticallyNonCallable helper that returns true for null, undefined,
  other literals, and spread elements — Promise treats these as the default
  thrower rather than a real rejection handler
- Apply isStaticallyNonCallable check to .catch() and .then(,) argument
  positions so .catch(null) and .then(ok, undefined) are flagged
- Add regression tests for optional-chain and non-callable rejection cases

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan
pelikhan merged commit 850fb9e into main Jul 25, 2026
@pelikhan
pelikhan deleted the copilot/require-fetch-try-catch-fix branch July 25, 2026 10:54
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, make sure the branch is up to date, and address any remaining review feedback or follow-up comments before rerunning checks.

Run: https://github.com/github/gh-aw/actions/runs/30155206984

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 9.98 AIC · ⌖ 8.65 AIC · ⊞ 7.1K ·
Comment /souschef to run again

@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.83.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

require-fetch-try-catch: member-chained await fetch(...).then()/.catch()/.<responseMethod>() escapes detection

4 participants