skip empty message #2352
skip empty message #2352
Conversation
📝 WalkthroughWalkthroughThe changes focus on improving WebSocket message handling in the Changes
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
node/pkg/wss/utils.go (3)
123-130: Consider adding goroutine management.While the empty message check is good, the goroutine handling could be improved. Consider:
- Adding a wait group to track in-flight goroutines during shutdown
- Implementing a timeout mechanism for long-running message processing
Example implementation:
func (ws *WebsocketHelper) Run(ctx context.Context, router func(context.Context, map[string]any) error) { + var wg sync.WaitGroup + defer wg.Wait() // Wait for in-flight goroutines during shutdown ... if len(data) != 0 { + wg.Add(1) go func(context.Context, map[string]any) { + defer wg.Done() + // Add timeout for long-running processing + ctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() routerErr := router(ctx, data) if routerErr != nil { log.Warn().Err(routerErr).Str("endpoint", ws.Endpoint).Msg("error processing websocket message") } }(ctx, data) }
234-238: Add debug logging for empty messages.Consider adding debug-level logging when empty messages are received to help with monitoring and debugging.
if len(data) == 0 { + log.Debug().Msg("received empty websocket message, skipping") return nil, nil }
234-238: Document the empty message handling behavior.Please add comments explaining:
- Why empty messages are skipped
- What constitutes an empty message (empty map vs. other forms)
- Any specific scenarios or providers where this handling is particularly important
func defaultReader(ctx context.Context, conn *websocket.Conn) (map[string]interface{}, error) { + // Read JSON message from the WebSocket connection var data map[string]interface{} err := wsjson.Read(ctx, conn, &data) if err != nil { return nil, err } + // Skip empty messages to prevent unnecessary processing + // Some WebSocket providers may send keep-alive messages as empty JSON objects if len(data) == 0 { return nil, nil }
Description
Please include a summary of the changes and the related issue.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Checklist before requesting a review
Deployment