feat(adapter/server): new Bunny.net Edge Scripting#179
feat(adapter/server): new Bunny.net Edge Scripting#179
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Bunny.net WebSocket support: new Bunny adapter and types, server plugin and serve wrapper, package exports and build config updates, documentation, and a basic test. New adapter entrypoints and WSOptions.bunny configuration were introduced. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/2.adapters/bunny.md`:
- Around line 75-77: The docs reference to test/fixture/bunny.ts is broken;
either add a demo fixture file matching that path (test/fixture/bunny.ts) that
demonstrates the adapter usage or remove/update the ::read-more block in
docs/2.adapters/bunny.md so it only links to src/adapters/bunny.ts (and/or to an
existing demo file); ensure the docs no longer point to the nonexistent
test/fixture/bunny.ts link.
In `@src/adapters/bunny.ts`:
- Around line 96-99: The error handler for the Bunny socket is passing the
entire ErrorEvent object into WSError; update the
socket.addEventListener("error", (error) => {...}) callback to extract and pass
the underlying error (use event.error) into new WSError so you preserve real
error details—i.e., change the parameter name to event (or keep error but
reference error.error) and call peers.delete(peer); hooks.callHook("error",
peer, new WSError(event.error)); to mirror the Cloudflare adapter's behavior.
🧹 Nitpick comments (1)
src/adapters/bunny.ts (1)
22-30: JSDoc says@default 30but no default is applied in code.The JSDoc documents
@default 30(seconds), but the adapter code (lines 62-64) only passesidleTimeoutthrough when explicitly set — it never applies a default of30. This is fine if the intent is to document Bunny's platform default, but readers may expect the adapter itself to enforce this default. Consider clarifying the JSDoc (e.g., "Defaults to30on the Bunny platform") to avoid ambiguity.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/adapters/bunny.ts`:
- Around line 51-55: The handleUpgrade function should destructure
upgradeHeaders from the result of hooks.upgrade() so custom headers aren’t
discarded; update the call in handleUpgrade to pull { endResponse, context,
namespace, upgradeHeaders } from hooks.upgrade(), then apply those headers when
completing the WebSocket upgrade via the platform API (or, if Bunny’s
upgradeWebSocket does not accept custom response headers, add a clear
comment/doc note in handleUpgrade explaining the limitation and why
upgradeHeaders cannot be applied). Ensure you reference handleUpgrade and
hooks.upgrade() and, if using the request.upgradeWebSocket API, attempt to pass
upgradeHeaders there or document the incompatibility.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/adapters/bunny.ts`:
- Around line 106-110: Add a peers.delete(peer) call to the Bunny adapter's
socket "error" event handler to avoid leaking peers when an "error" fires
without a subsequent "close"; specifically, in the error listener where
hooks.callHook("error", peer, new WSError(error)) is invoked, first call
peers.delete(peer) (it's idempotent so safe to also keep the existing delete in
the "close" handler) so the peer is removed from the peers Set regardless of
whether "close" follows.
🧹 Nitpick comments (1)
src/adapters/bunny.ts (1)
58-78:upgradeHeadersis only consumed for protocol negotiation; all other headers are silently discarded.You destructure
upgradeHeaders(good — addresses the prior review), but the only value extracted from them issec-websocket-protocol(line 67). Any other response headers theupgradehook returns are lost because they're never attached to theresponseobject returned byupgradeWebSocket. If this is a Bunny platform limitation, a brief comment would help future maintainers understand why.
sandros94
changed the title
2 participants
Discussed privately, for Bunny.net Edge Scripting
Summary by CodeRabbit
New Features
Documentation
Exports
Tests