doc: ProtocolError and PermissionDenied are for client bugs

[DemiMarie]

The exceptions ProtocolError and PermissionDenied are both raised by qubesd to indicate various problems with requests. However, both cause clients to print the extremely unhelpful "Got empty response from qubesd" message. Therefore, they should only be used for client programming errors (bugs), not for problems that the client could not have detected before making the request.

The current code does not obey these rules, but these violations are bugs and should eventually be fixed.

[marmarek]

I don't like this approach, it more or less says "don't use those" which reduces granularity of errors. There may be cases where error message is intentionally not sent to the user to not leak info that shouldn't be available to the caller, but that's probably very limited case. Maybe a better approach is to simply improve the error message, or send ProtocolError/PermissionDenied exceptions back to the user with a generic message?

PS black is not happy with your changes

[DemiMarie]

The problem is that PermissionDenied is heavily misused. Many if not most uses of enforce are for input validation, not for permission checks. PermissionDenied cannot be the correct exception to throw in that case.

[marmarek]

So, maybe enforce should raise ProtocolError instead?

[DemiMarie]

I think so, yes, but some of the uses of enforce() are for problems that a client cannot realistically know of before making the call. These should raise a better exception that explains exactly what went wrong.

My understanding is:

• ProtocolError is for client programming errors (bugs). It means that the client made a request that it should have known better than to send in the first place. This would include things like passing an argument or payload to a service documented to not take one. The only way that a correctly behaving client can get ProtocolError is qubesd is either buggy or too old. In HTTP, this would be 400 Bad Request.
• PermissionDenied means that the request is valid, but the caller does not have permission to perform the operation. Callers in dom0 should usually not get this error. In HTTP, this would be 403 Forbidden.
• Things like “no such snapshot” should use neither PermissionDenied nor ProtocolError. Instead, they should use a custom exception type.
• Dropping the connection without sending a response indicates an unexpected internal error in dom0. If any details are available, they will be in dom0 logs. In HTTP, this would be 500 Internal Server Error.

[marmarek]

Yes, the above is quite good summary,

[marmarek]

In fact, it would be good to adjust this PR to include the above explanation. And then fix exceptions usage (in this PR or another).