[BUG/FEATURE?] "Command" should have a timeout associated with it...

[puterboy]

If the command for a "Custom" display hangs, then bad things seem to happen...
First, the 'testing' itself hangs
Second and more ominously, it seems like Octoprint is waiting for the function to complete which seems to prevent other Octoprint functions from running, including refreshing the Octoprint webui.

I could not even restart Octoprint from the webui (though it gave me the confirmatory dialog box, it failed to execute a restart) or even from the command line using "octoprint daemon stop" or "octoprint daemon restart"
I had to manually "kill -KILL" (i.e., kill -9)) the process to get it to stop and then had to restart manually from the command line.

Suggest either a fixed or a manually settable timeout. Also, ideally, the 'Test' button should have the ability to abort/start over if you press 'Test' again in case you don't want to wait for the timeout (or infinitely long if it hangs without a timeout capacity)

You can test this by giving "sleep 100000" as the command...

[puterboy]

Thanks! Looks like you fixed all the "bugs" I referenced :)
I would suggest perhaps having a timeout longer than 5 seconds -- because for instance some of the data I display, I get from querying other servers or even the Internet. For example, I like to display the outside temperature which I can get either by querying my weather server on my LAN or by using curl to send a web request to one of the standard weather databases -- my concern is sometimes those queries could take longer than 5 seconds.
Perhaps either let it be configurable or use a relative longer timeout such as 30 seconds which would really only be there to protect against command errors or servers being down

[puterboy]

BTW, any idea when you might push the next release containing the bug fixes you reference?

[LazeMSS]

Should be fixed now: https://github.com/LazeMSS/OctoPrint-TopTemp/releases/tag/0.0.2.1

[puterboy]

Thanks! Works!!

[puterboy]

Actually, it doesn't work for some commands.
If I put in a simple NOP command like '/bin/true' and press 'test', it never times out.

Note this may be related to #93 (see details there)

[LazeMSS]

reFixed in: https://github.com/LazeMSS/OctoPrint-TopTemp/releases/tag/0.0.2.3

[puterboy]

Actually, doesn't seem to be working.
I tried a simple test command like: sleep 10000000 && echo 99
And it failed to timeout...

Even worse, 'sleep N && echo 99' properly displays '99' after an N second delay when N < 30 (i.e., sleep time is less than the 30 second default timeout in your code)).
However, the test command hangs forever with N >= 30 -- i.e., it fails to return any result when the sleep time is greater than 30 second default timeout in your code.

So it seems like the process is presumably timing out in init.py but but somehow control is not returning to the configuration dialog box.

[puterboy]

Here is the error code in the log that happens when the timeout is triggered.

2023-05-15 14:26:53,499 - octoprint.plugins.toptemp - WARNING - "sleep 30 && echo 10" timed out
2023-05-15 14:26:53,500 - octoprint.server.api - ERROR - Error while executing SimpleApiPlugin toptemp
Traceback (most recent call last):
File "/home/user/octoprint/venv/lib/python3.7/site-packages/octoprint_toptemp/init.py", line 887, in runcommand
std_out, std_err = proc.communicate(timeout=self.cmdTimeout)
File "/usr/lib/python3.7/subprocess.py", line 939, in communicate
stdout, stderr = self._communicate(input, endtime, timeout)
File "/usr/lib/python3.7/subprocess.py", line 1682, in _communicate
self._check_timeout(endtime, orig_timeout)
File "/usr/lib/python3.7/subprocess.py", line 982, in _check_timeout
raise TimeoutExpired(self.args, orig_timeout)
subprocess.TimeoutExpired: Command 'sleep 30 && echo 10' timed out after 30 seconds

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/home/user/octoprint/venv/lib/python3.7/site-packages/octoprint/server/api/init.py", line 158, in pluginCommand
response = api_plugin.on_api_command(command, data)
File "/home/user/octoprint/venv/lib/python3.7/site-packages/octoprint/util/init.py", line 1688, in wrapper
return f(*args, **kwargs)
File "/home/user/octoprint/venv/lib/python3.7/site-packages/octoprint_toptemp/init.py", line 837, in on_api_command
code, out, err = self.runcommand(cmdInput)
File "/home/user/octoprint/venv/lib/python3.7/site-packages/octoprint_toptemp/init.py", line 891, in runcommand
return -1, """+cmd+"" timed out", "Maximum execution time, "+self.cmdTimeout+" seconds, exceeded!"
TypeError: can only concatenate str (not "int") to str

[puterboy]

Here is a patch to correct the error which seems to be caused by incorrect/confused concatenation of strings in the multiple returned values. I fixed it by using an f-string instead of '+' concatenation.

+++ __init__.py 2023-05-15 16:35:21.903442981 -0400
@@ -888,8 +888,7 @@
         except subprocess.TimeoutExpired:
             proc.kill()
             self._logger.warning("\""+cmd+"\" timed out")
-            return -1, "\""+cmd+"\" timed out", "Maximum execution time, "+self.cmdTimeout+" seconds, exceeded!"
-
+            return -1, f"\"{cmd}\" timed out", f"Maximum execution time, {self.cmdTimeout} seconds, exceeded!"
         return proc.returncode, std_out.strip(), std_err```

[puterboy]

Has this patch been added yet?