DRILL-4586: Add ErrorType#CLIENT to UserException for client side errors

[sudheeshkatkam]

• Resolve relevant TODOs

@parthchandra please review.

[parthchandra]

Per my understanding User Exception was meant for errors on the server side that are to bet sent back to the user. Are you extending that definition?

[jacques-n]

This also breaks client/wire compatibility. In previous discussions, we said that we would avoid breaking compatibility. If you proceed with this, you'll need to add a compatibility layer in the server that understands the different client versions and whether a server can return this value on the wire.

[sudheeshkatkam]

Per Hakim's comment, SYSTEM type is meant for unexpected errors that do not have a "nice" error message yet. However in some error cases on the client side, there can be nice error messages, as shown in these changes.

Regarding compatibility: the server should not produce CLIENT type user exceptions, so old clients can talk to new servers; mentioned in the CLIENT type description as some error on the client side.

[jacques-n]

It seems error-prone/dangerous to change the wire protocol for something that will never be sent on the wire.

[sudheeshkatkam]

For now, I am not sure how else to fix this; will punt until compatibility later is introduced.

[jacques-n]

Since it is all on the client/in the same jvm, why not just throw an exception and catch it? No need to jump through all the hoops that UserException goes through since you're never trying to propagate between multiple nodes.

[sudheeshkatkam]

I am confused here. The patch enhances the catch clauses of thrown exceptions and, in a way, "corrects" what was being reported to the user as a SYSTEM (server) error to CLIENT error since these exceptions happened on the client side (abd eventually a UserException needs to be reported).

I tried to extend UserException to ClientException, but no contextual information can be added because of how the UserException.Builder is setup. I will punt this.