[
https://issues.apache.org/jira/browse/PROTON-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14214734#comment-14214734
]
Darryl L. Pierce commented on PROTON-743:
-----------------------------------------
Thank you for the patch! Here's some feedback from reading it:
Is there any need for the impl to be available to anything outside of the
object? I would suggest removing the attr_reader :impl until such a time as
there's a need to expose that detail outside of the class.
I think the methods to set errors on an object, such as Messenger, should
ultimately be in Messenger itself rather than being done through an instance of
another class. Unless the Messenger class is going to have a single instance of
Error to represent the most recent error, I'm not sure there's much benefit to
having an external object be able to change the state that way.
The Error method shouldn't really interact with the underlying error source. It
should be stateful and have the error code and error message given to it as a
part of its initializer, rather than giving it a reference to the object
itself. Imagine:
1. Messenger experiences a problem raises an error resulting in e[0] being
created.
2. The code beings to display details of e[0], but doesn't get to the text
message, but
3. Messenger experiences another problem and raises error e[1], so
4. When the code shows the text via e[0] it shows the current error's message
which is actually for e[1].
So unless there's only going to be one instance of Qpid::Proton::Error for each
instance of Qpid::Proton::Messenger, which doesn't seem to be the intent, then
it's very likely that an Error object will return incorrect information.
> [PATCH] ruby: user doesn't have access to clear messenger error object
> ----------------------------------------------------------------------
>
> Key: PROTON-743
> URL: https://issues.apache.org/jira/browse/PROTON-743
> Project: Qpid Proton
> Issue Type: Bug
> Components: ruby-binding
> Affects Versions: 0.8
> Reporter: Dominic Evans
> Assignee: Darryl L. Pierce
> Priority: Minor
> Attachments:
> 0001-Wrapper-pn_error-so-user-has-more-control-over-it.patch
>
>
> Once messenger error has been set and it has been read from messenger.error
> and logged / acted upon, it would be useful for the user to be able to clear
> the error object so they can continue operating. Wrappering it up in an Error
> class makes this easier.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
