[
https://issues.apache.org/jira/browse/THRIFT-3781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15229109#comment-15229109
]
Jens Geyer commented on THRIFT-3781:
------------------------------------
Re exceptions: The usual pattern in Thrift is to have all Thrift-related
exceptions derived from a {{TException}} base class. At least it is one of the
best practices (not so sure if it is really a written design goal) is to have
the client libraries handle things in similar ways, a policy which has served
us quite well so far. What I can't say is, whether or not this pattern makes
any sense with Ruby.
> Thrift::Client can become permanently corrupt when Thrift::ProtocolException
> is raised
> --------------------------------------------------------------------------------------
>
> Key: THRIFT-3781
> URL: https://issues.apache.org/jira/browse/THRIFT-3781
> Project: Thrift
> Issue Type: Bug
> Components: Ruby - Library
> Affects Versions: 0.9.3
> Reporter: Justin Mills
>
> {{Thrift::Client#send_message_args}} rescues from {{StandardError}}, but
> {{Thrift::ProtocolException}} inherits from {{Exception}}. This means if you
> have an argument to a function that is not valid (ie, missing a required
> attribute), you can get the following error:
> {code}
> Thrift::ProtocolException: Required field address is unset!
> from
> ~/.rvm/gems/ruby-2.3.0/gems/my-thrift-gem-1.0/lib/my-thrift-gem/gen-rb/my-thrift-gem_types.rb:32:in
> `validate'
> from
> ~/.rvm/gems/ruby-2.3.0/gems/thrift-0.9.3.0/lib/thrift/client.rb:44:in `write'
> from
> ~/.rvm/gems/ruby-2.3.0/gems/thrift-0.9.3.0/lib/thrift/client.rb:44:in
> `send_message_args'
> from
> ~/.rvm/gems/ruby-2.3.0/gems/thrift-0.9.3.0/lib/thrift/client.rb:30:in
> `send_message'
> from
> ~/.rvm/gems/ruby-2.3.0/gems/my-thrift-gem-1.0/lib/my-thrift-gem/gen-rb/my_service.rb:22:in
> `send_example_1'
> from
> ~/.rvm/gems/ruby-2.3.0/gems/my-thrift-gem-1.0/lib/my-thrift-gem/gen-rb/my_service.rb:17:in
> `example_1'
> {code}
> Since ProtocolException is not a StandardError, the exception handler is not
> called, and the {{@oprot.trans.close}} line is not executed, leaving the
> transport with data in it's buffers. All subsequent calls will fail that try
> to re-use this client because of this data.
> It is also possible that this problem only occurs when the validation error
> happens in the 2nd and beyond arguments to a function, as only then is data
> likely to have been written.
> I can see a couple of potential fixes:
> - Make Thrift::ProtocolException inherit from StandardError
> - Change the rescue block in client.rb to rescue from all Thrift exceptions
> that do not inherit from StandardError
> - Have the client validate each parameter before attempting to serialize
> anything, ensuring that the data is more likely to serialize successfully.
> From a ruby perspective, I think the first of these is the best fix as I'm
> not sure a ProtocolException is something that warrants subclassing directly
> from Exception. It seems more aligned to IOError. Unless the thought here is
> that ProtocolExceptions are reserved for cases where the client should be
> abandoned and re-built. If that's the case, I think the last of the options
> might be best since data validation could be done prior to serialization and
> need not waste the effort of serialization, nor burn the client connection
> because of it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
