Skip to content

Commit e5f55af

Browse files
committed
address comments
1 parent 07bd151 commit e5f55af

File tree

4 files changed

+95
-58
lines changed

4 files changed

+95
-58
lines changed

apps/sim/app/workspace/[workspaceId]/providers/workspace-permissions-provider.tsx

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -59,40 +59,61 @@ export function WorkspacePermissionsProvider({ children }: WorkspacePermissionsP
5959
const hasOperationError = useOperationQueueStore((state) => state.hasOperationError)
6060
const addNotification = useNotificationStore((state) => state.addNotification)
6161
const removeNotification = useNotificationStore((state) => state.removeNotification)
62-
const { isReconnecting } = useSocket()
63-
const reconnectingNotificationIdRef = useRef<string | null>(null)
62+
const { isReconnecting, isRetryingWorkflowJoin } = useSocket()
63+
const realtimeStatusNotificationIdRef = useRef<string | null>(null)
64+
const realtimeStatusNotificationMessageRef = useRef<string | null>(null)
6465

6566
const isOfflineMode = hasOperationError
67+
const realtimeStatusMessage = isReconnecting
68+
? 'Reconnecting...'
69+
: isRetryingWorkflowJoin
70+
? 'Joining workflow...'
71+
: null
72+
73+
const clearRealtimeStatusNotification = useCallback(() => {
74+
if (!realtimeStatusNotificationIdRef.current) {
75+
return
76+
}
77+
78+
removeNotification(realtimeStatusNotificationIdRef.current)
79+
realtimeStatusNotificationIdRef.current = null
80+
realtimeStatusNotificationMessageRef.current = null
81+
}, [removeNotification])
6682

6783
useEffect(() => {
68-
if (isReconnecting && !reconnectingNotificationIdRef.current && !isOfflineMode) {
69-
const id = addNotification({
70-
level: 'error',
71-
message: 'Reconnecting...',
72-
})
73-
reconnectingNotificationIdRef.current = id
74-
} else if (!isReconnecting && reconnectingNotificationIdRef.current) {
75-
removeNotification(reconnectingNotificationIdRef.current)
76-
reconnectingNotificationIdRef.current = null
84+
if (isOfflineMode || !realtimeStatusMessage) {
85+
clearRealtimeStatusNotification()
86+
return
7787
}
7888

79-
return () => {
80-
if (reconnectingNotificationIdRef.current) {
81-
removeNotification(reconnectingNotificationIdRef.current)
82-
reconnectingNotificationIdRef.current = null
83-
}
89+
if (
90+
realtimeStatusNotificationIdRef.current &&
91+
realtimeStatusNotificationMessageRef.current === realtimeStatusMessage
92+
) {
93+
return
8494
}
85-
}, [isReconnecting, isOfflineMode, addNotification, removeNotification])
95+
96+
clearRealtimeStatusNotification()
97+
98+
const id = addNotification({
99+
level: 'error',
100+
message: realtimeStatusMessage,
101+
})
102+
103+
realtimeStatusNotificationIdRef.current = id
104+
realtimeStatusNotificationMessageRef.current = realtimeStatusMessage
105+
}, [addNotification, clearRealtimeStatusNotification, isOfflineMode, realtimeStatusMessage])
106+
107+
useEffect(() => {
108+
return clearRealtimeStatusNotification
109+
}, [clearRealtimeStatusNotification])
86110

87111
useEffect(() => {
88112
if (!isOfflineMode || hasShownOfflineNotification) {
89113
return
90114
}
91115

92-
if (reconnectingNotificationIdRef.current) {
93-
removeNotification(reconnectingNotificationIdRef.current)
94-
reconnectingNotificationIdRef.current = null
95-
}
116+
clearRealtimeStatusNotification()
96117

97118
try {
98119
addNotification({
@@ -107,7 +128,7 @@ export function WorkspacePermissionsProvider({ children }: WorkspacePermissionsP
107128
} catch (error) {
108129
logger.error('Failed to add offline notification', { error })
109130
}
110-
}, [addNotification, removeNotification, hasShownOfflineNotification, isOfflineMode])
131+
}, [addNotification, clearRealtimeStatusNotification, hasShownOfflineNotification, isOfflineMode])
111132

112133
const {
113134
data: workspacePermissions,

apps/sim/app/workspace/providers/socket-join-controller.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ describe('SocketJoinController', () => {
122122

123123
expect(errorResult.apply).toBe(false)
124124
expect(errorResult.retryScheduled).toBe(true)
125-
expect(errorResult.retriesExhausted).toBe(false)
126125
expect(errorResult.commands).toEqual([
127126
{
128127
type: 'schedule-retry',

apps/sim/app/workspace/providers/socket-join-controller.ts

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,22 @@ export type SocketJoinCommand =
1212
delayMs: number
1313
}
1414

15-
export interface SocketJoinSuccessResult {
15+
interface SocketJoinSuccessResult {
1616
apply: boolean
1717
commands: SocketJoinCommand[]
1818
ignored: boolean
1919
workflowId: string
2020
}
2121

22-
export interface SocketJoinErrorResult {
22+
interface SocketJoinErrorResult {
2323
apply: boolean
2424
commands: SocketJoinCommand[]
2525
ignored: boolean
2626
retryScheduled: boolean
27-
retriesExhausted: boolean
2827
workflowId: string | null
2928
}
3029

31-
export interface SocketJoinDeleteResult {
30+
interface SocketJoinDeleteResult {
3231
commands: SocketJoinCommand[]
3332
shouldClearCurrent: boolean
3433
}
@@ -45,14 +44,6 @@ export class SocketJoinController {
4544
private retryAttempt = 0
4645
private isConnected = false
4746

48-
getDesiredWorkflowId(): string | null {
49-
return this.desiredWorkflowId
50-
}
51-
52-
getPendingJoinWorkflowId(): string | null {
53-
return this.pendingJoinWorkflowId
54-
}
55-
5647
getJoinedWorkflowId(): string | null {
5748
return this.joinedWorkflowId
5849
}
@@ -171,20 +162,18 @@ export class SocketJoinController {
171162
commands: [...baseCommands, ...this.flush()],
172163
ignored: true,
173164
retryScheduled: false,
174-
retriesExhausted: false,
175165
workflowId: resolvedWorkflowId,
176166
}
177167
}
178168

179169
if (retryable && resolvedWorkflowId) {
180-
const retryResult = this.scheduleRetry(resolvedWorkflowId)
170+
const commands = this.scheduleRetry(resolvedWorkflowId)
181171

182172
return {
183173
apply: false,
184-
commands: [...baseCommands, ...retryResult.commands],
174+
commands: [...baseCommands, ...commands],
185175
ignored: false,
186-
retryScheduled: retryResult.retryScheduled,
187-
retriesExhausted: false,
176+
retryScheduled: true,
188177
workflowId: resolvedWorkflowId,
189178
}
190179
}
@@ -196,7 +185,6 @@ export class SocketJoinController {
196185
commands: [...this.clearRetryCommands(), ...leaveCommands, ...this.flush()],
197186
ignored: false,
198187
retryScheduled: false,
199-
retriesExhausted: false,
200188
workflowId: resolvedWorkflowId,
201189
}
202190
}
@@ -240,10 +228,7 @@ export class SocketJoinController {
240228
return [{ type: 'join', workflowId: this.desiredWorkflowId }]
241229
}
242230

243-
private scheduleRetry(workflowId: string): {
244-
commands: SocketJoinCommand[]
245-
retryScheduled: boolean
246-
} {
231+
private scheduleRetry(workflowId: string): SocketJoinCommand[] {
247232
const nextAttempt = this.retryWorkflowId === workflowId ? this.retryAttempt + 1 : 1
248233
const delayMs = Math.min(
249234
SOCKET_JOIN_RETRY_BASE_DELAY_MS * 2 ** Math.max(0, nextAttempt - 1),
@@ -253,17 +238,14 @@ export class SocketJoinController {
253238
this.retryWorkflowId = workflowId
254239
this.retryAttempt = nextAttempt
255240

256-
return {
257-
commands: [
258-
{
259-
type: 'schedule-retry',
260-
workflowId,
261-
attempt: nextAttempt,
262-
delayMs,
263-
},
264-
],
265-
retryScheduled: true,
266-
}
241+
return [
242+
{
243+
type: 'schedule-retry',
244+
workflowId,
245+
attempt: nextAttempt,
246+
delayMs,
247+
},
248+
]
267249
}
268250

269251
private takeRetryResetCommands(nextWorkflowId?: string | null): SocketJoinCommand[] {

apps/sim/app/workspace/providers/socket-provider.tsx

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ interface SocketContextType {
6161
isConnected: boolean
6262
isConnecting: boolean
6363
isReconnecting: boolean
64+
isRetryingWorkflowJoin: boolean
6465
authFailed: boolean
6566
currentWorkflowId: string | null
6667
currentSocketId: string | null
@@ -110,6 +111,7 @@ const SocketContext = createContext<SocketContextType>({
110111
isConnected: false,
111112
isConnecting: false,
112113
isReconnecting: false,
114+
isRetryingWorkflowJoin: false,
113115
authFailed: false,
114116
currentWorkflowId: null,
115117
currentSocketId: null,
@@ -146,6 +148,7 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
146148
const [isConnected, setIsConnected] = useState(false)
147149
const [isConnecting, setIsConnecting] = useState(false)
148150
const [isReconnecting, setIsReconnecting] = useState(false)
151+
const [isRetryingWorkflowJoin, setIsRetryingWorkflowJoin] = useState(false)
149152
const [currentWorkflowId, setCurrentWorkflowId] = useState<string | null>(null)
150153
const [currentSocketId, setCurrentSocketId] = useState<string | null>(null)
151154
const [presenceUsers, setPresenceUsers] = useState<PresenceUser[]>([])
@@ -236,10 +239,12 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
236239
commands.forEach((command) => {
237240
if (command.type === 'cancel-retry') {
238241
clearJoinRetryTimeout()
242+
setIsRetryingWorkflowJoin(false)
239243
return
240244
}
241245

242246
if (command.type === 'leave') {
247+
setIsRetryingWorkflowJoin(false)
243248
clearJoinedWorkflowState(true)
244249

245250
if (!socketInstance) {
@@ -278,6 +283,7 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
278283
}
279284

280285
clearJoinRetryTimeout()
286+
setIsRetryingWorkflowJoin(true)
281287
joinRetryTimeoutRef.current = setTimeout(() => {
282288
joinRetryTimeoutRef.current = null
283289
executeJoinCommands(joinControllerRef.current.retryJoin(command.workflowId))
@@ -376,6 +382,7 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
376382
socketInstance.on('disconnect', (reason) => {
377383
setIsConnected(false)
378384
setIsConnecting(false)
385+
setIsRetryingWorkflowJoin(false)
379386
setCurrentSocketId(null)
380387
executeJoinCommands(joinControllerRef.current.setConnected(false))
381388
clearJoinedWorkflowState(false)
@@ -469,6 +476,7 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
469476
if (result.ignored) {
470477
logger.debug(`Ignoring stale join-workflow-success for ${workflowId}`)
471478
} else {
479+
setIsRetryingWorkflowJoin(false)
472480
setVisibleWorkflowId(workflowId)
473481
setPresenceUsers(presenceUsers || [])
474482
logger.info(`Successfully joined workflow room: ${workflowId}`, {
@@ -495,6 +503,7 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
495503
code,
496504
})
497505
} else if (result.apply) {
506+
setIsRetryingWorkflowJoin(false)
498507
if (result.workflowId) {
499508
useOperationQueueStore.getState().cancelOperationsForWorkflow(result.workflowId)
500509
}
@@ -854,7 +863,19 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
854863
workflowId !== currentWorkflowIdRef.current ||
855864
!isWorkflowVisible(workflowId)
856865
) {
857-
logger.warn('Cannot emit subblock update: no socket connection', { workflowId, blockId })
866+
const reason = !socket
867+
? 'socket_unavailable'
868+
: workflowId !== currentWorkflowIdRef.current
869+
? 'joined_workflow_mismatch'
870+
: 'workflow_not_visible'
871+
872+
logger.debug('Skipping subblock update emit', {
873+
workflowId,
874+
blockId,
875+
subblockId,
876+
reason,
877+
currentWorkflowId: currentWorkflowIdRef.current,
878+
})
858879
return
859880
}
860881
socket.emit('subblock-update', {
@@ -882,7 +903,19 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
882903
workflowId !== currentWorkflowIdRef.current ||
883904
!isWorkflowVisible(workflowId)
884905
) {
885-
logger.warn('Cannot emit variable update: no socket connection', { workflowId, variableId })
906+
const reason = !socket
907+
? 'socket_unavailable'
908+
: workflowId !== currentWorkflowIdRef.current
909+
? 'joined_workflow_mismatch'
910+
: 'workflow_not_visible'
911+
912+
logger.debug('Skipping variable update emit', {
913+
workflowId,
914+
variableId,
915+
field,
916+
reason,
917+
currentWorkflowId: currentWorkflowIdRef.current,
918+
})
886919
return
887920
}
888921
socket.emit('variable-update', {
@@ -975,6 +1008,7 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
9751008
isConnected,
9761009
isConnecting,
9771010
isReconnecting,
1011+
isRetryingWorkflowJoin,
9781012
authFailed,
9791013
currentWorkflowId,
9801014
currentSocketId,
@@ -1003,6 +1037,7 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
10031037
isConnected,
10041038
isConnecting,
10051039
isReconnecting,
1040+
isRetryingWorkflowJoin,
10061041
authFailed,
10071042
currentWorkflowId,
10081043
currentSocketId,

0 commit comments

Comments
 (0)