feat(signalr): implement SignalR protocol support#8519
Open
Aragorn7372 wants to merge 14 commits into
Open
Conversation
…d Electron IPC
- Add SignalR schema types (SignalRInvocation with uid/name/type/content/selected)
- Add Yup validation for signalr-request items
- Add bru language grammar for signalr{} and body:signalr{} blocks
- Add bru format parse/stringify for SignalR requests
- Add YML format parse/stringify for SignalR requests
- Add OpenCollection converter to/from SignalR items
- Add Electron IPC handlers using @microsoft/signalr (HubConnection with auto-reconnect, WebSocket transport)
- Wire collection IPC to handle signalr-request create/delete
- Add @microsoft/signalr dependency to bruno-electron
…d tests - Add Redux actions (newSignalRRequest, signalrConnectOnly, sendSignalR) - Add Redux slices for signalr body mode and connection events - Add SignalRQueryUrl component with connect/disconnect controls - Refactor WsBody and SingleWSMessage to support messagesKey reuse - Add SignalR option to CreateTransientRequest, CreateUntitledRequest, NewRequest - Add SIGNALR badge to sidebar request method with themed color - Add signalr preset to collection settings - Wire SignalR event listeners (state-changed, error, hub events) - Add signalr color (#8caaee) to all 13 themes - Add signalr to schema/oss theme config - Add test signalr server (hub/negotiate, WebSocket upgrade) - Add e2e tests (connection, messages, persistence) with fixtures
Aragorn7372
requested review from
bijin-bruno,
helloanoop and
lohit-bruno
as code owners
Aragorn7372
requested review from
naman-bruno,
sid-bruno,
utkarsh-bruno and
vijayh-bruno
as code owners
Contributor
WalkthroughThis PR adds SignalR as a new request type across shared schemas, BRU/YAML/OpenCollection conversion paths, Electron IPC, renderer request panes, request creation entrypoints, and end-to-end test coverage. It also updates theming, tab/default routing, and request-method display for SignalR. ChangesSignalR Request Support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements end-to-end SignalR request support in Bruno, spanning schema/types, Bru/YML serialization, Electron IPC connection management, renderer UI/Redux handling, and an accompanying test hub + e2e coverage.
Changes:
- Added
signalr-requestas a first-class request type with schema/types updates and Bru + YML read/write support (including OpenCollection converters). - Implemented SignalR connection lifecycle + invocation sending via Electron IPC, plus renderer-side listeners that surface hub events/connection state in the existing WS-style timeline/response UI.
- Added SignalR fixtures, a SignalR-compatible test server endpoint, and Playwright e2e specs (connection/messages/persistence), plus theme color support.
Reviewed changes
Copilot reviewed 85 out of 86 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/utils/page/locators.ts | Adds shared Playwright locators for SignalR request UI. |
| tests/signalr/persistence.spec.ts | Adds SignalR persistence e2e coverage (URL + message name). |
| tests/signalr/messages.spec.ts | Adds SignalR multi-message UI e2e coverage. |
| tests/signalr/init-user-data/preferences.json | Adds init user data for SignalR e2e runs. |
| tests/signalr/fixtures/collection/signalr-test.bru | Adds a basic SignalR request fixture. |
| tests/signalr/fixtures/collection/signalr-persistence.bru | Adds a persistence-oriented SignalR fixture. |
| tests/signalr/fixtures/collection/signalr-multi-msg.bru | Adds a multi-invocation SignalR fixture. |
| tests/signalr/fixtures/collection/signalr-headers-auth.bru | Adds a headers/auth SignalR fixture. |
| tests/signalr/fixtures/collection/collection.bru | Adds collection-level fixture content for SignalR tests. |
| tests/signalr/fixtures/collection/bruno.json | Adds collection metadata for SignalR fixtures. |
| tests/signalr/connection.spec.ts | Adds SignalR connect/disconnect e2e coverage. |
| packages/bruno-tests/src/signalr/index.js | Adds a SignalR test hub implementation for e2e. |
| packages/bruno-tests/src/index.js | Wires SignalR negotiate + upgrade routing into test server. |
| packages/bruno-schema/src/collections/index.js | Adds Yup validation for signalr-request items and settings. |
| packages/bruno-schema-types/src/requests/signalr.ts | Introduces TS types for SignalR requests + invocations. |
| packages/bruno-schema-types/src/requests/index.ts | Exports SignalR request types and extends Request union. |
| packages/bruno-schema-types/src/collection/item.ts | Adds signalr-request to supported item types. |
| packages/bruno-lang/v2/src/jsonToBru.js | Adds JSON→Bru generation for signalr {} + body:signalr {}. |
| packages/bruno-lang/v2/src/bruToJson.js | Extends Bru grammar + AST to parse SignalR blocks/messages. |
| packages/bruno-filestore/src/formats/yml/stringifyItem.ts | Adds YML stringifier dispatch for SignalR items. |
| packages/bruno-filestore/src/formats/yml/parseItem.ts | Adds YML parser dispatch + backwards-compat auth handling for SignalR. |
| packages/bruno-filestore/src/formats/yml/items/stringifySignalRRequest.ts | Implements SignalR request→OpenCollection YML serialization. |
| packages/bruno-filestore/src/formats/yml/items/stringifySignalRRequest.spec.ts | Adds unit tests for stringify/parse round-trip. |
| packages/bruno-filestore/src/formats/yml/items/parseSignalRRequest.ts | Implements OpenCollection YML→Bruno SignalR parsing. |
| packages/bruno-filestore/src/formats/bru/index.ts | Adds .bru parse/stringify support for SignalR request type. |
| packages/bruno-electron/src/ipc/network/signalr-event-handlers.js | Adds main-process SignalR IPC handlers + event forwarding. |
| packages/bruno-electron/src/ipc/network/index.js | Registers SignalR IPC handlers during network IPC setup. |
| packages/bruno-electron/src/ipc/collection.js | Includes SignalR request type in filesystem collection operations. |
| packages/bruno-electron/package.json | Adds @microsoft/signalr dependency to Electron package. |
| packages/bruno-converters/tests/opencollection/signalr.spec.js | Adds OpenCollection conversion tests for SignalR items. |
| packages/bruno-converters/src/opencollection/types.ts | Exports SignalR types for converter layer. |
| packages/bruno-converters/src/opencollection/items/signalr.ts | Implements OC⇄Bruno conversion logic for SignalR items. |
| packages/bruno-converters/src/opencollection/items/index.ts | Wires SignalR into generic OC item conversion dispatch. |
| packages/bruno-app/src/utils/tabs/index.js | Treats SignalR items as request tabs. |
| packages/bruno-app/src/utils/snapshot/index.js | Adds SignalR to request tab snapshot/default tab selection logic. |
| packages/bruno-app/src/utils/signalr/signalr-client.js | Adds renderer-side SignalR client helper. |
| packages/bruno-app/src/utils/signalr/connections.js | Adds renderer IPC wrappers for SignalR connection lifecycle. |
| packages/bruno-app/src/utils/network/signalr-event-listeners.js | Adds renderer listeners for SignalR state/errors/events via IPC. |
| packages/bruno-app/src/utils/network/index.js | Adds SignalR connect/send/stop/status functions into network layer. |
| packages/bruno-app/src/utils/importers/common.js | Extends import normalization to support signalr type. |
| packages/bruno-app/src/utils/common/constants.js | Adds SignalR to request-type constants + presets. |
| packages/bruno-app/src/utils/collections/index.js | Extends save/export transforms to handle SignalR bodies and types. |
| packages/bruno-app/src/utils/collections/export.js | Includes SignalR requests/examples in export type mapping. |
| packages/bruno-app/src/utils/collections/emptyStateRequest.js | Adds SignalR option in empty-state create menu. |
| packages/bruno-app/src/themes/schema/oss.js | Extends theme schema to require a SignalR color. |
| packages/bruno-app/src/themes/light/vscode.js | Adds SignalR color to VS Code light theme. |
| packages/bruno-app/src/themes/light/light.js | Adds SignalR color to light theme. |
| packages/bruno-app/src/themes/light/light-pastel.js | Adds SignalR color to light pastel theme. |
| packages/bruno-app/src/themes/light/light-monochrome.js | Adds SignalR color to light monochrome theme. |
| packages/bruno-app/src/themes/light/catppuccin-latte.js | Adds SignalR color to Catppuccin Latte theme. |
| packages/bruno-app/src/themes/dark/vscode.js | Adds SignalR color to VS Code dark theme. |
| packages/bruno-app/src/themes/dark/nord.js | Adds SignalR color to Nord theme. |
| packages/bruno-app/src/themes/dark/dark.js | Adds SignalR color to dark theme. |
| packages/bruno-app/src/themes/dark/dark-pastel.js | Adds SignalR color to dark pastel theme. |
| packages/bruno-app/src/themes/dark/dark-monochrome.js | Adds SignalR color to dark monochrome theme. |
| packages/bruno-app/src/themes/dark/catppuccin-mocha.js | Adds SignalR color to Catppuccin Mocha theme. |
| packages/bruno-app/src/themes/dark/catppuccin-macchiato.js | Adds SignalR color to Catppuccin Macchiato theme. |
| packages/bruno-app/src/themes/dark/catppuccin-frappe.js | Adds SignalR color to Catppuccin Frappe theme. |
| packages/bruno-app/src/providers/ReduxStore/slices/tabs.js | Updates default request pane tab selection for SignalR. |
| packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js | Extends request body handling + WS-like response handling robustness. |
| packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js | Adds SignalR actions (create/connect/send) and request dispatch integration. |
| packages/bruno-app/src/pages/Bruno/index.js | Registers SignalR IPC listeners at app startup. |
| packages/bruno-app/src/components/SignalR/SignalRRequest.jsx | Adds a standalone SignalR UI component (currently not wired in). |
| packages/bruno-app/src/components/SignalR/SignalRConsole.jsx | Adds a basic console component for the standalone SignalR UI. |
| packages/bruno-app/src/components/Sidebar/NewRequest/index.js | Adds SignalR to “New Request” modal and presets handling. |
| packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RequestMethod/StyledWrapper.js | Adds themed sidebar method color styling for SignalR. |
| packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RequestMethod/index.js | Adds SignalR method badge/class handling in sidebar list. |
| packages/bruno-app/src/components/ShareCollection/index.js | Includes SignalR in “share collection” supported types display. |
| packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsx | Disables SignalR in runner configuration (like WS/gRPC). |
| packages/bruno-app/src/components/ResponsePane/Timeline/index.js | Treats SignalR like WS/gRPC for timeline/auth-source behavior. |
| packages/bruno-app/src/components/RequestTabs/RequestTab/index.js | Ensures SignalR tabs are treated as request tabs and handles close behavior. |
| packages/bruno-app/src/components/RequestTabPanel/index.js | Adds SignalR query URL + request pane rendering and URL validation. |
| packages/bruno-app/src/components/RequestPane/WsBody/SingleWSMessage/index.js | Generalizes WS message editor to support SignalR messages. |
| packages/bruno-app/src/components/RequestPane/WsBody/index.js | Generalizes WS body component for WS/SignalR via messagesKey. |
| packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/WSAuthMode/index.js | Adds auth mode selector for SignalR (reusing WS auth modes). |
| packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/StyledWrapper.js | Adds styling wrapper for SignalR auth UI. |
| packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/index.js | Adds SignalR auth panel (reusing WS auth behavior). |
| packages/bruno-app/src/components/RequestPane/SignalRRequestPane/StyledWrapper.js | Adds wrapper for SignalR request pane. |
| packages/bruno-app/src/components/RequestPane/SignalRRequestPane/index.js | Adds SignalR request pane UI (messages/headers/auth/settings/docs). |
| packages/bruno-app/src/components/RequestPane/SignalRQueryUrl/StyledWrapper.js | Adds SignalR URL bar styling + connect/disconnect UI. |
| packages/bruno-app/src/components/RequestPane/SignalRQueryUrl/index.js | Adds SignalR URL bar component with connect/disconnect + send. |
| packages/bruno-app/src/components/CreateUntitledRequest/index.js | Adds SignalR as an option when creating “Untitled” requests. |
| packages/bruno-app/src/components/CreateTransientRequest/index.js | Adds SignalR as an option for transient request creation. |
| packages/bruno-app/src/components/CollectionSettings/Presets/index.js | Adds SignalR request type to collection presets. |
| packages/bruno-app/package.json | Adds @microsoft/signalr dependency for app package. |
| package-lock.json | Locks new SignalR-related dependencies in root lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (17)
packages/bruno-app/src/components/RequestTabPanel/index.js (1)
600-611: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCombine identical
ws-request/signalr-requestbranches.
renderResponsePanereturns the exact same<WSResponsePane .../>JSX for bothws-requestandsignalr-request. A fallthrough case would remove the duplication.♻️ Proposed fix
const renderResponsePane = () => { switch (item.type) { case 'grpc-request': return <GrpcResponsePane item={item} collection={collection} response={item.response} />; case 'ws-request': - return <WSResponsePane item={item} collection={collection} response={item.response} />; case 'signalr-request': return <WSResponsePane item={item} collection={collection} response={item.response} />; default: return <ResponsePane item={item} collection={collection} response={item.response} />; } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/RequestTabPanel/index.js` around lines 600 - 611, The renderResponsePane switch contains duplicate handling for ws-request and signalr-request, both rendering the same WSResponsePane JSX. Refactor the switch in RequestTabPanel to combine these cases with a fallthrough so both item types share one branch, while keeping grpc-request and the default ResponsePane behavior unchanged.packages/bruno-app/src/components/ResponsePane/Timeline/index.js (1)
62-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVariable name no longer reflects its scope.
isGrpcRequestnow also coversws-requestandsignalr-request, which is a bit misleading — it drives which timeline item component (GrpcTimelineItemvsTimelineItem) is used for all three "binary/streaming" protocols, not just gRPC. Consider renaming to something likeisStreamingProtocolRequestfor clarity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/ResponsePane/Timeline/index.js` at line 62, The variable name in the timeline item selection logic is now misleading because it checks grpc-request, ws-request, and signalr-request, not just gRPC. Rename the boolean in the Timeline/index.js component (the one used to choose between GrpcTimelineItem and TimelineItem) to something that reflects all supported streaming protocols, such as isStreamingProtocolRequest, and update any nearby references to match.packages/bruno-app/src/themes/dark/catppuccin-macchiato.js (1)
247-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the named palette constant instead of a repeated hex literal.
signalr: '#8aadf4'duplicatescolors.BLUE(same hex value) as a raw literal, while every sibling entry (grpc: colors.SKY,ws: colors.MAUVE) references the palette. Referencingcolors.BLUEkeeps the theme internally consistent and easier to retheme later.♻️ Proposed fix
grpc: colors.SKY, ws: colors.MAUVE, - signalr: '`#8aadf4`', + signalr: colors.BLUE,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/themes/dark/catppuccin-macchiato.js` at line 247, The `signalr` entry in the Catppuccin Macchiato theme is using a raw hex literal that duplicates an existing palette value; update it to reference the named palette constant used by the other protocol entries. In `catppuccin-macchiato.js`, replace the `signalr` color assignment with the shared `colors.BLUE` symbol so the theme stays consistent and avoids hardcoded duplicates.packages/bruno-app/src/themes/dark/dark-pastel.js (1)
252-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded raw color value bypasses the theme's palette pattern.
Every other
request.methods/requestentry in this theme references a namedcolors.*constant (e.g.colors.MAGENTA,colors.PINK), butsignalruses a rawhsl(...)literal not defined anywhere in thecolorsobject. This makes it harder to keep the palette consistent/reusable across the theme.♻️ Proposed fix
MAGENTA: '`#e09fd9`', // Orchid + SIGNALR: 'hsl(210, 55%, 45%)', // Azure glowws: colors.MAGENTA, // Orchid - signalr: 'hsl(210, 55%, 45%)', + signalr: colors.SIGNALR,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/themes/dark/dark-pastel.js` at line 252, The `signalr` entry in the dark pastel theme is using a raw `hsl(...)` literal instead of the shared palette constants used throughout the `request.methods`/`request` map. Update the `dark-pastel` theme definition so `signalr` references a named `colors.*` value from the local `colors` object, matching the existing pattern in this theme and keeping palette values centralized and reusable.packages/bruno-app/src/components/Sidebar/NewRequest/index.js (1)
153-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating the grpc/ws/signalr submit branches.
The new
isSignalRRequestbranch (177-191) is a byte-for-byte copy of theisWsRequestbranch aside from the action creator, and closely mirrors theisGrpcRequestbranch too. Three near-identical dispatch+then+catch blocks now exist; a lookup fromrequestTypeto action creator would remove the duplication and prevent future divergence (e.g. a bugfix applied to one branch but not the others).♻️ Suggested consolidation
+ const actionByType = { + 'grpc-request': newGrpcRequest, + 'ws-request': newWsRequest, + 'signalr-request': newSignalRRequest + }; + onSubmit: (values) => { - const isGrpcRequest = values.requestType === 'grpc-request'; - const isWsRequest = values.requestType === 'ws-request'; - const isSignalRRequest = values.requestType === 'signalr-request'; + const nonHttpAction = actionByType[values.requestType]; const filename = values.filename; - if (isGrpcRequest) { - dispatch(newGrpcRequest({ ... })).then(...).catch(...); - } else if (isSignalRRequest) { - dispatch(newSignalRRequest({ ... })).then(...).catch(...); - } else if (isWsRequest) { - dispatch(newWsRequest({ ... })).then(...).catch(...); - } else if (isEphemeral) { + if (nonHttpAction) { + dispatch( + nonHttpAction({ + requestName: values.requestName, + requestMethod: values.requestMethod, + filename: filename, + requestType: values.requestType, + requestUrl: values.requestUrl, + collectionUid: collection.uid, + itemUid: item ? item.uid : null + }) + ) + .then(() => { + toast.success('New request created!'); + onClose(); + }) + .catch((err) => toast.error(err ? err.message : 'An error occurred while adding the request')); + } else if (isEphemeral) {(Verify
newGrpcRequesttolerates the extrarequestMethodfield it currently doesn't receive, since it's unused elsewhere.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/Sidebar/NewRequest/index.js` around lines 153 - 206, The onSubmit handler in NewRequest duplicates nearly identical dispatch/then/catch logic across grpc, ws, and signalr branches, so consolidate it by mapping requestType to the correct action creator and sharing one submit flow. Refactor the branch selection around newGrpcRequest, newWsRequest, and newSignalRRequest so the common toast/onClose and error handling lives in one place, and make sure newGrpcRequest still works if the shared path includes requestMethod.tests/signalr/messages.spec.ts (1)
6-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo
test.stepusage.None of the three tests use
test.step, making the generated report harder to scan for what failed within each test.As per path instructions, "Promote the use of
test.stepas much as possible so the generated reports are easier to read."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/signalr/messages.spec.ts` around lines 6 - 32, The three tests in the signalr messages suite should be wrapped in test.step blocks to make the report easier to scan and pinpoint failures. Update each test in the signalr messages describe block to group the navigation and assertion actions into clearly named test.step sections, using the existing test cases and the buildSignalRCommonLocators helper as the main entry points to structure the steps. Keep the test behavior unchanged, but split the setup, visibility/name checks, and delete flow into step-labeled chunks so failures are reported with more context.Source: Path instructions
packages/bruno-filestore/src/formats/yml/items/parseSignalRRequest.ts (1)
105-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid
as anycast forsettings.Casting
signalrSettingstoanybypasses type checking againstItem['settings']. Consider defining a proper settings type shared with the ws-request settings shape instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-filestore/src/formats/yml/items/parseSignalRRequest.ts` at line 105, The parseSignalRRequest mapping is bypassing type safety by casting signalrSettings to any for Item['settings']. Replace that cast by introducing or reusing a shared settings type that matches the ws-request settings shape, and update the parseSignalRRequest return object so settings is assignable without any. Use the signalrSettings and Item['settings'] types to guide the fix.packages/bruno-app/src/components/CreateUntitledRequest/index.js (1)
143-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSignalR menu item reuses the WebSocket icon.
websocket(Line 145) andsignalr(Line 151) both render<IconPlugConnected>, so the dropdown gives users no visual way to tell the two protocols apart at a glance.🎨 Suggested fix: use a distinct icon for SignalR
{ id: 'signalr', label: 'SignalR', - leftSection: <IconPlugConnected size={16} strokeWidth={2} />, + leftSection: <IconBroadcast size={16} strokeWidth={2} />, onClick: handleCreateSignalRRequest },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/CreateUntitledRequest/index.js` around lines 143 - 160, The SignalR dropdown item in CreateUntitledRequest currently uses the same icon as WebSocket, so update the signalr menu entry to render a distinct icon while keeping websocket on IconPlugConnected. Locate the menu items array in CreateUntitledRequest and change only the SignalR item’s leftSection so users can visually distinguish it at a glance.packages/bruno-app/src/components/SignalR/SignalRConsole.jsx (1)
1-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsole output isn't themed or testable.
The
<pre>element has nodata-testidand isn't wrapped in a styled-component, so it won't pick up theme colors and Playwright can't reliably target it.🎨 Suggested fix: styled wrapper + test id
import React from 'react'; +import styled from 'styled-components'; + +const StyledPre = styled.pre` + color: ${(props) => props.theme.text}; + background: ${(props) => props.theme.bg}; +`; export default function SignalRConsole({ logs = [] }) { return ( - <pre> + <StyledPre data-testid="signalr-console"> { JSON.stringify(logs, null, 2) } - </pre> + </StyledPre> ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/SignalR/SignalRConsole.jsx` around lines 1 - 13, The SignalRConsole output is a plain <pre>, so it is not theme-aware and has no stable test hook. Update SignalRConsole to wrap the console output in a styled-component so it can consume theme colors, and add a data-testid to the rendered element for Playwright targeting. Keep the existing logs rendering logic in SignalRConsole and ensure the styled wrapper is used in place of the raw <pre>.Source: Coding guidelines
packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/WSAuthMode/index.js (1)
1-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract a shared auth-mode selector or rename this copy. This is the same dropdown as
packages/bruno-app/src/components/RequestPane/WSRequestPane/WSAuth/WSAuthMode/index.js, so duplicating it underSignalRRequestPane/WSAuthwill drift. If SignalR needs the same behavior, share the component; otherwise rename the folder/component to something SignalR-specific.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/WSAuthMode/index.js` around lines 1 - 73, The WSAuthMode component duplicates the existing auth-mode dropdown used elsewhere, so it will drift over time. Refactor the shared logic in WSAuthMode and MenuDropdown usage into a common reusable component or hook, then have both request panes consume it; if SignalR truly needs different behavior, rename WSAuthMode and its folder to a SignalR-specific symbol so the copy is intentional and easy to distinguish.tests/signalr/connection.spec.ts (1)
8-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider wrapping actions in
test.step.Connect/assert/disconnect/assert sequences aren't broken into
test.stepblocks, making failure reports harder to scan.As per path instructions, "Promote the use of
test.stepas much as possible so the generated reports are easier to read."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/signalr/connection.spec.ts` around lines 8 - 41, Wrap the interaction flows in these SignalR tests with test.step blocks so the connect/assert/disconnect/assert sequences in signalr connects to hub and signalr disconnects from hub are easier to read in reports. Use test.step around the major actions in the connection.spec.ts cases, keeping the existing locators from buildSignalRCommonLocators, locators.connectionControls.connect/disconnect, and the message assertions intact while grouping each logical step separately.Source: Path instructions
packages/bruno-app/src/utils/signalr/connections.js (1)
1-1: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove the unused
eventHandlersregistry. SignalR callbacks are already wired and dispatched inpackages/bruno-electron/src/ipc/network/signalr-event-handlers.js, so this renderer-side map is never read.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/utils/signalr/connections.js` at line 1, The renderer-side eventHandlers registry is unused because SignalR callbacks are already handled in signalr-event-handlers.js, so remove the Map from connections.js and any related setup or references there. Keep the SignalR connection logic in connections.js focused on connection management only, and ensure no code still reads from or writes to eventHandlers after removing it.packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/index.js (1)
43-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
data-testidon auth-state messages for e2e targeting.The various inherited/unsupported-auth messages (Lines 46, 62, 74, 85-87, 94) have no
data-testid, making them harder to target from the new SignalR Playwright specs.Based on coding guidelines: "Add
data-testidto testable elements for Playwright in React components".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/index.js` around lines 43 - 104, The auth-state notice elements rendered by getAuthView in WSAuth need stable Playwright hooks. Add data-testid attributes to each user-facing message branch for unsupported/inherited auth states (including OAuth 2, OAuth 1.0, inherited supported auth, and inherited not supported) so the new SignalR e2e specs can target them reliably. Keep the existing logic and UI text unchanged, and update the JSX in the authMode switch to attach unique test ids to the relevant message containers.Source: Coding guidelines
packages/bruno-tests/src/signalr/index.js (1)
126-140: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUnbounded
connectionsmap growth for abandoned negotiations.Entries added in
handleNegotiateare only removed on socketclose(line 94); a negotiate call that never completes the WS handshake leaks its entry for the life of the test process.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-tests/src/signalr/index.js` around lines 126 - 140, The `connections` map grows forever when `handleNegotiate` creates an entry but the WebSocket handshake never finishes, since cleanup only happens on socket close. Update `handleNegotiate` and the surrounding SignalR test server flow to add a timeout or expiry-based cleanup for negotiated-but-unused `connectionId` entries, and make sure the cleanup path removes the same `connections` entry even if no socket is ever established.packages/bruno-tests/src/index.js (1)
108-115: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider exact/segment match instead of
startsWith.
url.pathname.startsWith('/hub')would also match unrelated paths like/hubfoo. Low risk here since it's test fixture code, but worth tightening (e.g./hubor/hub/) to avoid accidentally misrouting future endpoints.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-tests/src/index.js` around lines 108 - 115, The upgrade routing in the server.on('upgrade') handler is using a broad pathname check that can misroute unrelated paths. Tighten the condition in the upgrade callback by matching only the intended /hub endpoint (and optionally its subpaths) instead of using url.pathname.startsWith, so signalrRouter is only selected for exact hub routes and wsRouter handles everything else.tests/signalr/persistence.spec.ts (1)
25-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo
test.stepusage; consider wrapping Arrange/Act/Assert phases.Neither test uses
test.step, making generated reports harder to scan for the persistence flow (edit → save → restart → verify).As per path instructions, "Promote the use of
test.stepas much as possible so the generated reports are easier to read."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/signalr/persistence.spec.ts` around lines 25 - 74, The SignalR persistence tests are missing `test.step` wrappers, making the edit/save/restart/verify flow harder to read in reports. Update the `save new signalr url` and `save new message name` tests in `persistence.spec.ts` to wrap Arrange, Act, and Assert sections in `test.step` blocks while keeping the existing helpers like `buildSignalRCommonLocators`, `isRequestSaved`, and `restartApp` in place.Source: Path instructions
packages/bruno-app/src/themes/light/catppuccin-latte.js (1)
245-248: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
colors.SKYinstead of duplicating its hex literal.
signalr: '#04a5e5'hardcodes the same value ascolors.SKY(line 18) instead of referencing the constant, breaking the file's convention of using named palette colors everywhere else.🎨 Proposed fix
grpc: colors.SKY, ws: colors.MAUVE, - signalr: '`#04a5e5`', + signalr: colors.SKY, gql: colors.PINK🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/themes/light/catppuccin-latte.js` around lines 245 - 248, The theme palette in the catppuccin-latte color map is duplicating the SKY hex value for signalr instead of using the shared constant. Update the object in the light theme palette so signalr references colors.SKY, matching the existing convention used by grpc, ws, and the rest of the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/index.js`:
- Around line 23-25: The WSAuth save handler is returning the thunk from
saveRequest instead of dispatching it, so auth changes never persist when
BasicAuth/BearerAuth/ApiKeyAuth call save() directly from onSave. Update the
save function in SignalRRequestPane/WSAuth to dispatch saveRequest(item.uid,
collection.uid) before returning, using the existing saveRequest and save
symbols to preserve the current call flow.
In `@packages/bruno-app/src/components/SignalR/SignalRRequest.jsx`:
- Around line 5-19: The connect flow in SignalRRequest is missing both failure
handling and a guard against duplicate connections. Update connect() to
short-circuit when connection already exists/has already been started, and wrap
createSignalRConnection/conn.start in error handling so failures are caught and
reflected in state. Make sure setConnection and setLogs are updated for both
success and failure paths, and use the existing SignalRRequest, connect, and
conn.start symbols to keep the fix localized.
In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`:
- Around line 553-582: The signalrConnectOnly thunk is wrapping an async
executor in new Promise, which can swallow synchronous throws and leave callers
waiting forever. Remove the async Promise executor pattern and make
signalrConnectOnly handle failures by rejecting/returning the connectSignalR
chain properly, or explicitly wrapping the synchronous setup work in try/catch
and calling reject on error. Use the signalrConnectOnly,
getGlobalEnvironmentVariables, and findEnvironmentInCollection flow to ensure
any thrown error is surfaced through the returned promise and the toast/error
path.
In `@packages/bruno-app/src/utils/network/index.js`:
- Around line 382-390: The JSON parsing in the selected message args flow is
swallowing malformed input and defaulting to empty args, which hides invalid
message bodies. Update the parsing logic around selectedMsg.content so that
malformed JSON is surfaced as an error instead of continuing with []. Use the
existing args preparation path to either reject/throw on parse failure or notify
the caller before the hub invocation, and keep the array-wrapping behavior only
for valid parsed values in this network/index.js flow.
In `@packages/bruno-app/src/utils/network/signalr-event-listeners.js`:
- Around line 52-63: The ipcRenderer.on handler in signalr-event-listeners.js
can throw if JSON.stringify is called on non-serializable args, so update the
main:signalr:event listener to safely serialize event payloads before
dispatching wsResponseReceived. Add a guarded serialization path around the
eventData.message construction in the callback, falling back to a safe string
when stringify fails so the renderer does not crash on circular references,
BigInt, or similar values.
In `@packages/bruno-app/src/utils/signalr/connections.js`:
- Around line 45-66: `registerSignalRHandler` and `removeSignalRHandler` trigger
`ipcRenderer.invoke(...)` without exposing the returned promise, so callers
cannot handle failures. Update both functions in `connections.js` to return the
`ipcRenderer.invoke('renderer:signalr:register-handler', ...)` and
`ipcRenderer.invoke('renderer:signalr:remove-handler', ...)` promises, matching
the pattern used by the other exported SignalR helpers. Keep the event handler
map updates as-is, but ensure the IPC call is returned so rejections can be
awaited or caught by the caller.
In `@packages/bruno-electron/src/ipc/collection.js`:
- Line 1182: `renderer:resequence-items` is using a request type list that now
includes SignalR, but `REQUEST_TYPES` in the shared constants does not, so
SignalR items lose their persisted `seq` updates. Update the shared request-type
source of truth used by `collection.js` and `constants.js` to include
`signalr-request`, or consolidate both checks to use the same exported constant
so the type lists cannot drift again.
In `@packages/bruno-electron/src/ipc/network/signalr-event-handlers.js`:
- Around line 81-161: The renderer-side IPC handlers
`renderer:signalr:register-handler` and `renderer:signalr:remove-handler` in
`signalr-event-handlers` are no longer effective because
`connection._invokeClientMethod` now intercepts all incoming invocations and
bypasses `state.connection.on(...)`. Either remove these dead handlers entirely
or update the `_invokeClientMethod` override to dispatch through the existing
handler table so `register-handler`/`remove-handler` still work as intended.
In `@packages/bruno-filestore/src/formats/yml/items/stringifySignalRRequest.ts`:
- Around line 87-94: The SignalR settings serialization in
stringifySignalRRequest is incorrectly writing default 0 values for missing
timeout and keepAliveInterval fields whenever item.settings exists. Update the
item.settings handling so ocRequest.settings only includes timeout and
keepAliveInterval when those properties are actually present and valid on
signalrSettings, instead of coercing undefined to 0; keep the logic localized to
stringifySignalRRequest and its ocRequest.settings assignment.
In `@tests/signalr/connection.spec.ts`:
- Around line 9-10: The spec is bypassing the page-module pattern by using
inline locators instead of centralized builders/actions. Move the sidebar
collection and collections-title interactions into the appropriate page module
under tests/utils/page/*, then update this spec to consume those locator
builders via buildCommonLocators or a feature-specific page module; also replace
any other direct page.locator/page.getByTestId usages in this file the same way.
In `@tests/signalr/messages.spec.ts`:
- Around line 8-9: The SignalR spec is bypassing the page-module locator pattern
by using direct page selectors in the test body. Update the affected
assertions/actions in messages.spec.ts to go through buildSignalRCommonLocators
or a feature-specific page module instead of calling page.locator and
page.getByTestId directly; add any missing accessors to
buildCommonLocators/tests/utils/page/locators.ts so the spec can consume them
consistently.
---
Nitpick comments:
In `@packages/bruno-app/src/components/CreateUntitledRequest/index.js`:
- Around line 143-160: The SignalR dropdown item in CreateUntitledRequest
currently uses the same icon as WebSocket, so update the signalr menu entry to
render a distinct icon while keeping websocket on IconPlugConnected. Locate the
menu items array in CreateUntitledRequest and change only the SignalR item’s
leftSection so users can visually distinguish it at a glance.
In
`@packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/index.js`:
- Around line 43-104: The auth-state notice elements rendered by getAuthView in
WSAuth need stable Playwright hooks. Add data-testid attributes to each
user-facing message branch for unsupported/inherited auth states (including
OAuth 2, OAuth 1.0, inherited supported auth, and inherited not supported) so
the new SignalR e2e specs can target them reliably. Keep the existing logic and
UI text unchanged, and update the JSX in the authMode switch to attach unique
test ids to the relevant message containers.
In
`@packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/WSAuthMode/index.js`:
- Around line 1-73: The WSAuthMode component duplicates the existing auth-mode
dropdown used elsewhere, so it will drift over time. Refactor the shared logic
in WSAuthMode and MenuDropdown usage into a common reusable component or hook,
then have both request panes consume it; if SignalR truly needs different
behavior, rename WSAuthMode and its folder to a SignalR-specific symbol so the
copy is intentional and easy to distinguish.
In `@packages/bruno-app/src/components/RequestTabPanel/index.js`:
- Around line 600-611: The renderResponsePane switch contains duplicate handling
for ws-request and signalr-request, both rendering the same WSResponsePane JSX.
Refactor the switch in RequestTabPanel to combine these cases with a fallthrough
so both item types share one branch, while keeping grpc-request and the default
ResponsePane behavior unchanged.
In `@packages/bruno-app/src/components/ResponsePane/Timeline/index.js`:
- Line 62: The variable name in the timeline item selection logic is now
misleading because it checks grpc-request, ws-request, and signalr-request, not
just gRPC. Rename the boolean in the Timeline/index.js component (the one used
to choose between GrpcTimelineItem and TimelineItem) to something that reflects
all supported streaming protocols, such as isStreamingProtocolRequest, and
update any nearby references to match.
In `@packages/bruno-app/src/components/Sidebar/NewRequest/index.js`:
- Around line 153-206: The onSubmit handler in NewRequest duplicates nearly
identical dispatch/then/catch logic across grpc, ws, and signalr branches, so
consolidate it by mapping requestType to the correct action creator and sharing
one submit flow. Refactor the branch selection around newGrpcRequest,
newWsRequest, and newSignalRRequest so the common toast/onClose and error
handling lives in one place, and make sure newGrpcRequest still works if the
shared path includes requestMethod.
In `@packages/bruno-app/src/components/SignalR/SignalRConsole.jsx`:
- Around line 1-13: The SignalRConsole output is a plain <pre>, so it is not
theme-aware and has no stable test hook. Update SignalRConsole to wrap the
console output in a styled-component so it can consume theme colors, and add a
data-testid to the rendered element for Playwright targeting. Keep the existing
logs rendering logic in SignalRConsole and ensure the styled wrapper is used in
place of the raw <pre>.
In `@packages/bruno-app/src/themes/dark/catppuccin-macchiato.js`:
- Line 247: The `signalr` entry in the Catppuccin Macchiato theme is using a raw
hex literal that duplicates an existing palette value; update it to reference
the named palette constant used by the other protocol entries. In
`catppuccin-macchiato.js`, replace the `signalr` color assignment with the
shared `colors.BLUE` symbol so the theme stays consistent and avoids hardcoded
duplicates.
In `@packages/bruno-app/src/themes/dark/dark-pastel.js`:
- Line 252: The `signalr` entry in the dark pastel theme is using a raw
`hsl(...)` literal instead of the shared palette constants used throughout the
`request.methods`/`request` map. Update the `dark-pastel` theme definition so
`signalr` references a named `colors.*` value from the local `colors` object,
matching the existing pattern in this theme and keeping palette values
centralized and reusable.
In `@packages/bruno-app/src/themes/light/catppuccin-latte.js`:
- Around line 245-248: The theme palette in the catppuccin-latte color map is
duplicating the SKY hex value for signalr instead of using the shared constant.
Update the object in the light theme palette so signalr references colors.SKY,
matching the existing convention used by grpc, ws, and the rest of the file.
In `@packages/bruno-app/src/utils/signalr/connections.js`:
- Line 1: The renderer-side eventHandlers registry is unused because SignalR
callbacks are already handled in signalr-event-handlers.js, so remove the Map
from connections.js and any related setup or references there. Keep the SignalR
connection logic in connections.js focused on connection management only, and
ensure no code still reads from or writes to eventHandlers after removing it.
In `@packages/bruno-filestore/src/formats/yml/items/parseSignalRRequest.ts`:
- Line 105: The parseSignalRRequest mapping is bypassing type safety by casting
signalrSettings to any for Item['settings']. Replace that cast by introducing or
reusing a shared settings type that matches the ws-request settings shape, and
update the parseSignalRRequest return object so settings is assignable without
any. Use the signalrSettings and Item['settings'] types to guide the fix.
In `@packages/bruno-tests/src/index.js`:
- Around line 108-115: The upgrade routing in the server.on('upgrade') handler
is using a broad pathname check that can misroute unrelated paths. Tighten the
condition in the upgrade callback by matching only the intended /hub endpoint
(and optionally its subpaths) instead of using url.pathname.startsWith, so
signalrRouter is only selected for exact hub routes and wsRouter handles
everything else.
In `@packages/bruno-tests/src/signalr/index.js`:
- Around line 126-140: The `connections` map grows forever when
`handleNegotiate` creates an entry but the WebSocket handshake never finishes,
since cleanup only happens on socket close. Update `handleNegotiate` and the
surrounding SignalR test server flow to add a timeout or expiry-based cleanup
for negotiated-but-unused `connectionId` entries, and make sure the cleanup path
removes the same `connections` entry even if no socket is ever established.
In `@tests/signalr/connection.spec.ts`:
- Around line 8-41: Wrap the interaction flows in these SignalR tests with
test.step blocks so the connect/assert/disconnect/assert sequences in signalr
connects to hub and signalr disconnects from hub are easier to read in reports.
Use test.step around the major actions in the connection.spec.ts cases, keeping
the existing locators from buildSignalRCommonLocators,
locators.connectionControls.connect/disconnect, and the message assertions
intact while grouping each logical step separately.
In `@tests/signalr/messages.spec.ts`:
- Around line 6-32: The three tests in the signalr messages suite should be
wrapped in test.step blocks to make the report easier to scan and pinpoint
failures. Update each test in the signalr messages describe block to group the
navigation and assertion actions into clearly named test.step sections, using
the existing test cases and the buildSignalRCommonLocators helper as the main
entry points to structure the steps. Keep the test behavior unchanged, but split
the setup, visibility/name checks, and delete flow into step-labeled chunks so
failures are reported with more context.
In `@tests/signalr/persistence.spec.ts`:
- Around line 25-74: The SignalR persistence tests are missing `test.step`
wrappers, making the edit/save/restart/verify flow harder to read in reports.
Update the `save new signalr url` and `save new message name` tests in
`persistence.spec.ts` to wrap Arrange, Act, and Assert sections in `test.step`
blocks while keeping the existing helpers like `buildSignalRCommonLocators`,
`isRequestSaved`, and `restartApp` in place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 297884e3-7ac9-4f15-ab4b-360dee11af77
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (85)
packages/bruno-app/package.jsonpackages/bruno-app/src/components/CollectionSettings/Presets/index.jspackages/bruno-app/src/components/CreateTransientRequest/index.jspackages/bruno-app/src/components/CreateUntitledRequest/index.jspackages/bruno-app/src/components/RequestPane/SignalRQueryUrl/StyledWrapper.jspackages/bruno-app/src/components/RequestPane/SignalRQueryUrl/index.jspackages/bruno-app/src/components/RequestPane/SignalRRequestPane/StyledWrapper.jspackages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/StyledWrapper.jspackages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/WSAuthMode/index.jspackages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/index.jspackages/bruno-app/src/components/RequestPane/SignalRRequestPane/index.jspackages/bruno-app/src/components/RequestPane/WsBody/SingleWSMessage/index.jspackages/bruno-app/src/components/RequestPane/WsBody/index.jspackages/bruno-app/src/components/RequestTabPanel/index.jspackages/bruno-app/src/components/RequestTabs/RequestTab/index.jspackages/bruno-app/src/components/ResponsePane/Timeline/index.jspackages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsxpackages/bruno-app/src/components/ShareCollection/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RequestMethod/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RequestMethod/index.jspackages/bruno-app/src/components/Sidebar/NewRequest/index.jspackages/bruno-app/src/components/SignalR/SignalRConsole.jsxpackages/bruno-app/src/components/SignalR/SignalRRequest.jsxpackages/bruno-app/src/pages/Bruno/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/index.jspackages/bruno-app/src/providers/ReduxStore/slices/tabs.jspackages/bruno-app/src/themes/dark/catppuccin-frappe.jspackages/bruno-app/src/themes/dark/catppuccin-macchiato.jspackages/bruno-app/src/themes/dark/catppuccin-mocha.jspackages/bruno-app/src/themes/dark/dark-monochrome.jspackages/bruno-app/src/themes/dark/dark-pastel.jspackages/bruno-app/src/themes/dark/dark.jspackages/bruno-app/src/themes/dark/nord.jspackages/bruno-app/src/themes/dark/vscode.jspackages/bruno-app/src/themes/light/catppuccin-latte.jspackages/bruno-app/src/themes/light/light-monochrome.jspackages/bruno-app/src/themes/light/light-pastel.jspackages/bruno-app/src/themes/light/light.jspackages/bruno-app/src/themes/light/vscode.jspackages/bruno-app/src/themes/schema/oss.jspackages/bruno-app/src/utils/collections/emptyStateRequest.jspackages/bruno-app/src/utils/collections/export.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-app/src/utils/common/constants.jspackages/bruno-app/src/utils/importers/common.jspackages/bruno-app/src/utils/network/index.jspackages/bruno-app/src/utils/network/signalr-event-listeners.jspackages/bruno-app/src/utils/signalr/connections.jspackages/bruno-app/src/utils/signalr/signalr-client.jspackages/bruno-app/src/utils/snapshot/index.jspackages/bruno-app/src/utils/tabs/index.jspackages/bruno-converters/src/opencollection/items/index.tspackages/bruno-converters/src/opencollection/items/signalr.tspackages/bruno-converters/src/opencollection/types.tspackages/bruno-converters/tests/opencollection/signalr.spec.jspackages/bruno-electron/package.jsonpackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/network/signalr-event-handlers.jspackages/bruno-filestore/src/formats/bru/index.tspackages/bruno-filestore/src/formats/yml/items/parseSignalRRequest.tspackages/bruno-filestore/src/formats/yml/items/stringifySignalRRequest.spec.tspackages/bruno-filestore/src/formats/yml/items/stringifySignalRRequest.tspackages/bruno-filestore/src/formats/yml/parseItem.tspackages/bruno-filestore/src/formats/yml/stringifyItem.tspackages/bruno-lang/v2/src/bruToJson.jspackages/bruno-lang/v2/src/jsonToBru.jspackages/bruno-schema-types/src/collection/item.tspackages/bruno-schema-types/src/requests/index.tspackages/bruno-schema-types/src/requests/signalr.tspackages/bruno-schema/src/collections/index.jspackages/bruno-tests/src/index.jspackages/bruno-tests/src/signalr/index.jstests/signalr/connection.spec.tstests/signalr/fixtures/collection/bruno.jsontests/signalr/fixtures/collection/collection.brutests/signalr/fixtures/collection/signalr-headers-auth.brutests/signalr/fixtures/collection/signalr-multi-msg.brutests/signalr/fixtures/collection/signalr-persistence.brutests/signalr/fixtures/collection/signalr-test.brutests/signalr/init-user-data/preferences.jsontests/signalr/messages.spec.tstests/signalr/persistence.spec.tstests/utils/page/locators.ts
- Remove unused wsResponseReceived import from collections actions - Add protocol field to signalrRequestSchema Yup validation - Fix multi-frame SignalR message handling in test server - Return empty string instead of null in accessTokenFactory
- Forward full request context for variable interpolation and custom headers - Preserve original _invokeClientMethod for connection.on() compatibility - Guard signalr body map with Array.isArray null safety - Fix user-facing auth text from 'WebSockets' to 'SignalR'
Replace white-space: nowrap with white-space: normal and add max-width: 300px + overflow-wrap: break-word on infotip-text across SignalR, WS, and gRPC query URL tooltips
This reverts commit 8b6d676.
.infotip-text had z-index: 1, matching the active tab z-index, causing tooltips to render behind the tab bar. Increase to 10.
This reverts commit dc829fb.
Tooltip was positioned above the button (bottom: 34px) and clipped by the parent container overflow-hidden. Changed to top: calc(100% + 8px) with arrow flipped to point upward.
- WSAuth/index.js: dispatch(saveRequest) to persist auth changes - SignalRRequest.jsx: add connection guard and try/catch in connect() - actions.js: wrap async executor in try/catch with reject(err) - network/index.js: throw Error on malformed JSON instead of silent args=[] - signalr-event-listeners.js: wrap JSON.stringify in try/catch - connections.js: return ipcRenderer.invoke() promise with .catch() - constants.js: add 'signalr-request' to REQUEST_TYPES - stringifySignalRRequest.ts: only emit timeout/keepAliveInterval when present - connection.spec.ts, messages.spec.ts: use locator builders instead of inline selectors - locators.ts: add sidebarCollectionName() and requestByTitle() accessors
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/bruno-app/src/components/SignalR/SignalRRequest.jsx (3)
37-44: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMissing
data-testidon input/buttons.The PR adds e2e coverage for SignalR; without test ids on the URL input and Connect/Disconnect buttons, Playwright specs are stuck using brittle selectors.
As per coding guidelines, "Add `data-testid` to testable elements for Playwright in React components."🔧 Proposed fix
<input type="text" placeholder="SignalR URL" value={url} onChange={(e) => setUrl(e.target.value)} + data-testid="signalr-url-input" /> - <button onClick={connect}>Connect</button> - <button onClick={disconnect}>Disconnect</button> + <button onClick={connect} data-testid="signalr-connect-button">Connect</button> + <button onClick={disconnect} data-testid="signalr-disconnect-button">Disconnect</button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/SignalR/SignalRRequest.jsx` around lines 37 - 44, The SignalR request controls in SignalRRequest need stable Playwright hooks: add data-testid attributes to the URL input and the Connect/Disconnect buttons so e2e tests do not rely on brittle selectors. Update the JSX in SignalRRequest.jsx where the input element and the two button elements are rendered, using clear identifiers for each control, and keep the existing behavior unchanged.Source: Coding guidelines
25-31: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
disconnect()lacks the error handlingconnect()just gained.If
connection.stop()rejects, the rejection is unhandled andsetConnection(null)never runs — the UI keeps referencing a connection that may already be broken.🔧 Proposed fix
async function disconnect() { if (connection) { - await connection.stop(); - setConnection(null); - setLogs((prev) => [...prev, { event: 'SYSTEM', data: 'Disconnected' }]); + try { + await connection.stop(); + setLogs((prev) => [...prev, { event: 'SYSTEM', data: 'Disconnected' }]); + } catch (err) { + setLogs((prev) => [...prev, { event: 'SYSTEM', data: `Disconnect failed: ${err?.message || err}` }]); + } finally { + setConnection(null); + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/SignalR/SignalRRequest.jsx` around lines 25 - 31, The disconnect() flow in SignalRRequest should handle failures from connection.stop() the same way connect() now does. Wrap the stop/cleanup logic in a try/catch, and if stop rejects, still clear the stale connection state with setConnection(null) and record an appropriate log entry using the existing setLogs pattern. Keep the fix localized to disconnect() and preserve the current success behavior when stop() completes normally.
5-31: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winConnection lifecycle should live in a custom hook, and there's no unmount cleanup.
connect/disconnect/connection state are business logic + side effects embedded directly in the component. There's also nouseEffectto stop an active connection on unmount, so navigating away while connected leaks the underlying WebSocket. Extracting auseSignalRConnection(url)hook would centralize the lifecycle and let you add the missing cleanup effect in one place.As per coding guidelines, "MUST: Prefer custom hooks for business logic, data fetching, and side-effects in React" and "MUST: Avoid
useEffectunless absolutely needed" (unmount cleanup of an external connection qualifies as one of those necessary cases).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/SignalR/SignalRRequest.jsx` around lines 5 - 31, The SignalR connection lifecycle is implemented directly inside SignalRRequest instead of a reusable hook, and the active connection is not cleaned up on unmount. Move the connection state and the connect/disconnect logic from SignalRRequest into a custom hook such as useSignalRConnection(url), keeping the component focused on rendering and consuming the hook’s API. Add the necessary cleanup in that hook with a useEffect teardown so an active connection is stopped when the component unmounts or the URL changes, and ensure the hook owns createSignalRConnection, conn.start(), and connection.stop().Source: Coding guidelines
🧹 Nitpick comments (1)
packages/bruno-electron/src/ipc/network/signalr-event-handlers.js (1)
104-115: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMonkey-patching private SignalR internals is fragile across upgrades.
_invokeClientMethod,_sendWithProtocol, and_createCompletionMessageare underscore-prefixed private members in@microsoft/signalr's source, not part of the public API/semver contract. A minor library bump could silently break this override.Consider pinning
@microsoft/signalrto an exact version and adding a regression test that exercises server→client invocation forwarding, so an upgrade that changes this internal surface is caught immediately.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/src/ipc/network/signalr-event-handlers.js` around lines 104 - 115, The SignalR event forwarding logic in signalr-event-handlers.js relies on private underscore-prefixed internals like _invokeClientMethod, _sendWithProtocol, and _createCompletionMessage, which is fragile across `@microsoft/signalr` upgrades. Update the connection-hooking approach to avoid depending on those internals where possible, and if the override must remain, pin `@microsoft/signalr` to an exact version and add a regression test around the server→client invocation path in the same handler flow so API changes fail loudly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bruno-app/src/components/SignalR/SignalRRequest.jsx`:
- Around line 9-24: The current guard in SignalRRequest's connect function
relies on React state, so rapid repeated clicks can start multiple connections
before setConnection runs. Add a ref-based in-flight lock in SignalRRequest and
check it at the start of connect to prevent duplicate conn.start() calls, then
clear it in both success and catch paths (or a finally block) while keeping the
existing connection state update and ReceiveMessage listener logic intact.
In `@packages/bruno-electron/src/ipc/network/signalr-event-handlers.js`:
- Line 106: `main:signalr:event` is being emitted twice for registered SignalR
handlers because `_invokeClientMethod` broadcasts every invocation and
`register-handler` also forwards the same target through `connection.on(...)`.
Update `signalr-event-handlers.js` so a given invocation goes through only one
path: either keep the generic `sendEvent('main:signalr:event', ...)` broadcast
in `_invokeClientMethod` and prevent the registered handler from re-emitting, or
skip the broadcast when `register-handler` has a matching target. Use the
existing `_invokeClientMethod`, `register-handler`, and `sendEvent` flow to gate
the duplicate emission.
---
Outside diff comments:
In `@packages/bruno-app/src/components/SignalR/SignalRRequest.jsx`:
- Around line 37-44: The SignalR request controls in SignalRRequest need stable
Playwright hooks: add data-testid attributes to the URL input and the
Connect/Disconnect buttons so e2e tests do not rely on brittle selectors. Update
the JSX in SignalRRequest.jsx where the input element and the two button
elements are rendered, using clear identifiers for each control, and keep the
existing behavior unchanged.
- Around line 25-31: The disconnect() flow in SignalRRequest should handle
failures from connection.stop() the same way connect() now does. Wrap the
stop/cleanup logic in a try/catch, and if stop rejects, still clear the stale
connection state with setConnection(null) and record an appropriate log entry
using the existing setLogs pattern. Keep the fix localized to disconnect() and
preserve the current success behavior when stop() completes normally.
- Around line 5-31: The SignalR connection lifecycle is implemented directly
inside SignalRRequest instead of a reusable hook, and the active connection is
not cleaned up on unmount. Move the connection state and the connect/disconnect
logic from SignalRRequest into a custom hook such as useSignalRConnection(url),
keeping the component focused on rendering and consuming the hook’s API. Add the
necessary cleanup in that hook with a useEffect teardown so an active connection
is stopped when the component unmounts or the URL changes, and ensure the hook
owns createSignalRConnection, conn.start(), and connection.stop().
---
Nitpick comments:
In `@packages/bruno-electron/src/ipc/network/signalr-event-handlers.js`:
- Around line 104-115: The SignalR event forwarding logic in
signalr-event-handlers.js relies on private underscore-prefixed internals like
_invokeClientMethod, _sendWithProtocol, and _createCompletionMessage, which is
fragile across `@microsoft/signalr` upgrades. Update the connection-hooking
approach to avoid depending on those internals where possible, and if the
override must remain, pin `@microsoft/signalr` to an exact version and add a
regression test around the server→client invocation path in the same handler
flow so API changes fail loudly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33c5ddb8-6991-48c4-b564-2991889357af
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
packages/bruno-app/src/components/RequestPane/GrpcQueryUrl/StyledWrapper.jspackages/bruno-app/src/components/RequestPane/SignalRQueryUrl/StyledWrapper.jspackages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/index.jspackages/bruno-app/src/components/RequestPane/WsBody/SingleWSMessage/index.jspackages/bruno-app/src/components/RequestPane/WsBody/index.jspackages/bruno-app/src/components/RequestPane/WsQueryUrl/StyledWrapper.jspackages/bruno-app/src/components/RequestTabPanel/index.jspackages/bruno-app/src/components/SignalR/SignalRRequest.jsxpackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/index.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-app/src/utils/importers/common.jspackages/bruno-app/src/utils/network/index.jspackages/bruno-app/src/utils/network/signalr-event-listeners.jspackages/bruno-app/src/utils/signalr/connections.jspackages/bruno-electron/src/ipc/network/signalr-event-handlers.jspackages/bruno-electron/src/utils/constants.jspackages/bruno-filestore/src/formats/yml/items/stringifySignalRRequest.tspackages/bruno-lang/v2/src/bruToJson.jspackages/bruno-lang/v2/src/jsonToBru.jspackages/bruno-schema-types/src/collection/item.tspackages/bruno-schema/src/collections/index.jspackages/bruno-tests/src/signalr/index.jstests/signalr/connection.spec.tstests/signalr/messages.spec.tstests/utils/page/locators.ts
✅ Files skipped from review due to trivial changes (1)
- packages/bruno-electron/src/utils/constants.js
🚧 Files skipped from review as they are similar to previous changes (20)
- tests/signalr/messages.spec.ts
- packages/bruno-schema-types/src/collection/item.ts
- packages/bruno-app/src/components/RequestPane/WsBody/index.js
- tests/utils/page/locators.ts
- packages/bruno-schema/src/collections/index.js
- tests/signalr/connection.spec.ts
- packages/bruno-app/src/utils/importers/common.js
- packages/bruno-app/src/utils/network/signalr-event-listeners.js
- packages/bruno-app/src/utils/network/index.js
- packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js
- packages/bruno-app/src/components/RequestTabPanel/index.js
- packages/bruno-lang/v2/src/bruToJson.js
- packages/bruno-app/src/components/RequestPane/SignalRQueryUrl/StyledWrapper.js
- packages/bruno-filestore/src/formats/yml/items/stringifySignalRRequest.ts
- packages/bruno-lang/v2/src/jsonToBru.js
- packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
- packages/bruno-app/src/utils/collections/index.js
- packages/bruno-tests/src/signalr/index.js
- packages/bruno-app/src/components/RequestPane/SignalRRequestPane/WSAuth/index.js
- packages/bruno-app/src/components/RequestPane/WsBody/SingleWSMessage/index.js
Comment on lines
+9
to
+24
| async function connect() { | ||
| if (connection) return; | ||
| const conn = createSignalRConnection({ | ||
| url | ||
| }); | ||
| conn.on('ReceiveMessage', (...arg) => { | ||
| setLogs((prevLogs) => [...prevLogs, { type: 'ReceiveMessage', data: arg }]); | ||
| }); | ||
| try { | ||
| await conn.start(); | ||
| setConnection(conn); | ||
| setLogs((prev) => [...prev, { event: 'SYSTEM', data: 'Connected' }]); | ||
| } catch (err) { | ||
| setLogs((prev) => [...prev, { event: 'SYSTEM', data: `Connection failed: ${err?.message || err}` }]); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard is state-based, so rapid double-clicks can still open two connections.
if (connection) return checks React state, which won't reflect the in-flight connect() call until setConnection runs after await conn.start() resolves. A second click before that completes bypasses the guard and spins up a duplicate connection.
🔧 Proposed fix using a ref-based lock
+ const isConnectingRef = useRef(false);
+
async function connect() {
- if (connection) return;
+ if (connection || isConnectingRef.current) return;
+ isConnectingRef.current = true;
const conn = createSignalRConnection({
url
});
conn.on('ReceiveMessage', (...arg) => {
setLogs((prevLogs) => [...prevLogs, { type: 'ReceiveMessage', data: arg }]);
});
try {
await conn.start();
setConnection(conn);
setLogs((prev) => [...prev, { event: 'SYSTEM', data: 'Connected' }]);
} catch (err) {
setLogs((prev) => [...prev, { event: 'SYSTEM', data: `Connection failed: ${err?.message || err}` }]);
+ } finally {
+ isConnectingRef.current = false;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function connect() { | |
| if (connection) return; | |
| const conn = createSignalRConnection({ | |
| url | |
| }); | |
| conn.on('ReceiveMessage', (...arg) => { | |
| setLogs((prevLogs) => [...prevLogs, { type: 'ReceiveMessage', data: arg }]); | |
| }); | |
| try { | |
| await conn.start(); | |
| setConnection(conn); | |
| setLogs((prev) => [...prev, { event: 'SYSTEM', data: 'Connected' }]); | |
| } catch (err) { | |
| setLogs((prev) => [...prev, { event: 'SYSTEM', data: `Connection failed: ${err?.message || err}` }]); | |
| } | |
| } | |
| const isConnectingRef = useRef(false); | |
| async function connect() { | |
| if (connection || isConnectingRef.current) return; | |
| isConnectingRef.current = true; | |
| const conn = createSignalRConnection({ | |
| url | |
| }); | |
| conn.on('ReceiveMessage', (...arg) => { | |
| setLogs((prevLogs) => [...prevLogs, { type: 'ReceiveMessage', data: arg }]); | |
| }); | |
| try { | |
| await conn.start(); | |
| setConnection(conn); | |
| setLogs((prev) => [...prev, { event: 'SYSTEM', data: 'Connected' }]); | |
| } catch (err) { | |
| setLogs((prev) => [...prev, { event: 'SYSTEM', data: `Connection failed: ${err?.message || err}` }]); | |
| } finally { | |
| isConnectingRef.current = false; | |
| } | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 18-18: Avoid using the initial state variable in setState
Context: setConnection(conn)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[error] 18-18: React's useState should not be directly called
Context: setConnection(conn)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 19-19: React's useState should not be directly called
Context: setLogs((prev) => [...prev, { event: 'SYSTEM', data: 'Connected' }])
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 21-21: React's useState should not be directly called
Context: setLogs((prev) => [...prev, { event: 'SYSTEM', data: Connection failed: ${err?.message || err} }])
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-app/src/components/SignalR/SignalRRequest.jsx` around lines 9
- 24, The current guard in SignalRRequest's connect function relies on React
state, so rapid repeated clicks can start multiple connections before
setConnection runs. Add a ref-based in-flight lock in SignalRRequest and check
it at the start of connect to prevent duplicate conn.start() calls, then clear
it in both success and catch paths (or a finally block) while keeping the
existing connection state update and ReceiveMessage listener logic intact.
| // Preserve the original _invokeClientMethod so handlers registered via connection.on() still fire. | ||
| const originalInvokeClientMethod = connection._invokeClientMethod?.bind(connection); | ||
| connection._invokeClientMethod = async function (invocationMessage) { | ||
| sendEvent('main:signalr:event', requestId, collectionUid, invocationMessage.target, invocationMessage.arguments); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the SignalR event handler implementation around the cited lines.
file='packages/bruno-electron/src/ipc/network/signalr-event-handlers.js'
wc -l "$file"
sed -n '1,260p' "$file"
# Find all references to the emitted event and handler registration.
rg -n "main:signalr:event|register-handler|_invokeClientMethod|originalInvokeClientMethod|connection\.on\(" packages/bruno-electron/src/ipc/network -SRepository: usebruno/bruno
Length of output: 7891
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find renderer consumers of the SignalR event stream and handler registration.
rg -n "main:signalr:event|renderer:signalr:register-handler|signalr:event" packages/bruno-electron -S
# Narrow to likely renderer-side files if present.
fd -a "signalr" packages/bruno-electronRepository: usebruno/bruno
Length of output: 746
🌐 Web query:
@microsoft/signalr _invokeClientMethod connection.on invocationMessage.target source code
💡 Result:
In the @microsoft/signalr JavaScript client, the method invokeClientMethod (or _invokeClientMethod in newer versions) is an internal function responsible for dispatching incoming messages from the server to the appropriate, user-registered client-side handlers [1][2]. When a message is received from the server, the client parses it into an InvocationMessage, which contains a target property representing the name of the client method to be executed [3][4]. The invokeClientMethod function then performs the following steps: 1. It takes the target string from the InvocationMessage and converts it to lowercase to perform a case-insensitive lookup [1][2]. 2. It searches a registry (typically stored as this.methods or this._methods) for any handlers registered with that specific method name using the connection.on method [1][2]. 3. If matching handlers are found, it executes them by passing the arguments included in the invocationMessage [1][2]. 4. If no handlers are found, it logs a warning stating that no client method with that name was found [1][2]. The connection.on method is the public API used to register these handlers [5][6]. When you call connection.on('methodName', handler), the client stores the provided handler function in an internal map associated with 'methodname' [1][2]. Consequently, when the server sends an invocation with a target matching that name, the internal invokeClientMethod logic retrieves and executes the corresponding function [1][2].
Citations:
- 1: https://github.com/aspnet/SignalR/blob/master/clients/ts/signalr/src/HubConnection.ts
- 2: https://github.com/dotnet/aspnetcore/blob/main/src/SignalR/clients/ts/signalr/src/HubConnection.ts
- 3: https://learn.microsoft.com/en-us/javascript/api/@microsoft/signalr/invocationmessage?view=signalr-js-latest
- 4: https://github.com/dotnet/aspnetcore/blob/master/src/SignalR/clients/ts/signalr/src/IHubProtocol.ts
- 5: https://learn.microsoft.com/en-us/javascript/api/@microsoft/signalr/hubconnection?view=signalr-js-latest
- 6: https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/signalr/javascript-client.md
main:signalr:event is emitted twice for registered handlers. _invokeClientMethod broadcasts every invocation, then register-handler forwards the same target again through connection.on(...). That duplicates renderer events for any registered SignalR method; route each invocation through one path only, or skip the generic broadcast when a handler exists.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-electron/src/ipc/network/signalr-event-handlers.js` at line
106, `main:signalr:event` is being emitted twice for registered SignalR handlers
because `_invokeClientMethod` broadcasts every invocation and `register-handler`
also forwards the same target through `connection.on(...)`. Update
`signalr-event-handlers.js` so a given invocation goes through only one path:
either keep the generic `sendEvent('main:signalr:event', ...)` broadcast in
`_invokeClientMethod` and prevent the registered handler from re-emitting, or
skip the broadcast when `register-handler` has a matching target. Use the
existing `_invokeClientMethod`, `register-handler`, and `sendEvent` flow to gate
the duplicate emission.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Bruno lacked support for SignalR protocol, one of the most popular real-time communication frameworks for ASP.NET. Users had no way to test SignalR connections, send invocations, or receive server events from within Bruno.
Fix
SignalRInvocationtype withuid/name/type/content/selectedfields and Yup validation forsignalr-requestitems.signalr {}meta block andbody:signalr {}message blocks for parsing and generation.@microsoft/signalrviaHubConnectionBuilderwith WebSocket transport, automatic reconnect, and server event interception through_invokeClientMethodoverride.messagesKeyprop; SignalR creation options (transient, untitled, new request); SIGNALR badge in sidebar with themed color.newSignalRRequest,signalrConnectOnly, and sending SignalR invocations; slices for body modesignalrand connection event handling./hub/negotiate) with WebSocket upgrade path, plus e2e tests (connection, messages, persistence) with fixtures.signalrcolor (#8caaee) across all 13 themes.Summary
New Features
Bug Fixes
SignalRRequestPaneto usebody.signalrinstead ofbody.ws.StyledWrapperexport name in WSAuth component.RecieveMessage→ReceiveMessagein preview component.Tests
Test Repository / SignalR Hubs
You can test SignalR requests using a local Bruno test server or a realistic ASP.NET Core SignalR server:
http://localhost:3001/hubnode packages/bruno-tests/src/index.jsTo test with the ASP.NET Core example: