CU-868ex18rd And16k SignalR update. #68
Conversation
|
Caution Review failedThe pull request is closed. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx (1)
56-70: Use RN components, not string host names, in mock UI factoriesCreating 'View'/'Text' by string can be unreliable under RN test renderer. Use RN.View/RN.Text.
-const createMockUIComponent = (displayName: string) => ({ children, testID, ...props }: any) => { - const React = require('react'); - return React.createElement('View', { testID: testID || displayName.toLowerCase(), ...props }, children); -}; +const createMockUIComponent = (displayName: string) => ({ children, testID, ...props }: any) => { + const React = require('react'); + const RN = require('react-native'); + return React.createElement(RN.View, { testID: testID || displayName.toLowerCase(), ...props }, children); +}; -const createMockTextComponent = (displayName: string) => ({ children, testID, ...props }: any) => { - const React = require('react'); - return React.createElement('Text', { testID: testID || displayName.toLowerCase(), ...props }, children); -}; +const createMockTextComponent = (displayName: string) => ({ children, testID, ...props }: any) => { + const React = require('react'); + const RN = require('react-native'); + return React.createElement(RN.Text, { testID: testID || displayName.toLowerCase(), ...props }, children); +}; -const createMockInputComponent = ({ testID, ...props }: any) => { - const React = require('react'); - return React.createElement('TextInput', { testID: testID || 'input-field', ...props }); -}; +const createMockInputComponent = ({ testID, ...props }: any) => { + const React = require('react'); + const RN = require('react-native'); + return React.createElement(RN.TextInput, { testID: testID || 'input-field', ...props }); +};src/services/signalr.service.ts (1)
386-494: Persist legacy hub configs for reconnect in handleConnectionClosehandleConnectionClose only uses hubConfigs (from the EventingUrl flow), so hubs connected via connectToHub() never auto-reconnect. Store each legacy SignalRHubConfig in a new map (e.g. directHubConfigs) in connectToHub(config), remove it on disconnect, and in handleConnectionClose fall back to the direct config when hubConfigs.get(hubName) is missing, invoking connectToHub(directConfig) instead of connectToHubWithEventingUrl. Verified connectToHub is used in src/services/signalr.service.ts and src/hooks/useSignalR.ts.
src/api/mapping/mapping.ts (1)
6-6: Rename getMayLayers to getMapLayers
- In src/api/mapping/mapping.ts line 6: change
getMayLayersApi→getMapLayersApiand update endpoint string'/Mapping/GetMayLayers'→'/Mapping/GetMapLayers'- In src/api/mapping/mapping.ts line 15: change
export const getMayLayers→export const getMapLayersNo other call sites detected.
🧹 Nitpick comments (20)
src/hooks/use-status-signalr-updates.ts (1)
26-47: Type-guard parsed messages to avoid false positives and narrow logging contextEnsure we only process messages with a UserId (and optionally status/staffing fields).
- if (lastUpdateMessage && typeof lastUpdateMessage === 'string') { + if (lastUpdateMessage && typeof lastUpdateMessage === 'string') { try { - const parsedMessage = JSON.parse(lastUpdateMessage); + const parsedMessage = JSON.parse(lastUpdateMessage) as unknown as { UserId?: string; StatusId?: number; StaffingId?: number }; // Check if this is a personnel status or staffing update message for the current user - if (parsedMessage && parsedMessage.UserId === userId) { + if (parsedMessage && typeof parsedMessage.UserId === 'string' && parsedMessage.UserId === userId) { logger.info({ message: 'Processing personnel status/staffing update for current user', context: { userId, timestamp: lastUpdateTimestamp, message: parsedMessage, }, }); // Refresh the current user's status and staffing await fetchCurrentUserInfo();src/hooks/__tests__/use-status-signalr-updates.test.ts (2)
188-215: Avoid duplicate processing assertions using helper to advance timestampsMinor DRY: extract a helper that sets state and rerenders; reduces repetition across dedupe tests.
+const setSignalRState = (timestamp: number, msg: any) => { + mockedUseSignalRStore.mockImplementation((selector) => + selector(createMockSignalRState({ lastUpdateTimestamp: timestamp, lastUpdateMessage: JSON.stringify(msg) })) + ); +}; ... - let timestamp = 1000; - mockedUseSignalRStore.mockImplementation((selector) => { - const state = createMockSignalRState({ - lastUpdateTimestamp: timestamp, - lastUpdateMessage: JSON.stringify(personnelStatusMessage), - }); - return selector(state); - }); + let timestamp = 1000; + setSignalRState(timestamp, personnelStatusMessage); ... - rerender({}); + rerender({});
272-301: Loosen error message match to reduce brittleness across environmentsUsing expect.stringContaining('Failed to') is OK; consider also asserting error instance only to avoid coupling to exact wording.
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (2)
98-101: Deduplicate room fixtures to avoid brittle tests.Define a shared rooms fixture (e.g., in a test utils module) and reuse it across tests to prevent scattered updates when room IDs/names change.
- availableRooms: [ - { id: 'general-chat', name: 'General Chat' }, - { id: 'dev-team-sync', name: 'Dev Team Sync' }, - { id: 'product-updates', name: 'Product Updates' }, - ], + availableRooms: TEST_ROOMS,Also applies to: 348-348
161-163: Stabilize connection-state timing in tests.Using setImmediate can be flaky across environments. Prefer fake timers or invoke the listener directly.
- // Simulate connected state immediately - setImmediate(() => callback('connected')); + // Simulate connected state synchronously for test stability + callback('connected');Or use Jest fake timers:
- // Wait for the connection state change event to fire - await new Promise(resolve => setImmediate(resolve)); + jest.useFakeTimers(); + jest.runAllTimers(); + jest.useRealTimers();Also applies to: 175-175
src/api/common/client.tsx (1)
123-126: Avoid “undefined” params; model options as an object and type request bodies.Current call pattern requires passing undefined to use a signal. Switch to an options object and add generics for request payloads.
export const createApiEndpoint = (endpoint: string) => { return { - get: <T,>(params?: Record<string, unknown>, signal?: AbortSignal) => api.get<T>(endpoint, { params, signal }), - post: <T,>(data: Record<string, unknown>, signal?: AbortSignal) => api.post<T>(endpoint, data, { signal }), - put: <T,>(data: Record<string, unknown>, signal?: AbortSignal) => api.put<T>(endpoint, data, { signal }), - delete: <T,>(params?: Record<string, unknown>, signal?: AbortSignal) => api.delete<T>(endpoint, { params, signal }), + get: <T,>(options?: { params?: Record<string, unknown>; signal?: AbortSignal }) => + api.get<T>(endpoint, options), + post: <TResponse, TBody = unknown>(data?: TBody, options?: { signal?: AbortSignal }) => + api.post<TResponse>(endpoint, data, options), + put: <TResponse, TBody = unknown>(data?: TBody, options?: { signal?: AbortSignal }) => + api.put<TResponse>(endpoint, data, options), + delete: <T,>(options?: { params?: Record<string, unknown>; signal?: AbortSignal }) => + api.delete<T>(endpoint, options), }; };Follow-up: update callers to pass get({ signal }) instead of get(undefined, signal).
src/services/signalr.service.ts (3)
549-576: Preserve invoke return type.invoke currently erases the hub method’s return type (Promise). Make it generic to propagate the result.
- public async invoke(hubName: string, method: string, data: unknown): Promise<void> { + public async invoke<T = unknown>(hubName: string, method: string, data: unknown): Promise<T> { @@ - try { - return await connection.invoke(method, data); + try { + return await connection.invoke<T>(method, data);
578-593: Make resetInstance await disconnection for cleaner test isolation.Return a Promise and await disconnectAll before nulling the singleton.
- public static resetInstance(): void { + public static async resetInstance(): Promise<void> { if (SignalRService.instance) { - // Disconnect all connections before resetting - SignalRService.instance.disconnectAll().catch((error) => { + try { + await SignalRService.instance.disconnectAll(); + } catch (error) { logger.error({ message: 'Error disconnecting all hubs during instance reset', context: { error }, }); - }); + } } SignalRService.instance = null; logger.debug({ message: 'SignalR service singleton instance reset', }); }
56-62: Public state helpers are helpful; consider limiting deprecated API surface.isHubAvailable/isHubReconnecting are useful for tests. If isHubReconnecting is test-only, hide behind a test flag or move to a test helper to avoid accidental prod use.
Also applies to: 71-78, 79-95
src/services/__tests__/signalr.service.test.ts (2)
539-560: Reconnection cap test is solid; minor timer flush tweak.Consider using runAllTimers/runAllTimersAsync to flush setTimeout callbacks instead of runAllTicks for consistency across Jest versions.
- await jest.runAllTicks(); + jest.runOnlyPendingTimers();
518-519: Timer flushing method may be environment-dependent.runAllTicks isn’t always available. Prefer runAllTimers/runOnlyPendingTimers or await jest.runAllTimersAsync().
Also applies to: 547-548
src/api/mapping/mapping.ts (1)
10-12: Pass AbortSignal via options object (avoid undefined param).Align with the proposed client shape to pass signal cleanly.
-export const getMapDataAndMarkers = async (signal?: AbortSignal) => { - const response = await getMapDataAndMarkersApi.get<GetMapDataAndMarkersResult>(undefined, signal); +export const getMapDataAndMarkers = async (signal?: AbortSignal) => { + const response = await getMapDataAndMarkersApi.get<GetMapDataAndMarkersResult>({ signal }); return response.data; };src/hooks/__tests__/use-map-signalr-updates.test.ts (4)
9-10: Export and import DEBOUNCE_DELAY to avoid brittle magic number (1000) in tests.Hardcoding 1000 couples tests to implementation. Export the constant and reuse it here.
Apply:
-import { useMapSignalRUpdates } from '../use-map-signalr-updates'; +import { useMapSignalRUpdates, DEBOUNCE_DELAY } from '../use-map-signalr-updates';
117-125: Replace hardcoded timer values with DEBOUNCE_DELAY.This keeps tests resilient if debounce changes.
Examples (apply similarly to other occurrences):
- jest.advanceTimersByTime(1000); + jest.advanceTimersByTime(DEBOUNCE_DELAY); - jest.advanceTimersByTime(500); + jest.advanceTimersByTime(DEBOUNCE_DELAY / 2); - jest.advanceTimersByTime(800); + jest.advanceTimersByTime(DEBOUNCE_DELAY * 0.8); - jest.advanceTimersByTime(200); + jest.advanceTimersByTime(DEBOUNCE_DELAY - DEBOUNCE_DELAY * 0.8);Also applies to: 132-146, 169-181, 194-214, 237-248, 267-283, 300-314, 327-334, 370-383, 401-419
217-255: Verify that a queued follow-up fetch is executed after the first completes.Current test only asserts de-duping while in-flight; it doesn’t assert the queued fetch fires post-completion. Add an assertion to catch regressions in the “pendingTimestamp” path.
Minimal augmentation:
// Resolve first call resolveFirstCall!(); - await waitFor(() => { - expect(mockOnMarkersUpdate).toHaveBeenCalledTimes(1); - }); + // Allow queued 0ms task to schedule next fetch + jest.runOnlyPendingTimers(); + await waitFor(() => { + expect(mockedGetMapDataAndMarkers).toHaveBeenCalledTimes(2); + expect(mockOnMarkersUpdate).toHaveBeenCalledTimes(1); + });Want me to add a dedicated test covering this path?
150-215: Exercise unmount/abort behavior.Add a test ensuring the in-flight request is aborted on unmount and no updates/log errors occur.
Sketch:
it('aborts in-flight request on unmount without updating', async () => { let resolve!: (v: GetMapDataAndMarkersResult) => void; mockedGetMapDataAndMarkers.mockImplementation( () => new Promise((r) => (resolve = r)) ); mockedUseSignalRStore.mockReturnValue(1000); const { unmount } = renderHook(() => useMapSignalRUpdates(mockOnMarkersUpdate)); jest.advanceTimersByTime(DEBOUNCE_DELAY); unmount(); // triggers abort // Resolve anyway; should be ignored resolve(createMockMapData()); await Promise.resolve(); expect(mockOnMarkersUpdate).not.toHaveBeenCalled(); expect(mockedLogger.error).not.toHaveBeenCalledWith( expect.objectContaining({ message: 'Failed to update map markers from SignalR update' }) ); });src/hooks/use-map-signalr-updates.ts (4)
135-139: Re-check before executing the debounced fetch.Even with the guard above, it’s cleaner to avoid scheduling work when already up-to-date. Pass the timestamp explicitly (optional) or re-check inside the timeout.
- debounceTimer.current = setTimeout(() => { - fetchAndUpdateMarkers(); - }, DEBOUNCE_DELAY); + debounceTimer.current = setTimeout(() => { + fetchAndUpdateMarkers(lastUpdateTimestamp); + }, DEBOUNCE_DELAY);
15-16: Use ReturnType for RN compatibility.NodeJS.Timeout can mismatch in React Native. ReturnType keeps it portable.
- const debounceTimer = useRef<NodeJS.Timeout | null>(null); + const debounceTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
84-92: Broaden Axios cancel detection beyond message === 'canceled'.Axios v1 typically sets code = 'ERR_CANCELED' and name = 'CanceledError'. Check these to avoid false negatives.
- if (error instanceof Error && error.message === 'canceled') { + if ( + (error && typeof error === 'object' && (error as any).code === 'ERR_CANCELED') || + (error instanceof Error && error.name === 'CanceledError') + ) { logger.debug({ message: 'Map markers request was canceled', context: { timestamp: timestampToProcess }, }); return; }
8-10: Export DEBOUNCE_DELAY for tests and reuse.This avoids duplication of debounce values across the codebase and tests.
-// Debounce delay in milliseconds to prevent rapid consecutive API calls -const DEBOUNCE_DELAY = 1000; +// Debounce delay in milliseconds to prevent rapid consecutive API calls +export const DEBOUNCE_DELAY = 1000;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
jest-setup.ts(1 hunks)package.json(3 hunks)src/api/common/client.tsx(1 hunks)src/api/mapping/mapping.ts(1 hunks)src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx(1 hunks)src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts(6 hunks)src/hooks/__tests__/use-map-signalr-updates.test.ts(1 hunks)src/hooks/__tests__/use-status-signalr-updates.test.ts(1 hunks)src/hooks/use-map-signalr-updates.ts(1 hunks)src/hooks/use-status-signalr-updates.ts(4 hunks)src/services/__tests__/signalr.service.test.ts(5 hunks)src/services/signalr.service.ts(13 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/hooks/__tests__/use-status-signalr-updates.test.tssrc/hooks/__tests__/use-map-signalr-updates.test.tssrc/api/mapping/mapping.tssrc/hooks/use-map-signalr-updates.tsjest-setup.tssrc/hooks/use-status-signalr-updates.tssrc/api/common/client.tsxsrc/services/__tests__/signalr.service.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.tssrc/services/signalr.service.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/api/common/client.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/hooks/__tests__/use-status-signalr-updates.test.tssrc/hooks/__tests__/use-map-signalr-updates.test.tssrc/api/mapping/mapping.tssrc/hooks/use-map-signalr-updates.tssrc/hooks/use-status-signalr-updates.tssrc/api/common/client.tsxsrc/services/__tests__/signalr.service.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.tssrc/services/signalr.service.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/hooks/__tests__/use-status-signalr-updates.test.tssrc/hooks/__tests__/use-map-signalr-updates.test.tssrc/services/__tests__/signalr.service.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/hooks/__tests__/use-status-signalr-updates.test.tssrc/hooks/__tests__/use-map-signalr-updates.test.tssrc/services/__tests__/signalr.service.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
🧠 Learnings (3)
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to src/**/*.test.{ts,tsx} : Create and use Jest tests to validate all generated components
Applied to files:
src/hooks/__tests__/use-map-signalr-updates.test.ts
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or navigation features
Applied to files:
package.json
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps, mapping, or vehicle navigation
Applied to files:
package.json
🧬 Code graph analysis (5)
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx (1)
__mocks__/@gorhom/bottom-sheet.js (2)
React(2-2)require(3-3)
src/hooks/__tests__/use-status-signalr-updates.test.ts (2)
src/stores/home/home-store.ts (1)
useHomeStore(48-132)src/hooks/use-status-signalr-updates.ts (1)
useStatusSignalRUpdates(8-67)
src/hooks/__tests__/use-map-signalr-updates.test.ts (2)
src/api/mapping/mapping.ts (1)
getMapDataAndMarkers(10-13)src/hooks/use-map-signalr-updates.ts (1)
useMapSignalRUpdates(11-164)
src/hooks/use-map-signalr-updates.ts (1)
src/api/mapping/mapping.ts (1)
getMapDataAndMarkers(10-13)
src/hooks/use-status-signalr-updates.ts (1)
src/stores/home/home-store.ts (1)
useHomeStore(48-132)
⏰ 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). (1)
- GitHub Check: test
🔇 Additional comments (10)
package.json (1)
88-88: Install dependencies and verify Expo SDK alignment
Runnpm installoryarn, thennpx expo-doctorandnpx expo install --checkto surface any mismatches between Expo SDK 52 (~React Native 0.76.6) and your current RN 0.77.3 plus Reanimated, Gesture Handler, Screens, Safe Area Context, and WebView versions. If conflicts appear, either pin React Native to the Expo-supported version or upgrade the Expo SDK. Replace@rnmapbox/[email protected]with the latest stable release unless you need RC-only fixes.src/hooks/use-status-signalr-updates.ts (1)
16-66: LGTM: scopes SignalR processing to current user and refreshes home store safelyParsing guard, per-timestamp gating, and error logs are sound.
src/hooks/__tests__/use-status-signalr-updates.test.ts (1)
37-59: Solid baseline: isolated store mocks and logger silencing make tests deterministicMock shape mirrors store selectors; good coverage setup.
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (2)
434-437: LGTM: error state assertion after mic failure.Asserts the user-facing error message after awaited operation; this guards against swallowed async errors.
172-172: No action required: startCall signature matches implementation and tests
The CallKeepService.startCall signature is(roomName: string, handle?: string), and the code invokes.startCall(roomId)with only the requiredroomNameparameter—exactly what the tests assert.src/services/signalr.service.ts (3)
35-38: Per-hub connection lock and state tracking: solid.Prevents duplicate connects and serializes connect/disconnect paths. Good use of finally to release locks.
Also applies to: 97-117, 259-280
291-309: Duplicate-connect guard: good.Checking DIRECT_CONNECTING avoids parallel start() calls on the same hub.
Also applies to: 128-136
175-183: Token masking in logs: good.access_token is redacted from fullUrl before logging. No action needed.
src/services/__tests__/signalr.service.test.ts (2)
203-206: LGTM: URLSearchParams encoding expectations.Asserting + for spaces and proper escaping strengthens geolocation URL construction checks.
Also applies to: 226-229
407-411: LGTM: explicit error when invoking on a non-connected hub.Clearer failure mode; prevents silent no-ops.
| jest.mock('react-native', () => ({ | ||
| ...jest.requireActual('react-native'), | ||
| Platform: { | ||
| ...jest.requireActual('react-native').Platform, | ||
| OS: 'ios', | ||
| select: jest.fn().mockImplementation((obj) => obj.ios || obj.default), | ||
| }, | ||
| ScrollView: ({ children, ...props }: any) => { | ||
| const React = require('react'); | ||
| return React.createElement('View', { testID: 'scroll-view', ...props }, children); | ||
| }, | ||
| useWindowDimensions: () => ({ | ||
| width: 400, | ||
| height: 800, | ||
| }), | ||
| })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Over-mocking 'react-native' drops core components; use requireActual and override only what you need
This mock removes View/Text/TextInput exports and can break host components and library internals.
-jest.mock('react-native', () => ({
- Platform: {
- OS: 'ios',
- select: jest.fn().mockImplementation((obj) => obj.ios || obj.default),
- },
- ScrollView: ({ children, ...props }: any) => {
- const React = require('react');
- return React.createElement('View', { testID: 'scroll-view', ...props }, children);
- },
- useWindowDimensions: () => ({
- width: 400,
- height: 800,
- }),
-}));
+jest.mock('react-native', () => {
+ const RN = jest.requireActual('react-native');
+ const React = require('react');
+ return {
+ ...RN,
+ Platform: {
+ ...RN.Platform,
+ OS: 'ios',
+ select: jest.fn().mockImplementation((obj) => obj.ios || obj.default),
+ },
+ ScrollView: ({ children, ...props }: any) =>
+ React.createElement(RN.View, { testID: 'scroll-view', ...props }, children),
+ useWindowDimensions: () => ({ width: 400, height: 800 }),
+ };
+});📝 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.
| jest.mock('react-native', () => ({ | |
| ...jest.requireActual('react-native'), | |
| Platform: { | |
| ...jest.requireActual('react-native').Platform, | |
| OS: 'ios', | |
| select: jest.fn().mockImplementation((obj) => obj.ios || obj.default), | |
| }, | |
| ScrollView: ({ children, ...props }: any) => { | |
| const React = require('react'); | |
| return React.createElement('View', { testID: 'scroll-view', ...props }, children); | |
| }, | |
| useWindowDimensions: () => ({ | |
| width: 400, | |
| height: 800, | |
| }), | |
| })); | |
| // src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx | |
| jest.mock('react-native', () => { | |
| const RN = jest.requireActual('react-native'); | |
| const React = require('react'); | |
| return { | |
| ...RN, | |
| Platform: { | |
| ...RN.Platform, | |
| OS: 'ios', | |
| select: jest.fn().mockImplementation((obj) => obj.ios || obj.default), | |
| }, | |
| ScrollView: ({ children, ...props }: any) => | |
| React.createElement(RN.View, { testID: 'scroll-view', ...props }, children), | |
| useWindowDimensions: () => ({ width: 400, height: 800 }), | |
| }; | |
| }); |
🤖 Prompt for AI Agents
In src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx
around lines 15 to 28, the test currently mocks the entire 'react-native' module
which removes core exports (View, Text, TextInput, etc.) and breaks components;
change the mock to call requireActual('react-native') and spread its exports,
then override only Platform (with OS and select), useWindowDimensions, and if
necessary ScrollView — this preserves core components while replacing just the
bits you need for the test.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/services/signalr.service.ts (4)
391-490: Reconnection only attempts once after onclose; MAX_RECONNECT_ATTEMPTS is never exercisedAfter a failed reconnect in the setTimeout branch, no subsequent attempts are scheduled (there will be no new onclose). This leaves the hub disconnected until manual intervention. Replace the one-shot with a recursive/backoff loop that retries up to MAX_RECONNECT_ATTEMPTS and clears state correctly.
Apply this diff:
- private handleConnectionClose(hubName: string): void { - const attempts = this.reconnectAttempts.get(hubName) || 0; - if (attempts < this.MAX_RECONNECT_ATTEMPTS) { - this.reconnectAttempts.set(hubName, attempts + 1); - const currentAttempts = attempts + 1; - - const hubConfig = this.hubConfigs.get(hubName); - const directHubConfig = this.directHubConfigs.get(hubName); - - if (hubConfig || directHubConfig) { - logger.info({ - message: `Scheduling reconnection attempt ${currentAttempts}/${this.MAX_RECONNECT_ATTEMPTS} for hub: ${hubName}`, - }); - - setTimeout(async () => { - try { - // Check if the hub config was removed (e.g., by explicit disconnect) - const currentHubConfig = this.hubConfigs.get(hubName); - const currentDirectHubConfig = this.directHubConfigs.get(hubName); - - if (!currentHubConfig && !currentDirectHubConfig) { - logger.debug({ - message: `Hub ${hubName} config was removed, skipping reconnection attempt`, - }); - return; - } - - // If a live connection exists, skip; if it's stale/closed, drop it - const existingConn = this.connections.get(hubName); - if (existingConn && existingConn.state === HubConnectionState.Connected) { - logger.debug({ - message: `Hub ${hubName} is already connected, skipping reconnection attempt`, - }); - return; - } - - // Mark as reconnecting and remove stale entry (if any) to allow a fresh connect - this.setHubState(hubName, HubConnectingState.RECONNECTING); - if (existingConn) { - this.connections.delete(hubName); - } - - try { - // Refresh authentication token before reconnecting - logger.info({ - message: `Refreshing authentication token before reconnecting to hub: ${hubName}`, - }); - - await useAuthStore.getState().refreshAccessToken(); - - // Verify we have a valid token after refresh - const token = useAuthStore.getState().accessToken; - if (!token) { - throw new Error('No valid authentication token available after refresh'); - } - - logger.info({ - message: `Token refreshed successfully, attempting to reconnect to hub: ${hubName} (attempt ${currentAttempts}/${this.MAX_RECONNECT_ATTEMPTS})`, - }); - - // Remove the connection from our maps to allow fresh connection - // This is now safe because we have the reconnecting flag set - this.connections.delete(hubName); - - // Use the appropriate reconnection method based on the config type - if (currentHubConfig) { - await this.connectToHubWithEventingUrl(currentHubConfig); - } else if (currentDirectHubConfig) { - await this.connectToHub(currentDirectHubConfig); - } - - // Clear reconnecting state on successful reconnection - this.setHubState(hubName, HubConnectingState.IDLE); - - logger.info({ - message: `Successfully reconnected to hub: ${hubName} after ${currentAttempts} attempts`, - }); - } catch (reconnectionError) { - // Clear reconnecting state on failed reconnection - this.setHubState(hubName, HubConnectingState.IDLE); - - logger.error({ - message: `Failed to refresh token or reconnect to hub: ${hubName}`, - context: { error: reconnectionError, attempts: currentAttempts, maxAttempts: this.MAX_RECONNECT_ATTEMPTS }, - }); - - // Re-throw to trigger the outer catch block - throw reconnectionError; - } - } catch (error) { - // This catch block handles the overall reconnection attempt failure - // The reconnecting flag has already been cleared in the inner catch block - logger.error({ - message: `Reconnection attempt failed for hub: ${hubName}`, - context: { error, attempts: currentAttempts, maxAttempts: this.MAX_RECONNECT_ATTEMPTS }, - }); - - // Don't immediately retry; let the next connection close event trigger another attempt - // This prevents rapid retry loops that could overwhelm the server - } - }, this.RECONNECT_INTERVAL); - } else { - logger.error({ - message: `No stored config found for hub: ${hubName}, cannot attempt reconnection`, - }); - } - } else { - logger.error({ - message: `Max reconnection attempts (${this.MAX_RECONNECT_ATTEMPTS}) reached for hub: ${hubName}`, - }); - - // Clean up resources for this failed connection - this.connections.delete(hubName); - this.reconnectAttempts.delete(hubName); - this.hubConfigs.delete(hubName); - this.directHubConfigs.delete(hubName); - this.setHubState(hubName, HubConnectingState.IDLE); - } - } + private handleConnectionClose(hubName: string): void { + const hubConfig = this.hubConfigs.get(hubName); + const directHubConfig = this.directHubConfigs.get(hubName); + if (!hubConfig && !directHubConfig) { + logger.error({ message: `No stored config found for hub: ${hubName}, cannot attempt reconnection` }); + return; + } + + const attemptFrom = this.reconnectAttempts.get(hubName) ?? 0; + const tryReconnect = async (attempt: number): Promise<void> => { + if (attempt >= this.MAX_RECONNECT_ATTEMPTS) { + logger.error({ message: `Max reconnection attempts (${this.MAX_RECONNECT_ATTEMPTS}) reached for hub: ${hubName}` }); + this.connections.delete(hubName); + this.reconnectAttempts.delete(hubName); + this.hubConfigs.delete(hubName); + this.directHubConfigs.delete(hubName); + this.setHubState(hubName, HubConnectingState.IDLE); + return; + } + + this.reconnectAttempts.set(hubName, attempt + 1); + logger.info({ message: `Scheduling reconnection attempt ${attempt + 1}/${this.MAX_RECONNECT_ATTEMPTS} for hub: ${hubName}` }); + + setTimeout(async () => { + const existingConn = this.connections.get(hubName); + if (existingConn && existingConn.state === HubConnectionState.Connected) { + logger.debug({ message: `Hub ${hubName} is already connected, skipping reconnection attempt` }); + return; + } + + this.setHubState(hubName, HubConnectingState.RECONNECTING); + if (existingConn) this.connections.delete(hubName); + try { + logger.info({ message: `Refreshing authentication token before reconnecting to hub: ${hubName}` }); + await useAuthStore.getState().refreshAccessToken(); + const token = useAuthStore.getState().accessToken; + if (!token) throw new Error('No valid authentication token available after refresh'); + + logger.info({ message: `Token refreshed, attempting reconnect to hub: ${hubName} (attempt ${attempt + 1}/${this.MAX_RECONNECT_ATTEMPTS})` }); + const currentHubConfig = this.hubConfigs.get(hubName); + const currentDirectHubConfig = this.directHubConfigs.get(hubName); + if (currentHubConfig) { + await this.connectToHubWithEventingUrl(currentHubConfig); + } else if (currentDirectHubConfig) { + await this.connectToHub(currentDirectHubConfig); + } + this.setHubState(hubName, HubConnectingState.IDLE); + this.reconnectAttempts.set(hubName, 0); + logger.info({ message: `Successfully reconnected to hub: ${hubName} after ${attempt + 1} attempts` }); + } catch (error) { + this.setHubState(hubName, HubConnectingState.IDLE); + logger.error({ message: `Reconnection attempt failed for hub: ${hubName}`, context: { error, attempt: attempt + 1, maxAttempts: this.MAX_RECONNECT_ATTEMPTS } }); + // schedule next attempt + void tryReconnect(attempt + 1); + } + }, this.RECONNECT_INTERVAL); + }; + + void tryReconnect(attemptFrom); + }Also applies to: 496-507
181-183: Avoid logging full config objects; potential sensitive data exposureLogging entire config may leak URLs or future tokens if config surfaces them. Log only safe fields.
Apply:
- context: { config, fullUrl: isGeolocationHub ? fullUrl.replace(/access_token=[^&]+/, 'access_token=***') : fullUrl }, + context: { name: config.name, hub: config.hubName, fullUrl: isGeolocationHub ? fullUrl.replace(/access_token=[^&]+/, 'access_token=***') : fullUrl },- context: { config }, + context: { name: config.name, url: config.url },Also applies to: 316-319
565-592: Return typed results from invoke; current Promise discards hub responsesExpose a generic result to preserve type-safety and payloads from server invocations.
Apply:
- public async invoke(hubName: string, method: string, data: unknown): Promise<void> { + public async invoke<T = unknown>(hubName: string, method: string, data: unknown): Promise<T> { @@ - if (connection) { + if (connection) { try { - return await connection.invoke(method, data); + return await connection.invoke<T>(method, data); } catch (error) { @@ - } else if (this.reconnectingHubs.has(hubName)) { + } else if (this.reconnectingHubs.has(hubName)) { throw new Error(`Cannot invoke method ${method} on hub ${hubName}: hub is currently reconnecting`); } else { throw new Error(`Cannot invoke method ${method} on hub ${hubName}: hub is not connected`); } }
157-177: Ensure URL polyfills are loaded at app startup
The project includesreact-native-url-polyfillin yarn.lock but it’s never imported. Add at the very top of your JS/TS entry point (e.g.index.js/App.tsx):import 'react-native-url-polyfill/auto';This guarantees
URLandURLSearchParamsare defined across all RN runtimes.
♻️ Duplicate comments (1)
src/hooks/use-map-signalr-updates.ts (1)
21-35: Nice: duplicate-timestamp guard added as previously requestedPrevents refetching the same timestamp; good logging too.
🧹 Nitpick comments (13)
jest-setup.ts (1)
12-18: Minor: prefer microtask when emulating setImmediateFor closer semantics, prefer
queueMicrotaskwhen available, falling back tosetTimeout(0). Keeps behavior consistent under fake timers.Apply:
-if (typeof global.setImmediate === 'undefined') { - // @ts-ignore - Simple polyfill for setImmediate - global.setImmediate = (callback: (...args: any[]) => void, ...args: any[]) => { - return setTimeout(callback, 0, ...args); - }; -} +if (typeof global.setImmediate === 'undefined') { + // @ts-ignore - Simple polyfill for setImmediate + global.setImmediate = (callback: (...args: any[]) => void, ...args: any[]) => { + if (typeof queueMicrotask === 'function') { + queueMicrotask(() => callback(...args)); + return 0; + } + return setTimeout(callback, 0, ...args); + }; +}src/stores/personnel/__tests__/store.test.ts (1)
123-124: Avoid timer-mode leaks; use try/finally or stay on fake timersSwitching timer modes mid-test can leak if the test throws before restoration. Either (a) use try/finally, or (b) keep fake timers and advance time.
Apply (try/finally):
- jest.useRealTimers(); // Use real timers for this test to avoid timing issues + jest.useRealTimers(); // Use real timers for this test to avoid timing issues + try { ... - jest.useFakeTimers(); // Restore fake timers + } finally { + jest.useFakeTimers(); // Always restore + }Or stay on fake timers:
- jest.useRealTimers(); // Use real timers for this test to avoid timing issues - - mockGetAllPersonnelInfos.mockImplementation(() => new Promise((resolve) => setTimeout(() => resolve({ Data: mockPersonnelData } as any), 100))); + // Keep fake timers; advance to simulate delay + mockGetAllPersonnelInfos.mockImplementation( + () => new Promise((resolve) => setTimeout(() => resolve({ Data: mockPersonnelData } as any), 100)) + ); ... - await fetchPromise; + await Promise.resolve(); // flush microtasks + jest.advanceTimersByTime(100); + await fetchPromise; - jest.useFakeTimers(); // Restore fake timersAlso applies to: 137-139
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (1)
158-166: Store the connectionState callback — also reset between testsStoring
_connectionStateCallbackis fine. Add a reset inbeforeEachto avoid cross-test bleed if a test forgets to set it.Apply:
- // Reset the Room mock to return mockRoom by default + // Reset the Room mock to return mockRoom by default MockedRoom.mockImplementation(() => mockRoom); + (mockRoom as any)._connectionStateCallback = undefined;src/hooks/use-map-signalr-updates.ts (4)
16-17: Use portable timer type
NodeJS.Timeoutcan be incorrect in RN/web builds. PreferReturnType<typeof setTimeout>for cross-env compatibility.Apply:
- const debounceTimer = useRef<NodeJS.Timeout | null>(null); + const debounceTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
37-45: Queue latest while in-flight — consider dropping stale resultsYou queue the newest timestamp, but if a slower request resolves after a newer timestamp arrives, it still applies its data. Consider skipping apply when the result is stale to avoid flicker.
Apply:
- if (mapDataAndMarkers && mapDataAndMarkers.Data) { + // Drop stale results if a newer timestamp arrived meanwhile + if (timestampToProcess < lastUpdateTimestamp) { + logger.debug({ message: 'Dropping stale map markers result', context: { processed: timestampToProcess, latest: lastUpdateTimestamp } }); + return; + } + if (mapDataAndMarkers && mapDataAndMarkers.Data) { ... }
85-88: Reduce duplication between lastProcessed refsYou maintain both
lastProcessedTimestampandlastProcessedTimestampRef. One source of truth simplifies reasoning. Suggest keeping onlylastProcessedTimestampRef(init 0) and reading.currenteverywhere.Apply:
- const lastProcessedTimestamp = useRef<number>(0); - const lastProcessedTimestampRef = useRef<number | undefined>(undefined); + const lastProcessedTimestampRef = useRef<number>(0); ... - if (lastUpdateTimestamp > 0 && lastUpdateTimestamp !== lastProcessedTimestamp.current) { + if (lastUpdateTimestamp > 0 && lastUpdateTimestamp !== lastProcessedTimestampRef.current) { ... - lastProcessedTimestamp.current = timestampToProcess; lastProcessedTimestampRef.current = timestampToProcess;
112-135: Follow-up scheduling: prefer microtask to avoid timer coupling in tests
setTimeout(..., 0)ties this to fake timers. UsingqueueMicrotaskorsetImmediatedecouples sequencing from timers.Apply:
- // Use setTimeout to avoid potential stack overflow in case of rapid updates - setTimeout(() => fetchAndUpdateMarkers(nextTimestamp), 0); + // Use microtask to avoid stack growth and test-time timer coupling + (typeof queueMicrotask === 'function' + ? queueMicrotask + : (fn: () => void) => setTimeout(fn, 0))(() => fetchAndUpdateMarkers(nextTimestamp));src/services/signalr.service.ts (6)
203-206: Capture and log the close error for diagnosticsonclose provides the error; include it to aid triage.
- connection.onclose(() => { - this.handleConnectionClose(config.name); - }); + connection.onclose((error) => { + logger.warn({ message: `Connection closed: ${config.name}`, context: { error } }); + this.handleConnectionClose(config.name); + });Repeat for the direct URL path.
Also applies to: 330-333
594-609: Make resetInstance await disconnectAll to avoid racey tests and new-instance overlapsresetInstance is sync and fires-and-forgets disconnectAll. Prefer an async API that awaits teardown before nulling the singleton.
- public static resetInstance(): void { + public static async resetInstance(): Promise<void> { if (SignalRService.instance) { - // Disconnect all connections before resetting - SignalRService.instance.disconnectAll().catch((error) => { - logger.error({ - message: 'Error disconnecting all hubs during instance reset', - context: { error }, - }); - }); + try { + await SignalRService.instance.disconnectAll(); + } catch (error) { + logger.error({ message: 'Error disconnecting all hubs during instance reset', context: { error } }); + } } SignalRService.instance = null; logger.debug({ message: 'SignalR service singleton instance reset', }); }
129-137: Terminology nit: “direct-connecting” used in Eventing URL pathLogs say “direct-connecting” inside the Eventing URL path, which may confuse ops dashboards. Consider “connecting” or “eventing-connecting”.
Also applies to: 301-310
537-556: Optionally remove handlers on disconnect to prevent leaks if a connection instance is reusedYou create fresh HubConnection objects, so this is low risk. Still, calling connection.off for registered methods before stop() guards against duplicate callbacks if reuse ever changes.
197-199: Two reconnection layers (SignalR automatic + custom). Ensure they don’t conflict and document the rationaleYou rely on withAutomaticReconnect and a custom token-refresh reconnect. Keep the custom loop (after fix) to refresh tokens and rebuild connections for query-string token hubs, but document the interplay to avoid future regressions.
Also applies to: 325-327, 390-490
57-63: Public isHubAvailable may be misleading during long reconnectsIt returns true for connecting states, but invoke() will still throw while reconnecting. If used for UI gating, consider exposing a tri-state or adding isConnected(hubName) to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
jest-setup.ts(1 hunks)src/api/mapping/mapping.ts(1 hunks)src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts(6 hunks)src/features/livekit-call/store/useLiveKitCallStore.ts(1 hunks)src/hooks/use-map-signalr-updates.ts(1 hunks)src/services/signalr.service.ts(13 hunks)src/stores/personnel/__tests__/store.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/mapping/mapping.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/features/livekit-call/store/useLiveKitCallStore.tssrc/stores/personnel/__tests__/store.test.tssrc/hooks/use-map-signalr-updates.tsjest-setup.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.tssrc/services/signalr.service.ts
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/features/livekit-call/store/useLiveKitCallStore.tssrc/stores/personnel/__tests__/store.test.tssrc/hooks/use-map-signalr-updates.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.tssrc/services/signalr.service.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/stores/personnel/__tests__/store.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/stores/personnel/__tests__/store.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
🧠 Learnings (2)
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.{ts,tsx} : Use zustand for state management
Applied to files:
src/features/livekit-call/store/useLiveKitCallStore.ts
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Use zustand for state management
Applied to files:
src/features/livekit-call/store/useLiveKitCallStore.ts
🧬 Code graph analysis (2)
src/hooks/use-map-signalr-updates.ts (1)
src/api/mapping/mapping.ts (1)
getMapDataAndMarkers(10-13)
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (1)
src/features/livekit-call/store/useLiveKitCallStore.ts (1)
useLiveKitCallStore(46-335)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
src/features/livekit-call/store/useLiveKitCallStore.ts (1)
3-3: Approve named import ofcreate: Zustand v4.5.5 exportscreateas a named symbol, and all existing stores already useimport { create } from 'zustand'consistently—no changes needed.jest-setup.ts (1)
9-11: Good move: enable Jest fake timers globallyThis replaces brittle hand-rolled timer shims with deterministic timers. LGTM.
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (7)
98-101: Room presets updated — consistent with store defaultsThe new room list matches the store’s
initialRooms. LGTM.
172-178: Deterministic connect flow with manual triggerManual firing of
'connected'keeps assertions stable. Good approach.
188-194: Android path validated without CallKeepEnsures platform guard works. LGTM.
206-211: Graceful CallKeep start error handling coveredSolid negative-path coverage.
216-216: Updated logger context to roomId=general-chatMatches new IDs; assertion reads well. LGTM.
359-359: Assertion for first room matches new orderingKeeps tests aligned with store defaults. LGTM.
446-448: Assert error state after mic failureGood—verifies user-visible error propagation, not just logging.
src/hooks/use-map-signalr-updates.ts (1)
47-55: Abort previous in-flight request — goodSolid concurrency hygiene with AbortController.
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 (1)
src/services/signalr.service.ts (1)
224-229: Update state on onreconnecting to gate invoke() sooner.Set RECONNECTING in onreconnecting handlers so callers get a clear “currently reconnecting” error instead of invoking on a dying connection.
connection.onreconnecting((error) => { logger.warn({ message: `Reconnecting to hub: ${config.name}`, context: { error }, }); + this.setHubState(config.name, HubConnectingState.RECONNECTING); });Apply in both internal connect methods.
Also applies to: 368-374
♻️ Duplicate comments (1)
src/services/signalr.service.ts (1)
97-135: Avoid early return after awaiting a newer lock; re-attempt if still disconnected.If another lock appears while waiting, you await it and return—even if it failed—leaving the caller with no retry. Re-check connectivity post-wait and start a fresh attempt when still disconnected. Same pattern applies in connectToHub.
- const currentLock = this.connectionLocks.get(config.name); - if (currentLock && currentLock !== existingLock) { - // Another connection attempt is already in progress, wait for it - await currentLock; - return; - } + const currentLock = this.connectionLocks.get(config.name); + if (currentLock && currentLock !== existingLock) { + // Another attempt in progress; wait, then re-check connection/lock. + await currentLock; + const conn = this.connections.get(config.name); + if (conn?.state === HubConnectionState.Connected) return; + const postLock = this.connectionLocks.get(config.name); + if (postLock) { + await postLock; // someone else started again; let it finish + return; + } + // fall through to create a new attempt + }Apply the same change to connectToHub below.
🧹 Nitpick comments (11)
src/services/__tests__/signalr-lock-retry.test.ts (2)
148-163: Stabilize cleanup by giving mock connections a stop() stub.In this test the mock connection only sets state; disconnectAll may call stop() and hit undefined. Add a no-op stop to avoid noisy cleanup errors even if they’re ignored.
- const mockConnection = { - state: HubConnectionState.Connected, - }; + const mockConnection = { + state: HubConnectionState.Connected, + stop: jest.fn(), + };
57-105: Add a true concurrent callers test to validate the per-hub lock path.Current tests cover sequential failure→retry. Add a case where two callers start while the first connect is in-flight and fails, asserting the second re-attempts after awaiting the first lock.
I can add a test that issues Promise.all on two connect calls, forces the first internal connect to reject, then verifies the second call performs a fresh attempt and succeeds. Want me to push that test?
Also applies to: 140-178, 181-229
src/services/__tests__/signalr-reconnection-fix.test.ts (2)
24-26: Scope fake timers per-test to avoid cross-test timer mode drift.File-level jest.useFakeTimers with per-test useRealTimers in afterEach can cause inconsistent modes. Prefer enabling fake timers only in tests that need them.
-// Mock timers to have control over setTimeout -jest.useFakeTimers(); +// Enable fake timers only within individual tests that need them.And remove useRealTimers() in afterEach/afterAll.
Also applies to: 43-50
116-142: Backoff test mirrors implementation; consider exercising the real scheduler.This asserts a reimplemented formula. For stronger coverage, export a small helper for delay calculation or advance timers and assert scheduling with attemptReconnection instead.
src/services/__tests__/signalr.service.test.ts (1)
569-571: Prefer runOnlyPendingTimersAsync for clarity.After advancing 6000ms, use await jest.runOnlyPendingTimersAsync() to flush the scheduled reconnection callback instead of runAllTicks which doesn’t flush macrotasks.
-// Wait for all promises to resolve -await jest.runAllTicks(); +// Flush scheduled reconnection +await jest.runOnlyPendingTimersAsync?.();src/services/__examples__/signalr-typed-invoke-examples.ts (2)
32-45: Use project logger instead of console to keep logs consistent.Switch to '@/lib/logging'. This keeps logging format uniform and testable.
+import { logger } from '@/lib/logging'; @@ - console.log(`User ${user.name} (${user.email}) is ${user.isActive ? 'active' : 'inactive'}`); + logger.info({ message: `User ${user.name} (${user.email}) is ${user.isActive ? 'active' : 'inactive'}` }); @@ - console.error('Failed to get user info:', error); + logger.error({ message: 'Failed to get user info', context: { error } }); @@ - console.log(`Call ${call.callId} has status: ${call.status} with priority: ${call.priority}`); + logger.info({ message: `Call ${call.callId} has status: ${call.status} with priority: ${call.priority}` }); @@ - console.error('Failed to get call details:', error); + logger.error({ message: 'Failed to get call details', context: { error } }); @@ - console.log(`Successfully retrieved user: ${response.data.name}`); - console.log(`Response message: ${response.message}`); + logger.info({ message: `Successfully retrieved user: ${response.data.name}; ${response.message}` }); @@ - console.warn(`API returned failure: ${response.message}`); + logger.warn({ message: `API returned failure: ${response.message}` }); @@ - console.error('Failed to get wrapped user data:', error); + logger.error({ message: 'Failed to get wrapped user data', context: { error } }); @@ - console.log('Command executed, result:', result); + logger.info({ message: 'Command executed', context: { hasResult: result !== undefined } }); @@ - console.error('Command execution failed:', error); + logger.error({ message: 'Command execution failed', context: { error } }); @@ - console.log('Notification sent successfully'); + logger.info({ message: 'Notification sent successfully' }); @@ - console.error('Failed to send notification:', error); + logger.error({ message: 'Failed to send notification', context: { error } }); @@ - units.forEach(unit => { - console.log(`Unit ${unit.name} at ${unit.location.address} - Status: ${unit.status}`); - }); + units.forEach(unit => { + logger.debug({ message: `Unit ${unit.name} at ${unit.location.address} - Status: ${unit.status}` }); + }); @@ - console.error('Failed to get active units:', error); + logger.error({ message: 'Failed to get active units', context: { error } });Also applies to: 50-66, 71-91, 96-107, 112-126, 145-163
1-6: Folder naming nit: prefer hyphen-case over underscores in src.Guidelines recommend lowercase, hyphenated names; consider moving examples to examples or examples-signalr.
src/services/signalr.service.ts (4)
499-526: Remove redundant connections.delete(hubName).It’s called both before and after the token refresh block. Keep one.
- if (existingConn) { - this.connections.delete(hubName); - } @@ - // Remove the connection from our maps to allow fresh connection - // This is now safe because we have the reconnecting flag set - this.connections.delete(hubName); + // Ensure a fresh connection + this.connections.delete(hubName);
622-654: Guard invoke() by connection.state to avoid calling into a closed hub.A closed connection may still be present in the map until cleanup. Check for Connected to fail fast with a clear error.
- const connection = this.connections.get(hubName); - if (connection) { + const connection = this.connections.get(hubName); + if (connection?.state === HubConnectionState.Connected) { @@ - } else if (this.reconnectingHubs.has(hubName)) { + } else if (this.reconnectingHubs.has(hubName) || this.hubStates.get(hubName) === HubConnectingState.RECONNECTING) { throw new Error(`Cannot invoke method ${method} on hub ${hubName}: hub is currently reconnecting`); } else { throw new Error(`Cannot invoke method ${method} on hub ${hubName}: hub is not connected`); }
656-671: Consider awaiting disconnectAll during reset to avoid cross-instance races (tests).resetInstance fires an async disconnect without awaiting; new instances can be created while old disconnects run. If feasible, add an async variant used in tests.
- public static resetInstance(): void { + public static resetInstance(): void { if (SignalRService.instance) { - // Disconnect all connections before resetting - SignalRService.instance.disconnectAll().catch((error) => { + // Fire-and-forget disconnect (keep for prod code path) + SignalRService.instance.disconnectAll().catch((error) => { logger.error({ message: 'Error disconnecting all hubs during instance reset', context: { error }, }); }); } SignalRService.instance = null; logger.debug({ message: 'SignalR service singleton instance reset', }); } + + // For tests + public static async resetInstanceAsync(): Promise<void> { + if (SignalRService.instance) { + try { await SignalRService.instance.disconnectAll(); } catch { /* noop */ } + } + SignalRService.instance = null; + logger.debug({ message: 'SignalR service singleton instance reset (async)' }); + }
20-23: Unused type.SignalRMessage isn’t used; remove or wire it into emit/handlers.
-export interface SignalRMessage { - type: string; - data: unknown; -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
package.json(3 hunks)src/app/_layout.tsx(1 hunks)src/services/__examples__/signalr-typed-invoke-examples.ts(1 hunks)src/services/__tests__/signalr-lock-retry.test.ts(1 hunks)src/services/__tests__/signalr-reconnection-fix.test.ts(1 hunks)src/services/__tests__/signalr.service.test.ts(6 hunks)src/services/signalr.service.ts(12 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/services/__tests__/signalr-reconnection-fix.test.tssrc/services/__examples__/signalr-typed-invoke-examples.tssrc/services/__tests__/signalr-lock-retry.test.tssrc/app/_layout.tsxsrc/services/__tests__/signalr.service.test.tssrc/services/signalr.service.ts
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/services/__tests__/signalr-reconnection-fix.test.tssrc/services/__examples__/signalr-typed-invoke-examples.tssrc/services/__tests__/signalr-lock-retry.test.tssrc/app/_layout.tsxsrc/services/__tests__/signalr.service.test.tssrc/services/signalr.service.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/services/__tests__/signalr-reconnection-fix.test.tssrc/services/__tests__/signalr-lock-retry.test.tssrc/services/__tests__/signalr.service.test.ts
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/services/__tests__/signalr-reconnection-fix.test.tssrc/services/__tests__/signalr-lock-retry.test.tssrc/services/__tests__/signalr.service.test.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/app/_layout.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or navigation features
Applied to files:
package.json
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps, mapping, or vehicle navigation
Applied to files:
package.json
🧬 Code graph analysis (3)
src/services/__examples__/signalr-typed-invoke-examples.ts (1)
src/services/signalr.service.ts (1)
signalRService(697-697)
src/services/__tests__/signalr-lock-retry.test.ts (1)
src/services/signalr.service.ts (2)
SignalRHubConnectConfig(13-18)SignalRHubConfig(7-11)
src/services/__tests__/signalr.service.test.ts (1)
src/services/signalr.service.ts (1)
signalRService(697-697)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
src/app/_layout.tsx (1)
1-2: URL polyfill import is correctly placed at the very top. LGTM.This follows the package guidance to import
react-native-url-polyfill/autoat the entry point and prevents known RN URL pitfalls. (npmjs.com, github.com)package.json (2)
88-88: RN 0.77 on Expo SDK 52: versions and exclude list look spot on.
- The bumps to RN 0.77.3, Reanimated ~3.16.7, RNGH ~2.22.0, Screens ~4.8.0, Safe Area ~5.1.0, WebView ~13.13.1, plus the
expo.install.excludeentries, align with Expo’s guidance for opting into RN 0.77 under SDK 52. Also, 5.1.0 fixes RN 0.77 compatibility, and Reanimated 3.16.7 supports RN 0.77. (expo.dev, sourceforge.net, docs.swmansion.com)Action: Please confirm you’re testing with a dev build (Expo Go on SDK 52 stays on RN 0.76) and that you’ve run
expo prebuild --cleanafter the upgrade as recommended. (expo.dev)Also applies to: 140-140, 147-147, 153-153, 155-156, 158-158, 160-160, 231-236
88-88: Mapbox RC in production? Verify risk tolerance.You’ve pinned
@rnmapbox/mapsto10.1.42-rc.0(a release candidate). If this is headed to prod, confirm you’re comfortable with RC stability and recent deprecations (e.g., preferLocationPuckoverUserLocationrenderModenative). Consider moving to the next stable once available. (github.com)src/services/__tests__/signalr.service.test.ts (1)
207-212: LGTM: URLSearchParams “+” space-encoding expectation is correct.
| public async connectToHub(config: SignalRHubConfig): Promise<void> { | ||
| // Check for existing lock to prevent concurrent connections to the same hub | ||
| const existingLock = this.connectionLocks.get(config.name); | ||
| if (existingLock) { | ||
| logger.info({ | ||
| message: `Connection to hub ${config.name} is already in progress, waiting...`, | ||
| }); | ||
| await existingLock; | ||
|
|
||
| // After waiting, re-check the connection state and whether a lock still exists | ||
| // Only skip connection if the hub is already connected | ||
| if (this.connections.has(config.name)) { | ||
| const connection = this.connections.get(config.name); | ||
| if (connection && connection.state === HubConnectionState.Connected) { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // If no active connection exists or lock is gone, proceed to establish connection | ||
| // Check if another lock was created while we were waiting | ||
| const currentLock = this.connectionLocks.get(config.name); | ||
| if (currentLock && currentLock !== existingLock) { | ||
| // Another connection attempt is already in progress, wait for it | ||
| await currentLock; | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // Create a new connection promise and store it as a lock | ||
| const connectionPromise = this._connectToHubInternal(config); | ||
| this.connectionLocks.set(config.name, connectionPromise); | ||
|
|
||
| try { | ||
| await connectionPromise; | ||
| } finally { | ||
| // Remove the lock after connection completes (success or failure) | ||
| this.connectionLocks.delete(config.name); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mirror the post-lock re-check fix for direct URL connects.
- const currentLock = this.connectionLocks.get(config.name);
- if (currentLock && currentLock !== existingLock) {
- // Another connection attempt is already in progress, wait for it
- await currentLock;
- return;
- }
+ const currentLock = this.connectionLocks.get(config.name);
+ if (currentLock && currentLock !== existingLock) {
+ await currentLock;
+ const conn = this.connections.get(config.name);
+ if (conn?.state === HubConnectionState.Connected) return;
+ const postLock = this.connectionLocks.get(config.name);
+ if (postLock) { await postLock; return; }
+ }📝 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.
| public async connectToHub(config: SignalRHubConfig): Promise<void> { | |
| // Check for existing lock to prevent concurrent connections to the same hub | |
| const existingLock = this.connectionLocks.get(config.name); | |
| if (existingLock) { | |
| logger.info({ | |
| message: `Connection to hub ${config.name} is already in progress, waiting...`, | |
| }); | |
| await existingLock; | |
| // After waiting, re-check the connection state and whether a lock still exists | |
| // Only skip connection if the hub is already connected | |
| if (this.connections.has(config.name)) { | |
| const connection = this.connections.get(config.name); | |
| if (connection && connection.state === HubConnectionState.Connected) { | |
| return; | |
| } | |
| } | |
| // If no active connection exists or lock is gone, proceed to establish connection | |
| // Check if another lock was created while we were waiting | |
| const currentLock = this.connectionLocks.get(config.name); | |
| if (currentLock && currentLock !== existingLock) { | |
| // Another connection attempt is already in progress, wait for it | |
| await currentLock; | |
| return; | |
| } | |
| } | |
| // Create a new connection promise and store it as a lock | |
| const connectionPromise = this._connectToHubInternal(config); | |
| this.connectionLocks.set(config.name, connectionPromise); | |
| try { | |
| await connectionPromise; | |
| } finally { | |
| // Remove the lock after connection completes (success or failure) | |
| this.connectionLocks.delete(config.name); | |
| } | |
| } | |
| public async connectToHub(config: SignalRHubConfig): Promise<void> { | |
| // Check for existing lock to prevent concurrent connections to the same hub | |
| const existingLock = this.connectionLocks.get(config.name); | |
| if (existingLock) { | |
| logger.info({ | |
| message: `Connection to hub ${config.name} is already in progress, waiting...`, | |
| }); | |
| await existingLock; | |
| // After waiting, re-check the connection state and whether a lock still exists | |
| // Only skip connection if the hub is already connected | |
| if (this.connections.has(config.name)) { | |
| const connection = this.connections.get(config.name); | |
| if (connection && connection.state === HubConnectionState.Connected) { | |
| return; | |
| } | |
| } | |
| // If no active connection exists or lock is gone, proceed to establish connection | |
| // Check if another lock was created while we were waiting | |
| const currentLock = this.connectionLocks.get(config.name); | |
| if (currentLock && currentLock !== existingLock) { | |
| await currentLock; | |
| const conn = this.connections.get(config.name); | |
| if (conn?.state === HubConnectionState.Connected) return; | |
| const postLock = this.connectionLocks.get(config.name); | |
| if (postLock) { | |
| await postLock; | |
| return; | |
| } | |
| } | |
| } | |
| // Create a new connection promise and store it as a lock | |
| const connectionPromise = this._connectToHubInternal(config); | |
| this.connectionLocks.set(config.name, connectionPromise); | |
| try { | |
| await connectionPromise; | |
| } finally { | |
| // Remove the lock after connection completes (success or failure) | |
| this.connectionLocks.delete(config.name); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/services/signalr.service.ts around lines 277 to 315, the post-lock
re-check logic used in connectToHub is missing for direct-URL connection flows;
replicate the same pattern inside the connectToHubByUrl (or any direct URL
connect) method: after awaiting an existing lock, re-check whether a connected
hub already exists and return early if so, then check whether a new lock was
created while waiting and if so await it and return; also ensure you create/set
the connection lock before calling the internal connect, and always delete the
lock in a finally block after the connection attempt completes (success or
failure) to mirror the behavior in connectToHub.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Summary by CodeRabbit