Skip to content

Conversation

ChmaraX
Copy link
Contributor

@ChmaraX ChmaraX commented Oct 22, 2025

What changed? Why was the change needed?

Fixes NV-6817

Screenshots

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

Summary by CodeRabbit

  • New Features
    • Implemented context-aware message routing for WebSocket connections, enabling selective message delivery based on context keys rather than broadcasting to all users. Includes feature flag support for gradual rollout.

@linear
Copy link

linear bot commented Oct 22, 2025

@netlify
Copy link

netlify bot commented Oct 22, 2025

Deploy Preview for dashboard-v2-novu-staging canceled.

Name Link
🔨 Latest commit 84cc6f6
🔍 Latest deploy log https://app.netlify.com/projects/dashboard-v2-novu-staging/deploys/68f8cd207de9d2000893daac

@github-actions github-actions bot changed the title feat(ws): add context logic to enterprise WS worker feat(ws): add context logic to enterprise WS worker fixes NV-6821 Oct 22, 2025
Comment on lines 323 to 363
private formatContextDisplay(contextKeys?: string[]): string {
if (contextKeys === undefined) {
return 'FF disabled';
}

if (contextKeys.length === 0) {
return 'no context';
}

return contextKeys.join(', ');
}

/**
* Check if feature flag is OFF (contextKeys is undefined)
*/
private isFeatureFlagOff(contextKeys?: string[]): boolean {
return contextKeys === undefined;
}

/**
* Broadcast message to all user connections (FF OFF behavior)
*/
private async broadcastToAllSockets(
userId: string,
event: string,
message: string,
sockets: WebSocket[]
): Promise<void> {
console.log(`Sending event ${event} to all ${sockets.length} socket(s) (FF disabled)`);

const sendPromises = sockets.map(async (ws) => {
try {
ws.send(message);
} catch (error) {
console.error(`Failed to send message to user ${userId}:`, error);
throw error;
}
});

await Promise.allSettled(sendPromises);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part can be deleted when we remove the feature flag

@ChmaraX ChmaraX requested a review from scopsy October 22, 2025 10:50
@ChmaraX
Copy link
Contributor Author

ChmaraX commented Oct 22, 2025

@coderabbitai summary

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

✅ Actions performed

Summary regeneration triggered.

@linear
Copy link

linear bot commented Oct 22, 2025

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

Walkthrough

This pull request implements context-aware message routing for WebSocket connections. ContextKeys are extracted from JWT payloads via middleware, propagated through handlers during WebSocket handshake, and stored in socket attachments. Messages can now be routed either to all connected sockets or selectively to sockets with matching context keys, controlled by feature flags.

Changes

Cohort / File(s) Change Summary
Type Definitions
enterprise/workers/socket/src/types/index.ts
Added optional contextKeys?: string[] property to IConnectionMetadata interface; extended IWebSocketRoom.sendToUser() method signature to accept optional contextKeys parameter.
WebSocket Room Core Logic
enterprise/workers/socket/src/durable-objects/websocket-room.ts
Implemented context-aware message routing: extracts X-Context-Keys header during handshake, persists to attachment, added feature-flag-driven branching in sendToUser() to broadcast or deliver contextually, introduced multiple private helpers (extractContextKeysFromHeader, isExactMatch, sendToMatchingContexts, deliverMessageToSocket, etc.) and diagnostic logging.
WebSocket Handler
enterprise/workers/socket/src/handlers/websocket.ts
Added contextKeys retrieval from request context in handleWebSocketUpgrade() and forwarded as X-Context-Keys header; extended handleSendMessage() to extract and pass contextKeys to stub.sendToUser().
Authentication Middleware
enterprise/workers/socket/src/middleware/auth.ts
Added extraction of contextKeys from JWT payload and storage in request context via context.set('contextKeys', contextKeys).

Sequence Diagram

sequenceDiagram
    participant Client
    participant AuthMW as Auth Middleware
    participant Handler as WebSocket Handler
    participant Handshake as WS Handshake
    participant Room as WebSocket Room (DO)
    participant Sockets as Connected Sockets

    Client->>AuthMW: Request with JWT
    AuthMW->>AuthMW: Extract contextKeys from JWT
    AuthMW->>Handler: Forward context with contextKeys

    Client->>Handler: WebSocket upgrade
    Handler->>Handler: Get contextKeys from context
    Handler->>Handshake: includeHeaders: X-Context-Keys

    Handshake->>Room: Establish connection
    Room->>Room: Extract X-Context-Keys header
    Room->>Room: Store contextKeys in attachment

    Client->>Handler: Send message
    Handler->>Handler: Extract contextKeys from body
    Handler->>Room: sendToUser(userId, event, data, contextKeys)

    alt contextKeys is undefined (Feature OFF)
        Room->>Sockets: Broadcast to all sockets
    else contextKeys is defined (Feature ON)
        Room->>Room: Match contextKeys on each socket
        Room->>Sockets: Deliver only to matching contexts
    end
    
    Sockets-->>Client: Message received
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~35 minutes

The changes span multiple layers (middleware, handler, core logic, types) with cohesive context propagation flow. While introducing new conditional routing logic and helpers, the patterns are consistent across files. Public API signature changes require validation but the scope is focused on a single feature.

Possibly related PRs

Suggested labels

@novu/ws, @novu/worker

Suggested reviewers

  • scopsy
  • LetItRock

Poem

🐰 A rabbit hops through WebSocket halls,
With context keys in every call,
No more broadcast to each ear—
Messages reach the right frontier! 🎯✨

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch nv-6821-updating-cloudflare-sockets-with-new-context-logic

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ccbe7cf and 1b5ba3a.

📒 Files selected for processing (4)
  • enterprise/workers/socket/src/durable-objects/websocket-room.ts (6 hunks)
  • enterprise/workers/socket/src/handlers/websocket.ts (4 hunks)
  • enterprise/workers/socket/src/middleware/auth.ts (2 hunks)
  • enterprise/workers/socket/src/types/index.ts (1 hunks)

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.

@ChmaraX ChmaraX merged commit 6814e73 into next Oct 22, 2025
25 of 26 checks passed
@ChmaraX ChmaraX deleted the nv-6821-updating-cloudflare-sockets-with-new-context-logic branch October 22, 2025 12:32
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