fix: resolve VU meter race condition by waiting for mixers ready
Fixed issue where VU meters were not lighting up on session join due to a race condition between mixer initialization and track sync. ## Root Cause When hasJoined became true, both the mixer initialization effect and the trackSync effect would fire simultaneously. TrackSync was executing before mixers were fully organized, causing isReady to still be false when VU updates started coming in. This resulted in all VU updates being rejected. ## Solution 1. Added mixersReady dependency to trackSync effect in JKSessionScreen 2. TrackSync now waits until mixers are fully initialized before scheduling API calls 3. This ensures proper happens-before relationship: Session joined → Mixers fetched → organizeMixers → isReady=true → TrackSync ## Changes - JKSessionScreen.js: Added mixersReady selector and dependency - useMixerHelper.js: Added minimal logging for isReady state changes - trackSyncService.js: Changed to accept clientId string instead of jamClient - Fixes "Client ID not available" issue by using server.clientId - trackSyncService.test.js: Updated tests to pass clientId directly ## Testing Verified VU meters now light up correctly within 1-2 seconds of session join. TrackSync still executes at 1s, 1.4s, and 6s as expected. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
68f9eb607b
commit
5645284511
|
|
@ -25,7 +25,7 @@ import { syncTracksToServer } from '../../services/trackSyncService';
|
|||
|
||||
// Redux imports
|
||||
import { openModal, closeModal, toggleModal, selectModal } from '../../store/features/sessionUISlice';
|
||||
import { selectMediaSummary, selectMetronomeTrackMixers } from '../../store/features/mixersSlice';
|
||||
import { selectMediaSummary, selectMetronomeTrackMixers, selectMixersReady } from '../../store/features/mixersSlice';
|
||||
import {
|
||||
fetchActiveSession,
|
||||
joinActiveSession,
|
||||
|
|
@ -154,6 +154,7 @@ const JKSessionScreen = () => {
|
|||
const hasJoined = useSelector(selectHasJoined);
|
||||
const sessionGuardsPassed = useSelector(selectGuardsPassed);
|
||||
const userTracks = useSelector(selectUserTracks);
|
||||
const mixersReady = useSelector(selectMixersReady);
|
||||
const showConnectionAlert = useSelector(selectShowConnectionAlert);
|
||||
const reduxSessionId = useSelector(selectSessionId);
|
||||
|
||||
|
|
@ -401,29 +402,27 @@ const JKSessionScreen = () => {
|
|||
}, [sessionGuardsPassed, userTracks, hasJoined])
|
||||
|
||||
// Track sync: Sync tracks to server when session joined (3-call pattern matching legacy)
|
||||
// IMPORTANT: Wait for mixers to be ready before syncing to avoid race condition with mixer initialization
|
||||
useEffect(() => {
|
||||
if (!hasJoined || !sessionId || !jamClient) {
|
||||
if (!hasJoined || !sessionId || !server?.clientId || !mixersReady) {
|
||||
return;
|
||||
}
|
||||
|
||||
logger.debug('[Track Sync] Session joined, scheduling track sync calls');
|
||||
console.log('[Track Sync] Mixers ready, scheduling track sync calls');
|
||||
|
||||
// First sync: Initial setup (~1s after join)
|
||||
const timer1 = setTimeout(() => {
|
||||
logger.debug('[Track Sync] Executing first sync (1s)');
|
||||
dispatch(syncTracksToServer(sessionId, jamClient));
|
||||
dispatch(syncTracksToServer(sessionId, server.clientId));
|
||||
}, 1000);
|
||||
|
||||
// Second sync: Refinement (~1.4s after join)
|
||||
const timer2 = setTimeout(() => {
|
||||
logger.debug('[Track Sync] Executing second sync (1.4s)');
|
||||
dispatch(syncTracksToServer(sessionId, jamClient));
|
||||
dispatch(syncTracksToServer(sessionId, server.clientId));
|
||||
}, 1400);
|
||||
|
||||
// Third sync: Final config (~6s after join)
|
||||
const timer3 = setTimeout(() => {
|
||||
logger.debug('[Track Sync] Executing third sync (6s)');
|
||||
dispatch(syncTracksToServer(sessionId, jamClient));
|
||||
dispatch(syncTracksToServer(sessionId, server.clientId));
|
||||
}, 6000);
|
||||
|
||||
// Cleanup timers on unmount or if hasJoined/sessionId changes
|
||||
|
|
@ -432,9 +431,9 @@ const JKSessionScreen = () => {
|
|||
clearTimeout(timer2);
|
||||
clearTimeout(timer3);
|
||||
};
|
||||
// Note: jamClient intentionally NOT in deps to avoid re-running on jamClient reference changes
|
||||
// Note: server intentionally NOT in deps to avoid re-running on server reference changes
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [hasJoined, sessionId, dispatch])
|
||||
}, [hasJoined, sessionId, mixersReady, dispatch])
|
||||
|
||||
const joinSession = async () => {
|
||||
|
||||
|
|
|
|||
|
|
@ -237,7 +237,10 @@ const useMixerHelper = () => {
|
|||
|
||||
// Redux: organizeMixers is now a reducer, trigger it via action
|
||||
useEffect(() => {
|
||||
if (!currentSession || masterMixers.length === 0 || personalMixers.length === 0) return;
|
||||
if (!currentSession || masterMixers.length === 0 || personalMixers.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
dispatch(organizeMixers());
|
||||
}, [currentSession, masterMixers, personalMixers, dispatch]);
|
||||
|
||||
|
|
@ -308,7 +311,9 @@ const useMixerHelper = () => {
|
|||
// This ensures VU meter callbacks have immediate access to the ready state
|
||||
useEffect(() => {
|
||||
isReady.current = isReadyRedux;
|
||||
// console.log("useMixerHelper: isReady synced with Redux", isReadyRedux);
|
||||
if (isReadyRedux) {
|
||||
console.log('[useMixerHelper] Mixers ready, VU meters enabled');
|
||||
}
|
||||
}, [isReadyRedux]);
|
||||
|
||||
const getMixerByTrackId = useCallback((trackId, mode) => {
|
||||
|
|
@ -824,12 +829,13 @@ const useMixerHelper = () => {
|
|||
|
||||
// Change updateVU to use the ref
|
||||
const updateVU = useCallback((mixerId, mode, leftValue, leftClipping, rightValue, rightClipping) => {
|
||||
if (!isReady.current) return;
|
||||
if (!isReady.current) {
|
||||
return;
|
||||
}
|
||||
|
||||
const mixer = getMixer(mixerId, mode);
|
||||
|
||||
if (mixer) {
|
||||
// console.log("useMixerHelper: updateVU mixer", allMixersRef.current, mixerId, mode, mixer);
|
||||
updateVU3(mixer, leftValue, leftClipping, rightValue, rightClipping);
|
||||
}
|
||||
}, [getMixer, updateVU3]);
|
||||
|
|
|
|||
|
|
@ -17,11 +17,8 @@ const mockDispatch = jest.fn((action) => {
|
|||
const mockGetState = jest.fn();
|
||||
|
||||
describe('trackSyncService', () => {
|
||||
// Mock jamClient instance
|
||||
const mockJamClient = {
|
||||
clientID: 'test-client-uuid',
|
||||
GetClientID: () => 'test-client-uuid'
|
||||
};
|
||||
// Mock clientId
|
||||
const mockClientId = 'test-client-uuid';
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
|
|
@ -198,7 +195,7 @@ describe('trackSyncService', () => {
|
|||
backing_tracks: []
|
||||
});
|
||||
|
||||
const result = await syncTracksToServer('session-123', mockJamClient)(mockDispatch, mockGetState);
|
||||
const result = await syncTracksToServer('session-123', mockClientId)(mockDispatch, mockGetState);
|
||||
|
||||
expect(putTrackSyncChange).toHaveBeenCalledWith({
|
||||
id: 'session-123',
|
||||
|
|
@ -231,8 +228,7 @@ describe('trackSyncService', () => {
|
|||
|
||||
mockGetState.mockReturnValue(mockState);
|
||||
|
||||
const mockJamClientNoId = { clientID: null };
|
||||
const result = await syncTracksToServer('session-123', mockJamClientNoId)(mockDispatch, mockGetState);
|
||||
const result = await syncTracksToServer('session-123', null)(mockDispatch, mockGetState);
|
||||
|
||||
expect(putTrackSyncChange).not.toHaveBeenCalled();
|
||||
expect(result.skipped).toBe(true);
|
||||
|
|
@ -251,7 +247,7 @@ describe('trackSyncService', () => {
|
|||
|
||||
mockGetState.mockReturnValue(mockState);
|
||||
|
||||
const result = await syncTracksToServer(null, mockJamClient)(mockDispatch, mockGetState);
|
||||
const result = await syncTracksToServer(null, mockClientId)(mockDispatch, mockGetState);
|
||||
|
||||
expect(putTrackSyncChange).not.toHaveBeenCalled();
|
||||
expect(result.skipped).toBe(true);
|
||||
|
|
@ -271,7 +267,7 @@ describe('trackSyncService', () => {
|
|||
|
||||
mockGetState.mockReturnValue(mockState);
|
||||
|
||||
const result = await syncTracksToServer('session-123', mockJamClient)(mockDispatch, mockGetState);
|
||||
const result = await syncTracksToServer('session-123', mockClientId)(mockDispatch, mockGetState);
|
||||
|
||||
expect(putTrackSyncChange).not.toHaveBeenCalled();
|
||||
expect(result.skipped).toBe(true);
|
||||
|
|
@ -294,7 +290,7 @@ describe('trackSyncService', () => {
|
|||
const apiError = new Error('Network error');
|
||||
putTrackSyncChange.mockRejectedValue(apiError);
|
||||
|
||||
const result = await syncTracksToServer('session-123', mockJamClient)(mockDispatch, mockGetState);
|
||||
const result = await syncTracksToServer('session-123', mockClientId)(mockDispatch, mockGetState);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error).toBe(apiError);
|
||||
|
|
@ -320,7 +316,7 @@ describe('trackSyncService', () => {
|
|||
|
||||
putTrackSyncChange.mockResolvedValue(apiResponse);
|
||||
|
||||
await syncTracksToServer('session-123', mockJamClient)(mockDispatch, mockGetState);
|
||||
await syncTracksToServer('session-123', mockClientId)(mockDispatch, mockGetState);
|
||||
|
||||
// Should dispatch actions to update Redux state
|
||||
expect(mockDispatch).toHaveBeenCalled();
|
||||
|
|
|
|||
|
|
@ -54,29 +54,14 @@ export const buildTrackSyncPayload = (state, sessionId, clientId) => {
|
|||
* 5. Handles errors gracefully
|
||||
*
|
||||
* @param {string} sessionId - Session ID to sync tracks for
|
||||
* @param {Object} jamClient - jamClient instance from Context
|
||||
* @param {string} clientId - Client ID from server context
|
||||
* @returns {Function} Redux thunk
|
||||
*/
|
||||
export const syncTracksToServer = (sessionId, jamClient) => async (dispatch, getState) => {
|
||||
export const syncTracksToServer = (sessionId, clientId) => async (dispatch, getState) => {
|
||||
const state = getState();
|
||||
const { hasJoined } = state.activeSession;
|
||||
|
||||
// Guard: Check clientId exists
|
||||
// clientID is a function on the jamClient proxy, need to call it
|
||||
let clientId;
|
||||
try {
|
||||
if (typeof jamClient?.clientID === 'function') {
|
||||
clientId = await jamClient.clientID();
|
||||
} else if (typeof jamClient?.GetClientID === 'function') {
|
||||
clientId = await jamClient.GetClientID();
|
||||
} else {
|
||||
clientId = jamClient?.clientID || null;
|
||||
}
|
||||
} catch (error) {
|
||||
console.warn('[Track Sync] Error getting clientId:', error);
|
||||
clientId = null;
|
||||
}
|
||||
|
||||
if (!clientId) {
|
||||
console.warn('[Track Sync] Skipped: Client ID not available');
|
||||
return { skipped: true, reason: 'no_client_id' };
|
||||
|
|
@ -108,7 +93,7 @@ export const syncTracksToServer = (sessionId, jamClient) => async (dispatch, get
|
|||
// Call API
|
||||
const response = await putTrackSyncChange({ id: sessionId, ...payload });
|
||||
|
||||
console.log('[Track Sync] Success:', response);
|
||||
console.log('[Track Sync] Success:', response.tracks?.length || 0, 'tracks synced');
|
||||
|
||||
// Dispatch Redux actions to update state
|
||||
// TODO: Import actual action creators from activeSessionSlice
|
||||
|
|
|
|||
Loading…
Reference in New Issue