SSHReader: Handle the `asyncio.CancelledError` internally

[Kentzo]

Would be nice if asyncssh.StreamReader.readuntil handled asyncio.CancelledError by inserting an instance of asyncio.IncompleteReadError in the exception chain so that one could get the partial read without resorting to byte-by-byte reading via asyncssh.StreamReader.read

[ronf]

Are you doing this on SSHReader instances which are part of SSHClientProcess (stdout & stderr)? If so, there may already be a way to get what you want in the form of the wait() method, which will wait with an optional timeout and then return all of the data available so far from stdout and stderr. In the case of a timeout, it raises asyncssh.TimeoutError, which is a subclass of both asyncio.TimeoutError and asyncssh.ProcessError, the latter of which has additional members for returning a bunch of process information including the partial output so far on stdout and stderr.

Since the current version of readuntil() already returns an IncompleteReadError in the case of receiving EOF or a signal, I don't think it's a good idea to try and overload it to cover the case of TimeoutError or CancelledError. How would you tell the difference between a timeout, EOF, or another signal in this case? Also, I'm not sure how I would make a change like this backward compatible with existing behavior. It might be ok for TimeoutError if an explicit timeout argument were added to the readuntil() call which defaulted to None and triggered this new behavior only when explicitly set, but I'm not sure how do the same for CancelledError.

Can you give an example of what you were thinking the CancelledError version of this would look like in application code?

By the way, you should never need to resort to reading one byte at a time. When you call read() with an explicit length argument, the call will return immediately as soon as any data is received, up to the length specified. Only readexactly() will block until all of the requested number of bytes have been received.

[Kentzo]

I'm utilizing AsyncSSH for an expect-like tool. I'd like to use a timeout around readuntil to gracefully handle unexpected output. Currently I'm doing something like:

try:
    async with asyncio.timeout(...):
        return await stdout.readuntil(separator)
except asyncio.TimeoutError:
    data = await stdout.read(stdout.channel.get_recv_window())  # get all data received so far
    raise RuntimeError(f"expected to find {separator} but got {data} instead")

The goal is to get the data that was collected by readuntil up to the timeout.

[ronf]

The get_recv_window() isn't what you think. It indicates the amount of remaining space in the recv buffer before it will pause reading, not how much data is present in the buffer.

Unfortunately, whether you use get_recv_window() or just specify your own maximum amount of data to read, you run the risk of that call blocking if the receive buffer is currently empty, so calling read() may not be what you want if there's a chance the remove system might not output anything before the timeout occurs.

If you use create_process(), there's a method called collect_output() on the returned SSHClientProcess which should do what you want. It returns the accumulated data so far for both stdout and stderr without blocking. If you are requesting a PTY like most expect implementations do, all the output will be on stdout, though, so you can just ignore the stderr data. This would look something like:

try:
    async with asyncio.timeout(...):
        return await proc.stdout.readuntil(separator)
except asyncio.TimeoutError:
    data, _ = await proc.collect_output()  # get all data received so far
    raise RuntimeError(f"expected to find {separator} but got {data} instead")

In theory, I could actually add a method like this directly to SSHReader, but it sounds like for your use case the SSHClientProcess version might be a good fit and it gives your other advantages, like the ability to get back exit status information. You can even do stdin/stdout/stderr redirection from these process classes.