fix: prevent double serialization of websocket text messages. (#6182)#6479
fix: prevent double serialization of websocket text messages. (#6182)#6479bijin-bruno merged 2 commits intomainfrom
Conversation
WalkthroughAdds format-aware message normalization and queueing to the WebSocket client; Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant IPC as IPC Handler
participant WsClient as WsClient
participant WS as WebSocket
IPC->>WsClient: queueMessage(requestId, collectionUid, message, format)
rect rgba(220,235,255,0.6)
Note right of WsClient: enqueue { message, format }
end
WsClient->>WsClient: flush() / dequeue item
WsClient->>WsClient: normalizeMessageByFormat(message, format)
Note right of WsClient: format ∈ { "json", "raw", "xml" }
WsClient->>WS: send(normalizedPayload)
WS-->>WsClient: ack / error
WsClient-->>IPC: emit outgoing event (payload, hexdump, meta)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-requests/src/ws/ws-client.js (1)
164-164: Hexdump should reflect the actual payload sent.The hexdump generation still uses
JSON.stringify(messageToSend)instead ofpayload, causing a mismatch for raw string messages. When sending the string"hello", the wire data ishellobut the hexdump shows"hello"(with quotes), which could confuse debugging efforts.🔎 Proposed fix to align hexdump with transmitted data
- messageHexdump: hexdump(JSON.stringify(messageToSend)), + messageHexdump: hexdump(payload),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-requests/src/ws/ws-client.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-requests/src/ws/ws-client.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-requests/src/ws/ws-client.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
🔇 Additional comments (1)
packages/bruno-requests/src/ws/ws-client.js (1)
153-157: Excellent fix for double serialization!The conditional payload assignment correctly prevents double serialization of text messages. Strings are now sent raw while objects are properly stringified before transmission. This aligns with standard WebSocket text message handling.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-requests/src/ws/ws-client.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-requests/src/ws/ws-client.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-requests/src/ws/ws-client.js
🧬 Code graph analysis (1)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (5)
packages/bruno-app/src/components/RequestPane/Auth/index.js (1)
request(35-37)packages/bruno-electron/src/ipc/network/prepare-request.js (1)
request(306-306)packages/bruno-app/src/components/RequestPane/WSRequestPane/WSAuth/index.js (1)
request(21-23)packages/bruno-electron/src/utils/collection.js (1)
getFormattedCollectionOauth2Credentials(606-616)packages/bruno-app/src/components/RequestPane/WsBody/SingleWSMessage/index.js (1)
message(41-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
🔇 Additional comments (7)
packages/bruno-electron/src/ipc/network/ws-event-handlers.js (2)
30-32: LGTM: Formatting improvements.These multiline formatting changes improve readability without altering behavior.
Also applies to: 44-46
337-337: LGTM: Message format correctly passed.The
forEachreceives the full message object, so accessing.contentand.typeis correct and aligns with the updatedqueueMessagesignature.packages/bruno-requests/src/ws/ws-client.js (5)
25-40: LGTM: Normalization prevents double serialization.The format-aware normalization correctly handles
jsonby checking if the message is already stringified (preventing double encoding), whilerawandxmlpass through unchanged. The default case safely handles objects vs. primitives.
73-78: LGTM: Formatting refinement.The protocols array construction is now more readable with the explicit array operations spread across multiple lines. Functionally equivalent to the previous version.
127-127: LGTM: Format parameter correctly threaded through queue.The
formatparameter with default'raw'is now stored alongside the message in the queue, enabling format-aware processing during flush. This aligns with the PR objective to prevent double serialization.Also applies to: 132-135
147-148: LGTM: Queue flush correctly extracts format.The destructuring extracts both
messageandformatfrom queued items and passes them tosendMessage, maintaining consistency with the new queue structure.
158-158: LGTM: sendMessage correctly applies normalization.The
sendMessagemethod now acceptsformat, appliesnormalizeMessageByFormatto compute the final payload, and consistently uses that payload for both sending and event emission. This ensures the hexdump reflects what was actually sent.Also applies to: 162-162, 165-165, 171-172
904cc0b to
914baf7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-requests/src/ws/ws-client.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-electron/src/ipc/network/ws-event-handlers.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-requests/src/ws/ws-client.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-requests/src/ws/ws-client.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Linux
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (4)
packages/bruno-requests/src/ws/ws-client.js (4)
127-135: LGTM!The queue structure refactor is clean and maintains backward compatibility with the default
'raw'format. The change from storing raw messages to{ message, format }objects properly supports format-aware message handling.
147-148: LGTM!Correctly destructures the new queue entry format and forwards both
messageandformattosendMessage.
158-184: Correct implementation of format-aware message sending.The method properly normalizes messages using
normalizeMessageByFormatbefore sending and consistently uses the computedpayloadfor both WebSocket transmission and event emission. This correctly addresses the double serialization issue.Note: The robustness of this method depends on the validation improvements suggested for
normalizeMessageByFormat.
73-78: LGTM!Formatting improvements enhance readability. The explicit
.flat()call clarifies the intent.
- Added normalization for message format to prevent double encoding. - Updated queueMessage and sendMessage methods to handle message format. - Refactored code for better readability and maintainability. fix: enhance message normalization in WebSocket client
944fc43 to
1636a72
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/bruno-requests/src/ws/ws-client.js (1)
25-50: Input validation concerns remain unaddressed.The validation issues raised in the previous review remain relevant: JSON strings aren't validated for correctness, raw/xml formats don't verify the message is a string, and the format parameter itself isn't validated. These gaps could lead to runtime errors or malformed data being sent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-electron/src/ipc/network/ws-event-handlers.jspackages/bruno-requests/src/ws/ws-client.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-electron/src/ipc/network/ws-event-handlers.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-requests/src/ws/ws-client.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-requests/src/ws/ws-client.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
🔇 Additional comments (4)
packages/bruno-requests/src/ws/ws-client.js (4)
83-88: LGTM!The protocol extraction logic is correctly formatted and maintains semantic equivalence with improved readability.
137-145: LGTM!The format parameter is properly added with a sensible default, and queue entries correctly store both message and format for downstream processing.
157-159: LGTM!Queue flushing correctly destructures and propagates the format parameter to sendMessage.
168-194: LGTM!The sendMessage method correctly integrates format-aware normalization, ensuring the normalized payload is sent over WebSocket and emitted in events. The flow properly handles the new format parameter while maintaining existing error handling.
Description
Corrects the serialisation of Websocket messages
Extension of #6182
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.