Skip to content

Prefer explicit method option in fetch errors#583

Open
quyentonndbs wants to merge 1 commit into
unjs:mainfrom
quyentonndbs:maint/20260517074150
Open

Prefer explicit method option in fetch errors#583
quyentonndbs wants to merge 1 commit into
unjs:mainfrom
quyentonndbs:maint/20260517074150

Conversation

@quyentonndbs
Copy link
Copy Markdown

@quyentonndbs quyentonndbs commented May 17, 2026

When a Request object is passed with an overriding method option, the generated FetchError should report the effective method option.

This changes the error message method precedence and adds coverage for the Request plus method override case.

Verification:

  • pnpm vitest run test/index.test.ts

Summary by CodeRabbit

  • Bug Fixes

    • Fixed HTTP method resolution to prioritize fetch options when both a request object and fetch options containing different methods are provided.
  • Tests

    • Added test coverage to verify correct HTTP method precedence in error handling when multiple method specifications are present.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b8fc970b-c03c-4b41-9862-d6c9933ca8f7

📥 Commits

Reviewing files that changed from the base of the PR and between dfbe3ca and 186747e.

📒 Files selected for processing (2)
  • src/error.ts
  • test/index.test.ts

📝 Walkthrough

Walkthrough

The PR changes HTTP method resolution priority in createFetchError to prefer ctx.options?.method over ctx.request.method. A new test verifies this behavior when both Request and fetch options provide different HTTP methods.

Changes

Method Resolution Precedence

Layer / File(s) Summary
Method resolution precedence change
src/error.ts, test/index.test.ts
createFetchError now prioritizes ctx.options?.method over ctx.request.method for resolving the HTTP method. A test case validates that when both Request and fetch options provide different methods, the error correctly reflects and exposes the options method.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A method by any other name,
Now prefers the options claim,
Over request's prior state—
Small change, but handles it right! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Prefer explicit method option in fetch errors' directly and accurately describes the main change: adjusting error handling to prioritize the explicit method option over the request method when both are present.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant