Skip to content

feat(ws): allow optional HTTP handling in defineWebSocketHandler#1425

Merged
pi0 merged 10 commits into
mainfrom
feat/ws-http
Jul 2, 2026
Merged

feat(ws): allow optional HTTP handling in defineWebSocketHandler#1425
pi0 merged 10 commits into
mainfrom
feat/ws-http

Conversation

@pi0x

@pi0x pi0x commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

defineWebSocketHandler(hooks) currently returns 426 Upgrade Required for any request that isn't a WebSocket upgrade. This adds an optional second argument so the same route can serve both WebSocket upgrades and plain HTTP requests.

app.get("/_ws", defineWebSocketHandler(
  { message: (peer, message) => peer.send(message.text()) },
  () => "Send a WebSocket upgrade request to connect.",
));
  • WebSocket upgrade requests (Upgrade: websocket) always go to the crossws hooks.
  • Non-upgrade requests are served by the http handler when provided; otherwise the existing 426 response is returned (fully backward compatible).

Design notes

  • The http handler is a plain EventHandler passed as the second argument, keeping the call site terse.
  • It only handles the non-upgrade path — it does not replace the crossws upgrade hook for rejecting/customizing the handshake itself (documented in JSDoc).
  • Upgrade detection (upgrade: websocket header) mirrors crossws' own canonical check.

Changes

  • src/utils/ws.ts — optional http fallback handler + JSDoc examples.
  • test/ws.test.ts — regression tests for the HTTP fallback and for hooks still attaching on upgrade requests.
  • docs/2.utils/9.more.md — regenerated via automd (pnpm fmt).

Testing

  • pnpm vitest run test/ws.test.ts — 5/5 pass, types clean
  • pnpm lint — clean (lint + format + typecheck)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Updated the WebSocket guide and API docs with expanded instructions for handling non-upgrade HTTP requests on the same route.
    • Added clearer examples showing how to combine a playground page (plain HTTP) with a WebSocket endpoint.
  • New Features
    • WebSocket route handlers can now optionally respond to regular HTTP requests on the same path, while upgrade requests still go to the WebSocket hooks.
  • Tests
    • Added coverage for both non-upgrade HTTP handling and upgrade requests, including verification of upgrade behavior when an HTTP handler is also provided.

@pi0x
pi0x requested a review from pi0 as a code owner July 2, 2026 23:25
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 633fa93a-a25b-4e31-9bff-7db4f7c906b5

📥 Commits

Reviewing files that changed from the base of the PR and between b7051d2 and 7b12628.

📒 Files selected for processing (4)
  • .github/workflows/autofix.yml
  • docs/1.guide/901.advanced/2.websocket.md
  • docs/2.utils/9.more.md
  • examples/websocket.mjs
💤 Files with no reviewable changes (1)
  • .github/workflows/autofix.yml
✅ Files skipped from review due to trivial changes (1)
  • docs/2.utils/9.more.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/1.guide/901.advanced/2.websocket.md
  • examples/websocket.mjs

📝 Walkthrough

Walkthrough

Adds an optional http handler to defineWebSocketHandler, routes non-upgrade requests to it, and keeps upgrade requests on the websocket hook path. Updates tests, docs, and the websocket example to match the combined-route behavior.

Changes

WebSocket HTTP Fallback Support

Layer / File(s) Summary
Optional http handler and upgrade detection
src/utils/ws.ts
Adds a JSDoc example for defineWebSocket, updates defineWebSocketHandler to accept an optional http handler for non-upgrade requests, and introduces isWebSocketUpgrade to detect websocket upgrade requests from the request headers.
Tests for fallback and upgrade behavior
test/ws.test.ts
Adds tests that verify non-upgrade requests are served by the provided http handler and that upgrade-style requests still return 426 with crossws hooks attached when an http handler is also supplied.
Documentation and example updates
docs/1.guide/901.advanced/2.websocket.md, docs/2.utils/9.more.md, examples/websocket.mjs
Updates the websocket guide, utils docs, and example to document the combined HTTP/WebSocket route pattern, the default 426 Upgrade Required response, and the optional http handler on defineWebSocketHandler().

Estimated code review effort: 2 (Simple) | ~12 minutes

Possibly related PRs

  • h3js/h3#1242: Both PRs modify src/utils/ws.ts's defineWebSocketHandler behavior/signature around how upgrade requests are handled and how crossws hooks are attached.

Suggested labels: enhancement

Suggested reviewers: pi0

Poem

A rabbit hops through one shared gate,
HTTP or websockets—both feel great.
If upgrades come, the hooks still sing,
If not, plain HTML takes wing.
Tests and docs now join the show,
With one neat route, the burrow glows. 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding optional HTTP handling to defineWebSocketHandler for WebSocket routes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ws-http

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.

Non-upgrade (plain HTTP) requests to a WebSocket route return `426 Upgrade
Required` by default. Pass an `http` handler as the second argument to serve
those requests instead, letting the same route handle both WebSocket upgrades
and regular HTTP requests. Upgrade requests always go to the hooks.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
autofix-ci Bot and others added 7 commits July 2, 2026 23:28
The GitHub-style `> [!NOTE]` alert in the JSDoc was flattened by automd into a
single broken line. Use a plain "Note:" sentence instead.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Use the `html` util to serve a minimal self-contained playground for plain
HTTP requests, demonstrating the new optional `http` handler: the same route
serves the page and the WebSocket endpoint the page connects back to.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Fixes CI lint: the autofix-ci commit left the doc code blocks in a form that
`oxfmt --check` rejects. This is the idempotent `pnpm fmt` output.

Co-Authored-By: Claude Opus 4.8 <[email protected]>

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/2.utils/9.more.md`:
- Around line 94-98: The `defineWebSocketHandler` docs currently show the wrong
signature and suggest `http` is the only argument. Update the heading and any
nearby description to use `defineWebSocketHandler(hooks, http?)`, making it
clear that `hooks` is the required first parameter and `http` is optional. Keep
the explanation aligned with the actual API behavior in the
`defineWebSocketHandler` section.

In `@src/utils/ws.ts`:
- Around line 55-65: `defineWebSocketHandler` currently takes `http` as a
positional second argument, which conflicts with the repo’s options-object
guideline for multi-argument APIs. Update the public signature so the second
parameter is an options object (for example, an object containing `http`), then
adjust the `defineHandler`/`_webSocketHandler` implementation to read from that
options object while preserving the existing fallback behavior when
non-WebSocket requests are handled.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b22fdfcf-67c1-4cd2-a98e-64c383bacace

📥 Commits

Reviewing files that changed from the base of the PR and between ac0d6fe and ffa4f5e.

📒 Files selected for processing (4)
  • docs/1.guide/901.advanced/2.websocket.md
  • docs/2.utils/9.more.md
  • src/utils/ws.ts
  • test/ws.test.ts

Comment thread docs/2.utils/9.more.md
Comment thread src/utils/ws.ts

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (2)
examples/websocket.mjs (1)

2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Variable name ws reused for both the crossws plugin and the client-side WebSocket instance.

Line 2 imports the crossws plugin as ws; the embedded browser script (line 63) also declares const ws = new WebSocket(url). These live in separate execution contexts (server module vs. generated HTML string) so there's no actual bug, but the shared name can confuse readers skimming the example.

Also applies to: 63-63

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/websocket.mjs` at line 2, The identifier `ws` is being reused for
two different concepts in `examples/websocket.mjs`, which makes the example
harder to read. Rename the imported crossws plugin binding from `ws` to a more
explicit name in the module scope, and keep the browser-side `const ws = new
WebSocket(url)` as-is or rename that one for clarity; update all references tied
to the import so the `plugin` usage remains consistent.
docs/1.guide/901.advanced/2.websocket.md (1)

122-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Route path inconsistency in HTTP-fallback example.

The main "Full example" registers the combined route at / (line 47), but this snippet uses /_ws (line 130). Since the whole point of the section is to demonstrate the same combined-route pattern, using a consistent path avoids reader confusion.

✏️ Suggested tweak
 app.get(
-  "/_ws",
+  "/",
   defineWebSocketHandler(
     { message: (peer, message) => peer.send(message.text()) },
     () => "Send a WebSocket upgrade request to connect.",
   ),
 );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/1.guide/901.advanced/2.websocket.md` around lines 122 - 137, The
HTTP-fallback example uses a different route path than the earlier
combined-route example, which can confuse readers about the pattern being
demonstrated. Update the app.get call in this snippet to use the same route path
as the “Full example” so the defineWebSocketHandler usage stays consistent
across the section.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@docs/1.guide/901.advanced/2.websocket.md`:
- Around line 122-137: The HTTP-fallback example uses a different route path
than the earlier combined-route example, which can confuse readers about the
pattern being demonstrated. Update the app.get call in this snippet to use the
same route path as the “Full example” so the defineWebSocketHandler usage stays
consistent across the section.

In `@examples/websocket.mjs`:
- Line 2: The identifier `ws` is being reused for two different concepts in
`examples/websocket.mjs`, which makes the example harder to read. Rename the
imported crossws plugin binding from `ws` to a more explicit name in the module
scope, and keep the browser-side `const ws = new WebSocket(url)` as-is or rename
that one for clarity; update all references tied to the import so the `plugin`
usage remains consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: eef97a78-3b90-4662-8cc6-24f4608bdd4a

📥 Commits

Reviewing files that changed from the base of the PR and between ffa4f5e and b7051d2.

📒 Files selected for processing (2)
  • docs/1.guide/901.advanced/2.websocket.md
  • examples/websocket.mjs

`pnpm fmt` already runs `automd` (then `oxfmt` last), so the trailing
`pnpm automd` re-emitted embedded code blocks in a form that `oxfmt --check`
(run by `pnpm lint` in CI) rejects — causing autofix and CI to fight in a
loop. Removing it makes the bot's output match `pnpm fmt`.

Also restructure the WebSocket example to extract the playground HTML into a
top-level `const`, keeping the handler a trivial `() => playground`.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@pi0
pi0 merged commit debd797 into main Jul 2, 2026
7 of 9 checks passed
@pi0
pi0 deleted the feat/ws-http branch July 2, 2026 23:45
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.

2 participants