Handle EOF when DuplexStream is closed, avoiding a panic. #77
Conversation
| }, | ||
| // Data from user, forward it to the device | ||
| from_server = server.read(&mut buf) => { | ||
| let len = from_server.map_err(duplex_write_error_fn)?; |
There was a problem hiding this comment.
Unrelated, but I notice on this read error you map_err using a write error fn, and the same at line 308
There was a problem hiding this comment.
Right, the name is wrong, I can change it later.
| self.radio | ||
| // TODO: remove the skipping of the first 4 bytes | ||
| .write(&self.toradio_char, &buffer[4..], WriteType::WithResponse) | ||
| .write( |
There was a problem hiding this comment.
I added this:
- to check how ConnectedStreamAPI would handle it, which it does fine and avoids the panic also (tested removing the len != 0 check first)
- for consideration as additional protection from panics due to any caller providing an invalid length buffer. Up to you to decide to include or something similar. I imagine there will be a very small runtime overhead with the check, but maybe negligible for the number of packets being sent?
| let len = from_server.map_err(duplex_write_error_fn)?; | ||
| ble_handler.write_to_radio(&buf[..len]).await?; | ||
| if len != 0 { | ||
| ble_handler.write_to_radio(&buf[..len]).await?; |
There was a problem hiding this comment.
I think we can also return Err(InternalStreamError::ConnectionLost) in case len == 0.
I can look into this later, you can merge this PR as it is an improvement.
There was a problem hiding this comment.
You will have to merge it I think.
There was a problem hiding this comment.
If the result of an explicit disconnect call, strictly speaking not an error - but will let you decide.
| }, | ||
| // Data from user, forward it to the device | ||
| from_server = server.read(&mut buf) => { | ||
| let len = from_server.map_err(duplex_write_error_fn)?; |
There was a problem hiding this comment.
Right, the name is wrong, I can change it later.
|
Re testing: I want to add tests for BLE using mocking later, this will be longer effort. |
2 participants
Fixes #75
Summary
Handle the case when
serverDuplexChannel is closed upon disconnecting stream, which causes the server.read() to return a length of 0 bytes. In that case do not call write_to_radio() with zero length.Related Issues
Fixes #75
Proposed Changes
Checklist
Tests pass locally
Documentation updated if needed - N/A