Fix #99410: SocketIO Reconnect Web Interface #104235

Eveline Anderson · 2023-07-13T20:14:51+02:00

Eveline Anderson commented

2023-07-13 20:14:51 +02:00

Fix #99410: Web interface: fetch version on SocketIO reconnect (and maybe reload)

A feature implement where after losing the SocketIO connection and subsequently reconnecting, the webapp should re-fetch the Flamenco Manager version and display it in the top-right corner. If it's different from before, then it will log a notification about the upgrade and refresh the entire page to ensure the new version is loaded properly.

Fix #99410: Web interface: fetch version on SocketIO reconnect (and maybe reload) A feature implement where after losing the SocketIO connection and subsequently reconnecting, the webapp should re-fetch the Flamenco Manager version and display it in the top-right corner. If it's different from before, then it will log a notification about the upgrade and refresh the entire page to ensure the new version is loaded properly.

Eveline Anderson added 1 commit 2023-07-13 20:14:54 +02:00

Retrieve current Flamenco version cd1011066f

Eveline Anderson changed title from ~~WIP: SocketIO Reconnect Web Interface~~ to WIP: Fix #99410: SocketIO Reconnect Web Interface

2023-07-13 20:16:12 +02:00

Eveline Anderson commented

2023-07-13 20:18:01 +02:00

The WIP is just to show that I haven't finished yet, but I wanted to put up a small change so you could guide me in the right direction. I do believe I'm in the right file. I am actively working on this and hope to finish by tomorrow or Monday (depending on my schedule). We can talk more on the call, though.

Sybren A. Stüvel reviewed 2023-07-17 11:01:45 +02:00

Sybren A. Stüvel left a comment

I think the code is ok, but it shouldn't be in JobsView.vue. This behaviour should be implemented for the entire web interface, and not just for the "Jobs" tab. Putting this in App.vue would be better. There is already code there that can fetch the Flamenco version. I think a good approach would be:

Write a function that's hooked up such that it's automatically called whenever the SocketIO connection comes back.
Make that fetch the version from the Manager, and compare it to this.flamencoVersion and this.flamencoName to see if a reload is necessary.

I think the code is ok, but it shouldn't be in `JobsView.vue`. This behaviour should be implemented for the entire web interface, and not just for the "Jobs" tab. Putting this in `App.vue` would be better. There is already code there that can fetch the Flamenco version. I think a good approach would be: - Write a function that's hooked up such that it's automatically called whenever the SocketIO connection comes back. - Make that fetch the version from the Manager, and compare it to `this.flamencoVersion` and `this.flamencoName` to see if a reload is necessary.

Eveline Anderson changed title from ~~WIP: Fix #99410: SocketIO Reconnect Web Interface~~ to Fix #99410: SocketIO Reconnect Web Interface

2023-07-18 19:53:26 +02:00

Eveline Anderson commented

2023-07-18 21:20:42 +02:00

I don't even want to begin to address all these commits... I messed up with the git pull. So, for now, until if I figure out the best way to squash this without deleting anybody's progress, I'll just write what I did in here (it's very small).

This is the hardest part and I'm still not 100% sure with SocketIO, but in the mounting, I was just trying to get some sort of connection to make an event listener. It was originally named "$socket" for a placeholder, but then I changed it to socketURL.

mounted() {
    const socketURL = 'http://localhost:8000' // TODO: Replace with Flamenco's server;
    this.socket = io(socketURL);
    this.socket.on('reconnect', this.socketIOReconnect);
}

This part was easier and does the actual job of verifying the job.

     socketIOReconnect() {
      this.fetchManagerInfo()

      if (this.flamencoVersion !== DEFAULT_FLAMENCO_VERSION || 
          this.flamencoName !== DEFAULT_FLAMENCO_NAME ) {
        console.log(`Upgraded Flamenco Version: ${this.flamencoVersion}`);

        // Reload the page,
        location.reload();
      }
    }

The biggest question I have is with is checking that socket. I know the first code block I provided is wrong, but I'm still trying to figure out how to get it to automatically check the socket's connection. I was looking at the https://socket.io/docs/v2/ to get some sort of picture of how I could get it.

Let me know what you think!

I don't even want to begin to address all these commits... I messed up with the git pull. So, for now, until if I figure out the best way to squash this without deleting anybody's progress, I'll just write what I did in here (it's very small). This is the hardest part and I'm still not 100% sure with SocketIO, but in the mounting, I was just trying to get some sort of connection to make an event listener. It was originally named "$socket" for a placeholder, but then I changed it to socketURL. ``` mounted() { const socketURL = 'http://localhost:8000' // TODO: Replace with Flamenco's server; this.socket = io(socketURL); this.socket.on('reconnect', this.socketIOReconnect); } ``` This part was easier and does the actual job of verifying the job. ``` socketIOReconnect() { this.fetchManagerInfo() if (this.flamencoVersion !== DEFAULT_FLAMENCO_VERSION || this.flamencoName !== DEFAULT_FLAMENCO_NAME ) { console.log(`Upgraded Flamenco Version: ${this.flamencoVersion}`); // Reload the page, location.reload(); } } ``` The biggest question I have is with is checking that socket. I know the first code block I provided is wrong, but I'm still trying to figure out how to get it to automatically check the socket's connection. I was looking at the https://socket.io/docs/v2/ to get some sort of picture of how I could get it. Let me know what you think!

Eveline Anderson force-pushed socketio-web-interface from 33c17dd41a to 88cce2258e

2023-07-18 23:40:36 +02:00

Compare

Sybren A. Stüvel reviewed 2023-07-19 17:09:32 +02:00

Sybren A. Stüvel left a comment

There are a few issues with the code as it is now.

The code creates a new SocketIO connection, where it should be responding to the reconnection of the already-existing SocketIO connection.

There's two ways to go about this:

There is a component that provides SocketIO connection info (stores/socket-status.js) which is used by components/ConnectionStatus.vue (i.e. const sockStatus = useSocketStatus();). Once you have the sockStatus object, its isConnected needs to be monitored / watched via Vue's reactivity system.
Let the 'views' that contain an UpdateListener component emit a sioReconnected event to bubble that event up to the Vue App, and write an event handler that deals with this.

There are a few issues with the code as it is now. The code creates a new SocketIO connection, where it should be responding to the reconnection of the already-existing SocketIO connection. There's two ways to go about this: 1. There is a component that provides SocketIO connection info (`stores/socket-status.js`) which is used by `components/ConnectionStatus.vue` (i.e. `const sockStatus = useSocketStatus();`). Once you have the `sockStatus` object, its `isConnected` needs to be monitored / watched via Vue's reactivity system. 2. Let the 'views' that contain an `UpdateListener` component emit a `sioReconnected` event to bubble that event up to the Vue App, and write an event handler that deals with this.

Eveline Anderson force-pushed socketio-web-interface from 88cce2258e to cd1011066f

2023-07-19 19:20:25 +02:00

Compare

Eveline Anderson added 1 commit 2023-07-19 19:22:33 +02:00

Merge remote-tracking branch 'upstream/main' into socketio-web-interface 65ad945ffb

Eveline Anderson added 1 commit 2023-07-19 19:24:26 +02:00

Revert changes of JobsView to original 7fec955873

Eveline Anderson added 1 commit 2023-07-19 22:18:48 +02:00

Add SocketIO reconnection and version check eaf6fd3148

Eveline Anderson commented

2023-07-19 22:30:11 +02:00

I think I worked out the commit issue. It wouldn't do much for the merge, so I did a reset, and then updated it to Flamenco's most recent branch. It seemed to do the trick for now, but I wanted to focus on the actual code. I also redid the writing of the socket connection, since it was making a new one. I did what you said and used the useSocketStatus() from the socket-status.js file. The way it was implemented (this.watch) was based off of something I read on google, so I'm not 100% confident in its ability, but my goal is to make it reactive and execute the socketIOReconnect() when the connection has been reestablished. So, I'm hoping that is what is accomplished.

I didn't do any changes to the socketIOReconnect(), so it's the same functionality as before. Do you think I should rename it to a better name? I'm not very happy with the current name "socketIOReconnect"... I think the name I chose could be a bit more clearer...

Thank you for being patient as well, I was really messed up from git, so I will try to do better to be more careful when using it as well.

I think I worked out the commit issue. It wouldn't do much for the merge, so I did a reset, and then updated it to Flamenco's most recent branch. It seemed to do the trick for now, but I wanted to focus on the actual code. I also redid the writing of the socket connection, since it was making a new one. I did what you said and used the `useSocketStatus()` from the `socket-status.js` file. The way it was implemented (`this.watch`) was based off of something I read on google, so I'm not 100% confident in its ability, but my goal is to make it reactive and execute the `socketIOReconnect()` when the connection has been reestablished. So, I'm hoping that is what is accomplished. I didn't do any changes to the `socketIOReconnect()`, so it's the same functionality as before. Do you think I should rename it to a better name? I'm not very happy with the current name "socketIOReconnect"... I think the name I chose could be a bit more clearer... Thank you for being patient as well, I was really messed up from git, so I will try to do better to be more careful when using it as well.

Sybren A. Stüvel requested changes 2023-07-20 11:36:20 +02:00

Sybren A. Stüvel left a comment

I'm not 100% confident in its ability

👍 for being sceptical about the code you find online :)

It's easy enough to test that things work -- just stop & restart Flamenco Manager, and the web interface should indicate a disconnect & a subsequent reconnect.

Before worrying about naming, focus on getting the functionality working. Once it works, and you have a better understanding of how things click together, you'll also have a better idea for the names.

> I'm not 100% confident in its ability :+1: for being sceptical about the code you find online :) It's easy enough to test that things work -- just stop & restart Flamenco Manager, and the web interface should indicate a disconnect & a subsequent reconnect. Before worrying about naming, focus on getting the functionality working. Once it works, and you have a better understanding of how things click together, you'll also have a better idea for the names.

web/app/src/App.vue

						
				@ -50,0 +55,4 @@

				    const sockStatus = useSocketStatus();

				    this.$watch(() => sockStatus.isConnected, (isConnected) => {

				      if (isConnected) {

Sybren A. Stüvel commented

2023-07-20 11:32:50 +02:00

This should also check sockStatus.wasEverDisconnected, otherwise it'll also respond to the initial connection.

This should also check `sockStatus.wasEverDisconnected`, otherwise it'll also respond to the initial connection.

dr.sybren marked this conversation as resolved

web/app/src/App.vue Outdated

						
				@ -59,1 +72,4 @@

				    },

				    socketIOReconnect() {

				      this.fetchManagerInfo()

Sybren A. Stüvel commented

2023-07-20 11:33:36 +02:00

This function shouldn't be called here, as it does too much. It not only fetches the current version, but also writes to this.flamencoVersion. This means that it overwrites the value that you want to compare against.

Comparing with DEFAULT_FLAMENCO_VERSION is always going to result in a difference, as the Manager will never return the string 'unknown'.

This function shouldn't be called here, as it does too much. It not only fetches the current version, but also writes to `this.flamencoVersion`. This means that it overwrites the value that you want to compare against. Comparing with `DEFAULT_FLAMENCO_VERSION` is *always* going to result in a difference, as the Manager will never return the string 'unknown'.

dr.sybren marked this conversation as resolved

web/app/src/App.vue Outdated

						
				@ -60,0 +74,4 @@

				    socketIOReconnect() {

				      this.fetchManagerInfo()

				      if (this.flamencoVersion !== DEFAULT_FLAMENCO_VERSION || this.flamencoName !== DEFAULT_FLAMENCO_NAME ) {

				        console.log(`Upgraded Flamenco Version: ${this.flamencoVersion}`);

Sybren A. Stüvel commented

2023-07-20 11:35:18 +02:00

For debugging, it's better to log both versions here: the one that was stored on the webapp startup, and the one that was found now. That way you can see "upgraded from X to Y" in the JS console.