I think we can improve this comment by focusing on facts and changing its structure a bit, e.g. "Ending a connection on the other side results in ok flag being set to false. It makes it impossible to detect a failure based just on the flag. We plan to introduce additional mechanism which will allow distinguishing the outcomes in a task ENG-xxx." When that task is done we can replace the last sentence with the description of the solution (if necessary).

This status is used to determine if we want to write a message to stdout and is used as a parameter of Finish. What actually happens when a different status is passed to Finish?

Ok, that makes sense, but see my other comment with an idea of a generic solution

So in theory, this approach could result in client receiving OK when a failure occurred, right? What do you think about adding a virtual method that validates the data, is used here and determines whether we want to return OK or something else?

What data do you want to validate?

The same data you mentioned in the summary

We should manually check if the data we received is valid

So the plan is to always handle validation in handleRequest, right? It has some advantages, but in defense of introducing validate method, it would be more readable, as the responsibilities of these would be clearer. Also, it would be more intuitive to implement a subclass - the function name would dictate what's need to be done.

I'm not convinced that what you described makes it possible to handle correctly the validation. We should always call terminate at most once, so if we call it with OK in OnReadDone, there would be no way to tell the client that something actually went wrong. The solution where the validation happens in handleRequest would force us to call that method even when !ok which I find a bad practice.

So generally, the flow of OnReadDone would be to always call validate and when it reports some issues we would call terminate with proper error code.

This feels odd. I'd rather call validate in the terminate method

Could you explain what is odd in this approach? I think it is intuitive we're validating every chunk of data we receive, but maybe I'm missing something.

Having a validate method that should be called only when we finish doesn't sound too intuitive at first, but it makes some sense: we want to validate if the whole communication was successful. But at the same time, it would require storing all the data in memory which might be an issue - we should try to avoid being constrained by memory.

Regarding your proposed method, it has some issues:

1. If the status is not ok, do we really need to validate first or can we just finish with that status?
2. When do we want to call terminateCallback? We're calling it even when status is not ok, but we're not doing that when validation fails

We need to think through it and find some consistent solution.

Thanks for explaining this! Although I think that there's a lot of value in splitting the responsibilities here, it's also the case that with the current approach with a switch it's harder to implement. So I agree to avoid having to call validate after each message.