The new `Handler` definition makes it harder to build state machines

[elegaanz]

Previously, functions in Handler were taking an owned reference to self. Since v0.44 they take a &mut self.

The previous design allowed to easily model the state of the app as a state machine, which I believe is considered to be the canonical way to implement networking applications (or at least, is a very common way to do it). The new design is not working so well for that because you can't easily move fields that you want to keep for the next state. It is also easier to leave the application in an incoherent state, by returning from the function without updating the state at all (while the previous design forced you to build a new state, or to explicitly keep it as is).

I couldn't really find what motivated this change by browsing this repository, but there were probably good reasons to make it happen. So I'm opening this issue to understand what are the benefits of the new signatures, and have a discussion about it.

[Eugeny]

There isn't any particular reason beyond simplification - it's unusual for a library to require this sort of a to-and-fro passing of the handler object, and it made method signatures complex and the logic of returning Self unobvious.

I suspect at least some of it is due to the old thrussh Handler design pre-async_trait.

The Handler interface is seems to be close enough to a state machine but actually doesn't safely map to it - a lot of it is just verbal agreement that some methods won't be called in some situations (e.g. auth_xxx won't happen after the connection is authenticated etc.), so writing a state machine Handler still requires you to manually check for these invariants.

But disregarding all that, you can actually still have the Handler be a FSM enum - you still can overwrite *self from the &mut self handler methods

[elegaanz]

Okay, I see, thank you.