Fix critical publish() bug, address review findings
CRITICAL: createPubSub.publish() was dispatching CustomEvent with
just the event type (e.g. 'call.responded') instead of the composite
topic string ('call.responded:uuid-123'). This broke all adapters
that rely on topic-scoped dispatch — Redis subscribe/publish
channels didn't match, and WS server fan-out routing would fail.
Fixed to dispatch with the full type:id composite.
Other fixes:
- Add __ prefix runtime guard in publish() (reserved for control)
- Add Redis barrel re-export to src/index.ts (ADR-002 compliance)
- Clarify WS server: adapter's onclose calls removeConnection
internally; user doesn't need to
- WS client: document null callback no-op, removeEventListener
edge cases (unregistered callback, null callback)
- WS server: document dispatchEvent always returns true
- Redis spec: document in-flight message edge case after unsubscribe
- Worker adapter: rename createMainThreadEventTarget to
createWorkerThreadEventTarget, createWorkerEventTarget to
createWorkerHostEventTarget (fix inverted naming)
- api-surface.md: add PubSub.publish() section documenting the
type:id composite and __ guard
This commit is contained in:
@@ -54,9 +54,8 @@ const serverTarget = createWebSocketServerEventTarget({});
|
||||
app.get("/ws", (c) => {
|
||||
return c.upgrade(async (ws) => {
|
||||
serverTarget.addConnection(ws);
|
||||
ws.addEventListener("close", () => {
|
||||
serverTarget.removeConnection(ws);
|
||||
});
|
||||
// removeConnection is called automatically by the adapter's onclose handler.
|
||||
// Only call removeConnection manually for forced disconnections (backpressure, auth failures).
|
||||
});
|
||||
});
|
||||
```
|
||||
@@ -77,6 +76,8 @@ The server adapter exposes these methods on the returned `WebSocketServerEventTa
|
||||
|
||||
`removeConnection` cleans up internal state (subscription maps, event handlers) but does **not** close the `WebSocket`. The caller is responsible for closing the connection if needed. Typically it's called from a `close` event handler, where the connection is already closing.
|
||||
|
||||
**Note:** The adapter's `onclose` handler (set up by `addConnection`) calls `removeConnection` internally. This means the caller does not need to call `removeConnection` from their own `close` handler — the adapter handles cleanup automatically when a connection closes. The `removeConnection` method is exposed for cases where the caller needs to manually disconnect a connection (e.g., for backpressure or authentication failures).
|
||||
|
||||
### Error Handling
|
||||
|
||||
- **Malformed JSON from a spoke** → the message is silently ignored. The adapter logs a warning (via `console.warn` or a configurable logger) and continues processing other messages from that connection. The connection is not closed — a single malformed message should not disconnect a client.
|
||||
@@ -85,6 +86,8 @@ The server adapter exposes these methods on the returned `WebSocketServerEventTa
|
||||
- **Send failure** → if `ws.send()` throws (connection died between the `bufferedAmount` check and the send), the adapter catches the error, removes the connection from the subscription maps, and fires `onDisconnection`.
|
||||
- **`onclose` from client** → the adapter removes the connection from all subscription maps and fires `onDisconnection`.
|
||||
|
||||
- **`dispatchEvent` return value** — always returns `true`, regardless of subscriber count or send failures. Errors are handled via side effects (connection removal, `onDisconnection` callback), not via the return value. This matches the `EventTarget` contract where `return false` means `preventDefault` was called, not "send failed."
|
||||
|
||||
### Concurrency Model
|
||||
|
||||
This adapter assumes a single-threaded event loop (Node.js, Bun, Deno, browsers). In environments with worker threads, the caller must ensure `addConnection`/`removeConnection` and `dispatchEvent` are not called concurrently.
|
||||
|
||||
Reference in New Issue
Block a user