[
https://issues.apache.org/jira/browse/QPID-498?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12507880
]
Rafael H. Schloming commented on QPID-498:
------------------------------------------
I agree we should always throw an exception if someone tries to encode a bad
value. My point is simply that we shouldn't be throwing a protocol error since
a protocol error specifically indicates that the other end of the
connection/channel encountered an error *and closed the connection/channel*,
whereas in this case the other end of the connection will never encounter an
error because we abort the writing attempt before ever sending any bad data.
This will lead to bugs since the expected action taken by the user of the
python API is different in each case.
In case one (struct.pack("!H", 65538)) the user is trying to encode a bad
value. This is always guaranteed to be a programming error on the part of the
user or python library operating at the sending end of the connection. When
this situation is encountered the appropriate exception would be a ValueError,
or perhaps a codec specific exception akin to a ValueError. The user or library
would then catch this exception in some catchall piece of code somewhere and
then send a protocol-error ("internal-error", 541) indicating that there was an
internal problem at this end and this end of the connection has been shutdown,
and the other end should proceed to shutdown it's end of the connection/channel.
In case two when a channel or connection exception is spontaneously received
from the peer, this could be caused by a whole host of errors, e.g.
incompatible versions, or a bug at the *other* end of the connection. In all
cases though the action taken by the user/library at this end is different
because the user/library expects the other end to have already shutdown the
connection and so should not attempt to send any sort of error code to the peer.
Regarding exception specifics in the codebase, I agree it would be beneficial
to read the error codes and exception texts from the spec file, however that
would still leave the list and python name for all the exceptions being hand
written for each version of the spec. This makes our API overly brittle when
new error codes are introduced by new versions of the spec, or when vendor
specific error codes are used. At the moment I don't see why anyone would care
to catch the specific subclasses of channel or connection exception since in
each case the channel or connection is closed, and there is no recourse other
than gracefully shutting down and accurately reporting the problem to the user.
This seems to be best acomplished with two exceptions that each contain an
error code and text description to accurately identify what went wrong. This
seems to satisfy the common case, but also permits programatic interception of
errors using the error code if that is needed.
To be clear I'm not completely opposed to adding such a hierarchy if needed,
but I would probably want it added it differently. I believe the core of the
library should simply report the error codes as submitted by the peer since
that permits this library to be used to proxy error codes without understanding
them. I would then add in a specific exception hierarchy as part of an API
layer on top of the core library, much the way the client delegate adds
in-memory message queues on top of the core listener pattern. This keeps the
bulk of the code version and vendor independent, and isolates version/vendor
specific code to its own API layer. That said, I would still not be eager to
add that API layer at this point unless it is definitely needed since it will
almost surely need to be rewritten in a matter of months for 0-10.
> python code.py handling of data types beyond acceptable range.
> --------------------------------------------------------------
>
> Key: QPID-498
> URL: https://issues.apache.org/jira/browse/QPID-498
> Project: Qpid
> Issue Type: Bug
> Components: Python Client
> Reporter: Jimmy John
> Assignee: Rafael H. Schloming
> Priority: Minor
> Attachments: codec.py, constants.py, exception.py
>
>
> Perform range checking on all data types such as octet[0,255],
> short[0,65535], long etc. [Refer QPID-497 for unit test that detects these
> bugs]
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
