Skip to content

Commit bb82e49

Browse files
fix(app): Fix accidental polling when a query is initially disabled (#17853)
## Overview While testing EXEC-1342, we noticed a bug in our notifications machinery that would cause the app to poll even when MQTT was available and it shouldn't have to. ## Details Specifically, the bug would happen for queries that started off with `options` of `{enabled: false}` and became `{enabled: true}` later on. [Here is the motivating example](https://github.com/Opentrons/opentrons/blob/a3b78c28c0f9b51edbe68dc60908b4e3adc6cf2e/app/src/organisms/LabwarePositionCheck/LPCFlows/hooks/useLPCLabwareInfo/index.ts#L68-L77). The `useNotifyDataReady` hook had a piece of state, `isNotifyEnabled`, that would accidentally permanently latch to `false` whenever the hook saw `{enabled: false}`. ## Test Plan and Hands on Testing Tested as part of #17832 by monitoring the network logs. ## Risk assessment Medium. This is common code for a lot of stuff, and it's hard to test.
1 parent 394188c commit bb82e49

File tree

1 file changed

+7
-4
lines changed

1 file changed

+7
-4
lines changed

app/src/resources/useNotifyDataReady.ts

+7-4
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ export function useNotifyDataReady<TData, TError = Error>({
5858
const forcePollingFF = useFeatureFlag('forceHttpPolling')
5959
const seenHostname = useRef<string | null>(null)
6060
const [refetch, setRefetch] = useState<HTTPRefetchFrequency>(null)
61-
const [isNotifyEnabled, setIsNotifyEnabled] = useState(true)
61+
const [
62+
hasEncounteredNotificationsError,
63+
setHasEncounteredNotificationsError,
64+
] = useState(false)
6265

6366
const { enabled, staleTime, forceHttpPolling } = options
6467

@@ -80,8 +83,6 @@ export function useNotifyDataReady<TData, TError = Error>({
8083
})
8184
dispatch(notifySubscribeAction(hostname, topic))
8285
seenHostname.current = hostname
83-
} else {
84-
setIsNotifyEnabled(false)
8586
}
8687

8788
return () => {
@@ -98,7 +99,7 @@ export function useNotifyDataReady<TData, TError = Error>({
9899

99100
const onDataEvent = useCallback((data: NotifyResponseData): void => {
100101
if (data === 'ECONNFAILED' || data === 'ECONNREFUSED') {
101-
setIsNotifyEnabled(false)
102+
setHasEncounteredNotificationsError(true)
102103
if (data === 'ECONNREFUSED') {
103104
doTrackEvent({
104105
name: ANALYTICS_NOTIFICATION_PORT_BLOCK_ERROR,
@@ -120,6 +121,8 @@ export function useNotifyDataReady<TData, TError = Error>({
120121
[refetch, options.onSettled]
121122
)
122123

124+
const isNotifyEnabled =
125+
shouldUseNotifications && !hasEncounteredNotificationsError
123126
const queryOptionsNotify = {
124127
...options,
125128
onSettled: isNotifyEnabled ? notifyOnSettled : options.onSettled,

0 commit comments

Comments
 (0)