[THRIFT-3607] Unify exception handling policy of TProcessor

• Type: Improvement
• Status: Open
• Priority: Major
• Resolution: Unresolved
• Component/s: Wish List
• Assignee: Unassigned
• Reporter: Nobuaki Sukegawa
• Created: 10/Feb/16 10:04
• Updated: 01/Feb/19 22:44

Description

A discussion in THRIFT-1805 uncovered inconsistent error handling behaviors of TProcessors across languages and releases.
Most outstanding are Java sync and async processors as described there, but others are also subtly different in details.

I propose unifying the TProcessor behavior by specifying mappings from "uncaught server handler exceptions" to "observable server behaviors" as follows:

• TApplicationException -> send TApplicationException with the message and the type as thrown
• TTransportException -> the connection is either already broken or newly broken
• other exceptions -> send opaque TApplicationException(INTERNAL_ERROR)

That way users can still arbitrarily disconnect the client by throwing TTransportException.
(IMO ideally this should have been done by exposing "client context" object to the handler instead)

The first one can be a bit controversial as it can be regarded as information leak.

Also some may prefer unifying the TProcessor behavior to catch-all, so that servers never die on handler exceptions.

Issue Links

• is blocked by: THRIFT-1805 Thrift should not swallow ALL exceptions
• is related to: THRIFT-3600 Make TTwisted server send exception on unexpected handler error
• is related to: THRIFT-3602 Make Tornado server send exception on unexpected handler error
• is related to: THRIFT-3680 Java async processor fails to notify errors to clients
• relates to: THRIFT-1805 Thrift should not swallow ALL exceptions
• links to: GitHub Pull Request #1240

Sub-Tasks

• Document server uncaught exception behavior: Open