Improve fallback to http

[jsoules]

Fix issue #66. Obviates (& undoes) #68.

(Git history is a little confusing--I had originally branched this off the branch from PR #78, so there's weirdness.)

There are two main barriers to seamless HTTP fallback:

1. Broadening the conditions of the postApiRequest function so that it falls back in a wider variety of cases.
2. Refactoring the connect-first busy-waiting during startup.

Changes

Incidental Changes:

• A couple temporary changes for testing the effect of the other changes (all changes in service tree).
• As part of broadening the criteria in postApiRequest, I've removed web RTC connection status from the MCMCMonitorData client-side cache. Instead, relevant components can simply access the singleton connection object (if it exists) directly. (Changes to files in MCMCMonitorDataManager directory, to corresponding tests, and in components/ConnectionStatusWidget)
• An earlier revision to the code resulted in an ultimately insignificant, but annoying, error being thrown during spin-up of webrtc connections, due to a circular import in the src/config.ts global configuration setter. This can be fixed by deferring the actual initiation of connection with a setTimeout call.
• Since we no longer ever cause a fatal error due to failed WebRTC connection, part of MainWindow.tsx was no longer relevant (the WebRtcError component).
• Updated postApiRequest tests to reflect and test current intended behavior.

Major changes:

• Updating the fallback condition for postApiRequest is quite easy--that's the one-line change in postApiRequest.ts.
• Updating the connection-initiation process is a little bit more involved. That's the changes in src/MCMCMonitorDataManager/SetupMCMCMonitor.tsx and src/networking/WebrtcConnectionToService.ts.
  • In the former, I've removed the logic that tries to interpret the web rtc connection status.
  • In the latter, I've updated the connection loop. Previously we had a timer within the function and a hard loop that blocked while the status was pending. I've updated this to make the timer an instance variable (so it persists between calls to connect()) and have the connect function call itself on a timer. This allows other processing to proceed while the connection is being established. Any requests that come through during that lag time will see that the WebrtcConnection's status is not connected and fall through to HTTP.

Testing

We haven't really begun automated integration testing, so I don't have anything automated to test this--though the updated unit tests give confidence that everything should still work. The more important test here was practical.

I checked this out in a couple ways, running both the GUI and service on my local machine:

• Confirm that non-proxy connections work. No webrtc expected; connection works fine.
• Use proxy link with webrtc=0; confirm that data transmits successfully, connection status shows using proxy without webrtc.
• Set webrtc=1 in URL; confirm that data transmits successfully, connection status shows using proxy with webrtc.
• Keep webrtc=1. Uncomment the line in service/src/networking/RemotePeer.ts reading throw Error('Manually bypassing webrtc support') to simulate unconnectable remote service. Confirm that data is transmitted and connection status shows using proxy without webrtc. Recomment this line.
• Keep webrtc=1. Add a setTimeout call around l. 52-54 in service/src/networking/Server.ts (the call to createPeer) to impose a 5-second delay in initiating the connection. Confirmed that data can still be transferred to the GUI and that the connection status indicates a webRTC connection is pending, until it eventually times out and says the status is errored. In both cases, data is still transferring via HTTP.

Potential concern: I tested fake slowness, but was not able to hit the case where a connection is pending for a while, then eventually connects. I think this had to do with how I was imposing lag in the service. I am assuming that in an actual longer negotiation process it would eventually connect--after all, connecting clearly works fine--but we may want to find a better way to confirm this case before merging.

GUI changes deployed to dev branch and visible at https://flatironinstitute.github.io/mcmc-monitor/dev/ (though seeing anything interesting will require running and connecting to a local service).

[codecov-commenter]

Codecov Report

Merging #79 (e82be77) into main (2b979c7) will increase coverage by 0.20%. The diff coverage is 42.55%.

@@           Coverage Diff            @@
##            main     #79      +/-   ##
========================================
+ Coverage   9.54%   9.75%   +0.20%     
========================================
  Files         74      74              
  Lines       5677    5640      -37     
  Branches     112     111       -1     
========================================
+ Hits         542     550       +8     
+ Misses      5135    5090      -45     

Flag	Coverage Δ
gui_units	12.33% <44.94%> (+0.30%)	:arrow_up:
svc_units	2.27% <0.00%> (-0.01%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
service/src/networking/RemotePeer.ts	0.00% <0.00%> (ø)
service/src/networking/Server.ts	0.00% <0.00%> (ø)
...rc/MCMCMonitorDataManager/MCMCMonitorTypeguards.ts	0.00% <0.00%> (ø)
src/MCMCMonitorDataManager/SetupMCMCMonitor.tsx	0.00% <0.00%> (ø)
src/MCMCMonitorDataManager/useMCMCMonitor.ts	0.00% <ø> (ø)
src/components/ConnectionStatusWidget.tsx	0.00% <0.00%> (ø)
src/config.ts	0.00% <0.00%> (ø)
src/pages/MainWindow.tsx	0.00% <0.00%> (ø)
src/MCMCMonitorDataManager/MCMCMonitorData.ts	100.00% <100.00%> (ø)
src/networking/WebrtcConnectionToService.ts	100.00% <100.00%> (ø)
... and 1 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[magland]

Looks great to me!

[jsoules]

Glad to hear! Are there any additional checks you think we should do before publishing? I'd love to demonstrate that it picks up a connection after some delay, but I'm not sure how to simulate that in a way that doesn't just result in the connection timing out...

[magland]

Just for a one-off test you could put a delay at the final connection step, like the following (i wouldn't built this in to the test framework necessarily thought)

peer.on('connect', () => {
            console.info('Webrtc connection established')
            this.#status = 'connected'
        })

you could replace by

peer.on('connect', () => {
            console.log('Finalizing connection in 3 seconds')
            setTimeout(() => {
              console.info('Webrtc connection established')
              this.#status = 'connected'
            }, 3000)
        })

[jsoules]

That does behave as expected... I think I'll go ahead and push this. (We can always roll back if we start getting reports of errors or instability.)