Websocket disconnection sometimes causes instability

[zolstein]

Describe the bug
When a connection to a websocket server is lost and restored, Artisan will sometimes establish more than one connections to the server on reconnect. When this happens, when the roast is stopped, Artisan crashes.

This happens (to me) frequently but not reliably - it seems most common when the connection is lost for more than 10-15 seconds.

To Reproduce
Steps to reproduce the behavior:

1. Configure artisan to connect to a machine running a websocket server. Ideally, the server should report the number of active connections.
2. Start monitoring.
3. Reboot the machine running the websocket server (or otherwise cause the connection to drop unceremoniously from the server side).
4. Check for multiple websocket connections.
5. Stop monitoring.

Expected behavior
Even after a long dropped connection, Artisan should only establish one websocket connection. It should not crash.

Screenshots

Setup (please complete the following information):

• Artisan Version [e.g. v2.4.4]: v2.8.4
• Artisan Build (number in brackets shown in the about box) [e.g. 97e6dec]: ac17fbe
• Platform (Mac/Windows/Linux + OS version) [e.g. macOS 10.15.3]: Windows 10 (19045.4046)
• Connected devices or roasting machine [e.g. Probatone 5]: Custom device, Arduino, ESP32-C3 (But probably reproducible with a simple websocket server program.)

Additional context
Add any other context about the problem here.

Please attach your current Artisan settings file (as exported via menu Help >> Save Settings as *.aset) file.
Please attach any relevant Artisan *.alog profiles.

artisan-settings-bug-report.aset.txt

[MAKOMO]

Thanks for your report! Indeed the connect/disconnect handling for WebSockets seems to be not very clean. I tried to improve this with Commit 59e7186.

Could you please test the current Continuous Build and report if this performs any better?

[zolstein]

Thanks for the quick response!

I tried out the new build and was able to replicate almost the same bug. I could still cause it to create duplicate websocket connections, and when it stopped monitoring it still crashed, though now the crash was delayed a few seconds rather than immediate.

I've been trying to get this to run in a context with a debugger attached, but so far I haven't succeeded. (Seemingly, pdb stops showing the REPL and working when the window comes up.) I'll keep poking at it to see if I can get more information. If you can point me toward any changes that could get the debugger working or a log that would contain a stacktrace, that might also help.

[MAKOMO]

Ok. Only now I could reproduce the issue. I just removed the reconnect code which is not essentially needed as on next request the code automatically reconnects if not connected. Could you please test this again and report back?

In the end this needs to be reimplemented using asyncio and the WebSocket lib.

[MAKOMO]

This one is blocking the next Artisan release. Could you test my fix of the fix?

[zolstein]

Sorry, I'll test in the next day.

[zolstein]

I've tested with the most-recent patch and haven't been able to reproduce the issue. (Also, it seems like Artisan generally becomes responsive a bit more quickly after disconnecting, which is a nice bonus.)

[MAKOMO]

Thank you very much for testing and confirming that this now works as expected.

In the meantime I completely reimplemented the WebSocket port using a different library using the modern async IO pattern. If you did not run out of steam yet, maybe you can test this one as well. In my tests it worked at least as good as the fixed threading-based implementation, and I would prefer it as it is based on a more modern infrastructure. The just updated continuous build contains this re-implementation.

[zolstein]

Great. I'll probably take a look tomorrow.

[MAKOMO]

Excellent! Thanks you a lot!

[zolstein]

I did a test on the async websockets code and looks good to me.

[MAKOMO]

Thanks for this confirmation! We will switch to the async code for the release to be expected later today.