[ 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)