test(client): simulate network error for undefined route in parseResponse test by SambhavJ2004 · Pull Request #5439 · honojs/hono · GitHub
Skip to content

test(client): simulate network error for undefined route in parseResponse test - #5439

Merged
yusukebe merged 1 commit into
honojs:mainfrom
SambhavJ2004:test/client-noroute-network-error
Oct 1, 2026
Merged

yusukebe merged 1 commit into
honojs:mainfrom
SambhavJ2004:test/client-noroute-network-error

Conversation

@SambhavJ2004

@SambhavJ2004 SambhavJ2004 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #5431

Problem

The third case in parseResponse > should throw error matching snapshots
checks that when fetch itself rejects, parseResponse passes the original
error through unwrapped. It produced that rejection by requesting
http://localhost/noRoute, which has no MSW handler, so the request escaped
to the real network and relied on nothing listening on port 80.

On machines where something does listen on port 80 (in my case IIS 10.0 on
Windows), that request receives a 404 instead, and the test fails with:

  • Expected: [TypeError: fetch failed]
  • Received: [DetailedError: 404 Not Found]

Fix

Add an explicit MSW handler for that route that returns
HttpResponse.error(), so fetch rejects deterministically without the
request ever reaching the network. The comment above the case is updated to
explain this.

The snapshot message changes from fetch failed (Node's fetch, on a real
connection failure) to Failed to fetch (MSW's simulated network error).
It is still a TypeError thrown at the fetch call, which is what this
case verifies.

Verification

On Windows 11, with vitest 5.0.1 and msw 2.15.0 installed from the lockfile:

  • With IIS running on port 80: all 32 tests in src/client/utils.test.ts pass
  • With IIS stopped: all 32 tests pass
  • The MSW "intercepted a request without a matching request handler" warning
    no longer appears in either run

Before this change, the same file failed with IIS running and passed with
it stopped.

The author should do the following, if applicable

  • Add tests
  • Run tests
  • pnpm run format:fix && pnpm run lint:fix to format the code
  • Add TSDoc/JSDoc to document the code (not applicable, test-only change)

Copilot AI lite review requested due to automatic review settings September 24, 2026 13:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The reviewed test changes have no unresolved issues.

Review effort: Lite
Findings: None

What changed in this PR

Updates the parseResponse network-error test to deterministically simulate a failed fetch with MSW.

Changes:

  • Adds a network-error handler for the undefined route.
  • Updates the expected rejection snapshot.
File Description
src/​client/​utils.test.ts Adds explicit network-error test coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mhalikosen mhalikosen left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reproduced #5431 locally with a server on port 80 that answers /noRoute with a 404: main fails with [DetailedError: 404 Not Found], while this branch passes, and it also passes when the request to port 80 fails. HttpResponse.error() is MSW's own way to model a network failure, and the test still checks what it was written for: parseResponse passes a rejected fetch through unwrapped. utils.test.ts (32 tests), tsc -p tsconfig.spec.json, eslint and oxfmt pass locally.

The title got cut at parseResp… and the rest spilled into the body. The squash commit takes the title, so the full subject should be restored before merge.

@SambhavJ2004 SambhavJ2004 changed the title test(client): simulate network error for undefined route in parseResp… test(client): simulate network error for undefined route in parseResponse test Sep 24, 2026
@SambhavJ2004

Copy link
Copy Markdown
Contributor Author

Thanks for reproducing and reviewing! I've restored the full title.

@yusukebe yusukebe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@yusukebe

yusukebe commented Oct 1, 2026

Copy link
Copy Markdown
Member

@SambhavJ2004 Thanks!

@codecov

codecov Bot commented Oct 1, 2026

Copy link
Copy Markdown

@yusukebe
yusukebe merged commit deff529 into honojs:main Oct 1, 2026
19 checks passed
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.

parseResponse snapshot test fails when a server is listening on localhost:80

4 participants