diff --git a/app-shell/src/notifications/connect.ts b/app-shell/src/notifications/connect.ts index 1804be14380..4b86271bb20 100644 --- a/app-shell/src/notifications/connect.ts +++ b/app-shell/src/notifications/connect.ts @@ -67,29 +67,30 @@ export function addNewRobotsToConnectionStore( ): Promise { return new Promise(() => { const newRobots = healthyRobots.filter(({ ip, robotName }) => { - const isNewIP = connectionStore.isIPNewlyDiscovered(ip) + const isIPInStore = connectionStore.isIPInStore(ip) const isIPAssociatedWithKnownRobot = connectionStore.isAssociatedWithExistingHostData( robotName ) - // Not a new robot, but a new IP for a robot present in connectionStore. - if (isNewIP && isIPAssociatedWithKnownRobot) { - // Pass until the next discovery-client poll so the current connection can resolve. - if (!connectionStore.isAssociatedBrokerConnected(ip)) { - return false - } - // Robot is reachable on an associated IP. Do not connect on this IP. - if (connectionStore.isAssociatedBrokerReachable(ip)) { - void connectionStore.associateIPWithExistingHostData(ip, robotName) + if (!isIPInStore && isIPAssociatedWithKnownRobot) { + if (!connectionStore.isAssociatedBrokerErrored(robotName)) { + // If not yet connected, pass until the next discovery-client poll so the current connection can resolve. + if (connectionStore.isAssociatedBrokerConnected(robotName)) { + void connectionStore.associateIPWithExistingHostData(ip, robotName) + } return false } // The broker isn't reachable on existing IPs. - // Mark this IP as a new broker connection to see if the broker is reachable on this IP. else { - void connectionStore.deleteAllAssociatedIPsGivenRobotName(robotName) - return true + // Mark this IP as a new broker connection to see if the broker is reachable on this IP. + if (!connectionStore.isKnownPortBlockedIP(ip)) { + void connectionStore.deleteAllAssociatedIPsGivenRobotName(robotName) + return true + } else { + return false + } } } else { - return isNewIP + return !isIPInStore && !connectionStore.isKnownPortBlockedIP(ip) } }) newRobots.forEach(({ ip, robotName }) => { @@ -190,9 +191,7 @@ function establishListeners( }) client.on('end', () => { - void connectionStore - .deleteAllAssociatedIPsGivenIP(ip) - .then(() => notifyLog.debug(`Closed connection to ${robotName} on ${ip}`)) + notifyLog.debug(`Closed connection to ${robotName} on ${ip}`) }) client.on('disconnect', packet => { @@ -208,14 +207,15 @@ function establishListeners( export function closeConnectionsForcefullyFor( hosts: string[] ): Array> { - return hosts.map(hostname => { - const client = connectionStore.getClient(hostname) + return hosts.map(ip => { + const client = connectionStore.getClient(ip) return new Promise((resolve, reject) => { if (client != null) { client.end(true, {}) } + const robotName = connectionStore.getRobotNameFromIP(ip) as string void connectionStore - .deleteAllAssociatedIPsGivenIP(hostname) + .deleteAllAssociatedIPsGivenRobotName(robotName) .then(() => resolve()) }) }) diff --git a/app-shell/src/notifications/store.ts b/app-shell/src/notifications/store.ts index 8fe0f8070fd..660c5101234 100644 --- a/app-shell/src/notifications/store.ts +++ b/app-shell/src/notifications/store.ts @@ -26,6 +26,8 @@ class ConnectionStore { private browserWindow: BrowserWindow | null = null + private readonly knownPortBlockedIPs = new Set() + public getBrowserWindow(): BrowserWindow | null { return this.browserWindow } @@ -59,15 +61,6 @@ class ConnectionStore { return Object.keys(this.hosts) } - public getAssociatedIPsFromIP(ip: string): string[] { - if (ip in this.hosts) { - const robotName = this.hosts[ip].robotName - return Object.keys(this.hosts).filter( - ip => this.hosts[ip].robotName === robotName - ) - } else return [] - } - public getAssociatedIPsFromRobotName(robotName: string): string[] { return Object.keys(this.hosts).filter( ip => this.hosts[ip].robotName === robotName @@ -86,7 +79,7 @@ class ConnectionStore { public setPendingConnection(ip: string, robotName: string): Promise { return new Promise((resolve, reject) => { - if (!this.isAssociatedWithExistingHostData(robotName)) { + if (!this.isAssociatedBrokerConnecting(robotName)) { this.hosts[ip] = { robotName, client: null, @@ -99,7 +92,7 @@ class ConnectionStore { } else { reject( new Error( - 'Cannot create a new connection if IP is associated with an existing connection' + 'Cannot create a new connection while connecting on an associated IP.' ) ) } @@ -123,7 +116,8 @@ class ConnectionStore { /** * - * @description Adds the host as unreachable with an error status derived from the MQTT returned error object. + * @description Marks the host as unreachable with an error status derived from the MQTT returned error object. + * */ public setFailedConnection(ip: string, error: Error): Promise { return new Promise((resolve, reject) => { @@ -135,6 +129,9 @@ class ConnectionStore { : FAILURE_STATUSES.ECONNFAILED this.hosts[ip].unreachableStatus = errorStatus + if (errorStatus === FAILURE_STATUSES.ECONNREFUSED) { + this.knownPortBlockedIPs.add(ip) + } resolve() } else { reject(new Error('IP is not associated with a connection')) @@ -208,18 +205,6 @@ class ConnectionStore { }) } - // Deleting associated IPs does not prevent re-establishing the connection on an associated IP if an - // associated IP is discoverable. - public deleteAllAssociatedIPsGivenIP(ip: string): Promise { - return new Promise((resolve, reject) => { - const associatedHosts = this.getAssociatedIPsFromIP(ip) - associatedHosts.forEach(ip => { - delete this.hosts[ip] - }) - resolve() - }) - } - public deleteAllAssociatedIPsGivenRobotName( robotName: string ): Promise { @@ -232,28 +217,39 @@ class ConnectionStore { }) } - public isIPNewlyDiscovered(ip: string): boolean { - return !(ip in this.hosts) + public isIPInStore(ip: string): boolean { + return ip in this.hosts } public isAssociatedWithExistingHostData(robotName: string): boolean { return this.getAssociatedIPsFromRobotName(robotName).length > 0 } - public isAssociatedBrokerReachable(ip: string): boolean { - const associatedRobots = this.getAssociatedIPsFromIP(ip) - return this.isBrokerReachable(head(associatedRobots) as string) + public isAssociatedBrokerErrored(robotName: string): boolean { + const associatedRobots = this.getAssociatedIPsFromRobotName(robotName) + return this.isBrokerErrored(head(associatedRobots) as string) } - public isAssociatedBrokerConnected(ip: string): boolean { - const associatedIPs = this.getAssociatedIPsFromIP(ip) + public isAssociatedBrokerConnected(robotName: string): boolean { + const associatedIPs = this.getAssociatedIPsFromRobotName(robotName) return this.isConnectedToBroker(head(associatedIPs) as string) } + public isAssociatedBrokerConnecting(robotName: string): boolean { + const associatedIPs = this.getAssociatedIPsFromRobotName(robotName) + return this.isConnectingToBroker(head(associatedIPs) as string) + } + public isConnectedToBroker(ip: string): boolean { return this.hosts[ip]?.client?.connected ?? false } + public isConnectingToBroker(ip: string): boolean { + return ( + (this.hosts[ip]?.client == null ?? false) && !this.isBrokerErrored(ip) + ) + } + public isPendingSub(ip: string, topic: NotifyTopic): boolean { if (ip in this.hosts) { const { pendingSubs } = this.hosts[ip] @@ -285,13 +281,17 @@ class ConnectionStore { * * @description Reachable refers to whether the broker connection has returned an error. */ - public isBrokerReachable(ip: string): boolean { + public isBrokerErrored(ip: string): boolean { if (ip in this.hosts) { - return this.hosts[ip].unreachableStatus == null + return this.hosts[ip].unreachableStatus != null } else { - return false + return true } } + + public isKnownPortBlockedIP(ip: string): boolean { + return this.knownPortBlockedIPs.has(ip) + } } export const connectionStore = new ConnectionStore() diff --git a/app-shell/src/notifications/subscribe.ts b/app-shell/src/notifications/subscribe.ts index b6c43067008..d33fc55a4dc 100644 --- a/app-shell/src/notifications/subscribe.ts +++ b/app-shell/src/notifications/subscribe.ts @@ -19,7 +19,7 @@ const CHECK_CONNECTION_INTERVAL = 500 export function subscribe(ip: string, topic: NotifyTopic): Promise { const robotName = connectionStore.getRobotNameFromIP(ip) - if (!connectionStore.isBrokerReachable(ip)) { + if (connectionStore.isBrokerErrored(ip)) { const errorMessage = connectionStore.getFailedConnectionStatus(ip) if (errorMessage != null) { sendDeserialized({