transport: Write errors not handled as fatal

[kennylevinsen]

Errors when writing messages are handled as non-fatal at the current state, letting the client make the decisions. A failed write is only okay if no data was written - if any data was written, the protocol is trashed and the connection must be let go. If the error is not handled as fatal, and another write is made (could have been queued already), the half-written message + the new message might be read as a valid message by the server which will then do something unexpected, such as responding to a tag the client never used, or writing random data to fids.

[stevvooe]

@joushou Could you send a PR that makes this fix?

[kennylevinsen]

Not sure if I can allocate the time right now, but the PR should be fairly easy to make. I also doubt that it's super high priority - normally, if a write fails, so will the next, and temporary errors are okay as long as no data was written, making problems unlikely. The issue is more about proper design.

[stevvooe]

For the most part, this was written to be fatal on any kind of error, so I am not sure what specifically you want changed.

A few questions:

Where in the code do you not see this as the case? What changes would you like to see happen? Is the problem that we aren't classifying temporary vs fatal errors or do you want the session to be poisoned after encountering an error? Should this happen to both the client and the server?

[stevvooe]

@joushou Found this little gem:

// BUG(stevvooe): There may be partial reads under
// timeout errors where this is actually fatal.

I'm not sure what the right fix here is. Let's fill out some unit test coverage in this code and reproduce these hypothetical situations. That will allow us to find the right error handling.