tokio-modbus 0.12 entering unreachable code in any response error

[gustavowd]

Hi,

Since tokio-modbus 0.12 i'm facing several application crashs. The reason is that in any returning error code the tokio-modbus enters an unreachable code. I'm communicating with a commercial equipament, which sometimes returns a invalid response. In tokio-modbus 0.11 a handle this erros, but now it always causes a application crash.

Mar 26 08:12:14 raspberrypi gateway.sh[29153]: thread 'main' panicked at /home/gateway/.cargo/registry/src/index.crates.io-1cd66030c949c28d/tokio-modbus-0.12.0/src/client/mod.rs:230:25: Mar 26 08:12:14 raspberrypi gateway.sh[29153]: internal error: entered unreachable code: unexpected response code: 4 (request code: 3) Mar 26 08:12:14 raspberrypi gateway.sh[29153]: note: Please report the issue at https://github.com/slowtec/tokio-modbus/issues with a minimal example reproducing this bug. Mar 26 08:12:14 raspberrypi gateway.sh[29153]: note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

Thanks in advance, Gustavo

[uklotzde]

@creberust Any ideas how to handle those cases?

We probably need to add a new error variant that allows clients to take care of handling those errors.

[uklotzde]

https://github.com/slowtec/tokio-modbus/pull/248#issuecomment-1955226882

Looks like some paths are marked as unreachable, although they could be reached if a server sends an invalid/unexpected response.

[creberust]

Hi,

We can remove the unreachable! parts, but we need to add an Error variant, or return an io::Error ?

I think we can return an std::io::Error with ErrorKind::InvalidData in case of Request/Response mismatch. I don't know how you handled it before ?

Would it be simple for you @gustavowd to handle the Error with this ?

[uklotzde]

I'll prepare a proposal.

[uklotzde]

Please review and test #254 thoroughly! ...because I would like to avoid another breaking change.

[uklotzde]

Due to this serious panic issue I have decided to yank v0.12.0. Not recommended for production use.

[gustavowd]

Hi,

We can remove the unreachable! parts, but we need to add an Error variant, or return an io::Error ?

I think we can return an std::io::Error with ErrorKind::InvalidData in case of Request/Response mismatch. I don't know how you handled it before ?

Would it be simple for you @gustavowd to handle the Error with this ?

I beleive that @uklotzde proposal solves the issue. Yes, it is simple to handle the error with a returning error.