qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rafael Schloming <...@alum.mit.edu>
Subject Re: svn commit: r1627444 - in /qpid/proton/trunk: proton-c/bindings/python/ proton-c/include/proton/ proton-c/src/engine/ proton-c/src/transport/ proton-j/src/main/java/org/apache/qpid/proton/engine/ proton-j/src/main/java/org/apache/qpid/proton/engine/imp...
Date Fri, 26 Sep 2014 10:43:53 GMT
I removed it because a previous commit had changed it to just return NULL.
I figured it would be better to break things than to fail silently. If this
is a problem I can add easily it back and convert what is held in the
condition into the error, but using the condition would be preferrable as
it can contain the full range of AMQP error information whereas the error
will just have a string description.

--Rafael

On Thu, Sep 25, 2014 at 6:16 PM, Chuck Rolke <crolke@redhat.com> wrote:

>
> This commit changes pn_transport_error() into pn_transport_condition() and
> that breaks qpid messaging at
>
> 18>..\..\src\qpid\messaging\amqp\ConnectionContext.cpp(811) : error C3861:
> 'pn_transport_error':
>   identifier not found
>
>
> ----- Original Message -----
> > From: rhs@apache.org
> > To: commits@qpid.apache.org
> > Sent: Wednesday, September 24, 2014 10:08:14 PM
> > Subject: svn commit: r1627444 - in /qpid/proton/trunk:
> proton-c/bindings/python/ proton-c/include/proton/
> > proton-c/src/engine/ proton-c/src/transport/
> proton-j/src/main/java/org/apache/qpid/proton/engine/
> > proton-j/src/main/java/org/apache/qpid/proton/engine/imp...
> >
> > Author: rhs
> > Date: Thu Sep 25 02:08:13 2014
> > New Revision: 1627444
> >
> > URL: http://svn.apache.org/r1627444
> > Log:
> > PROTON-689: added a condition to the transport along with bindings to
> expose
> > it
> >
> > Modified:
> >     qpid/proton/trunk/proton-c/bindings/python/proton.py
> >     qpid/proton/trunk/proton-c/include/proton/transport.h
> >     qpid/proton/trunk/proton-c/src/engine/engine-internal.h
> >     qpid/proton/trunk/proton-c/src/transport/transport.c
> >
>  qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/Transport.java
> >
>  qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java
> >     qpid/proton/trunk/proton-j/src/main/resources/cengine.py
> >     qpid/proton/trunk/tests/python/proton_tests/engine.py
> >
> > Modified: qpid/proton/trunk/proton-c/bindings/python/proton.py
> > URL:
> >
> http://svn.apache.org/viewvc/qpid/proton/trunk/proton-c/bindings/python/proton.py?rev=1627444&r1=1627443&r2=1627444&view=diff
> >
> ==============================================================================
> > --- qpid/proton/trunk/proton-c/bindings/python/proton.py (original)
> > +++ qpid/proton/trunk/proton-c/bindings/python/proton.py Thu Sep 25
> 02:08:13
> > 2014
> > @@ -3136,6 +3136,10 @@ The idle timeout of the connection (floa
> >        self._ssl = SSL(self, domain, session_details)
> >      return self._ssl
> >
> > +  @property
> > +  def condition(self):
> > +    return cond2obj(pn_transport_condition(self._trans))
> > +
> >  class SASLException(TransportException):
> >    pass
> >
> >
> > Modified: qpid/proton/trunk/proton-c/include/proton/transport.h
> > URL:
> >
> http://svn.apache.org/viewvc/qpid/proton/trunk/proton-c/include/proton/transport.h?rev=1627444&r1=1627443&r2=1627444&view=diff
> >
> ==============================================================================
> > --- qpid/proton/trunk/proton-c/include/proton/transport.h (original)
> > +++ qpid/proton/trunk/proton-c/include/proton/transport.h Thu Sep 25
> 02:08:13
> > 2014
> > @@ -24,6 +24,7 @@
> >
> >  #include <proton/import_export.h>
> >  #include <proton/type_compat.h>
> > +#include <proton/condition.h>
> >  #include <stddef.h>
> >  #include <sys/types.h>
> >
> > @@ -103,20 +104,18 @@ PN_EXTERN pn_transport_t *pn_transport(v
> >  PN_EXTERN void pn_transport_free(pn_transport_t *transport);
> >
> >  /**
> > - * Get additional error information associated with the transport.
> > + * Get additional information about the condition of the transport.
> >   *
> > - * Whenever a transport operation fails (i.e. returns an error code),
> > - * additional error details can be obtained using this function. The
> > - * error object that is returned may also be used to clear the error
> > - * condition.
> > + * When a PN_TRANSPORT_ERROR event occurs, this operation can be used
> > + * to access the details of the error condtion.
> >   *
> >   * The pointer returned by this operation is valid until the
> >   * transport object is freed.
> >   *
> >   * @param[in] transport the transport object
> > - * @return the transport's error object
> > + * @return the transport's condition object
> >   */
> > -PN_EXTERN pn_error_t *pn_transport_error(pn_transport_t *transport);
> > +PN_EXTERN pn_condition_t *pn_transport_condition(pn_transport_t
> *transport);
> >
> >  /**
> >   * Binds the transport to an AMQP connection.
> >
> > Modified: qpid/proton/trunk/proton-c/src/engine/engine-internal.h
> > URL:
> >
> http://svn.apache.org/viewvc/qpid/proton/trunk/proton-c/src/engine/engine-internal.h?rev=1627444&r1=1627443&r2=1627444&view=diff
> >
> ==============================================================================
> > --- qpid/proton/trunk/proton-c/src/engine/engine-internal.h (original)
> > +++ qpid/proton/trunk/proton-c/src/engine/engine-internal.h Thu Sep 25
> > 02:08:13 2014
> > @@ -127,6 +127,7 @@ struct pn_transport_t {
> >    uint32_t   local_max_frame;
> >    uint32_t   remote_max_frame;
> >    pn_condition_t remote_condition;
> > +  pn_condition_t condition;
> >
> >  #define PN_IO_SSL  0
> >  #define PN_IO_SASL 1
> >
> > Modified: qpid/proton/trunk/proton-c/src/transport/transport.c
> > URL:
> >
> http://svn.apache.org/viewvc/qpid/proton/trunk/proton-c/src/transport/transport.c?rev=1627444&r1=1627443&r2=1627444&view=diff
> >
> ==============================================================================
> > --- qpid/proton/trunk/proton-c/src/transport/transport.c (original)
> > +++ qpid/proton/trunk/proton-c/src/transport/transport.c Thu Sep 25
> 02:08:13
> > 2014
> > @@ -163,6 +163,7 @@ static void pn_transport_initialize(void
> >    transport->remote_properties = pn_data(0);
> >    transport->disp_data = pn_data(0);
> >    pn_condition_init(&transport->remote_condition);
> > +  pn_condition_init(&transport->condition);
> >
> >    transport->local_channels = pn_hash(PN_OBJECT, 0, 0.75);
> >    transport->remote_channels = pn_hash(PN_OBJECT, 0, 0.75);
> > @@ -256,6 +257,7 @@ static void pn_transport_finalize(void *
> >    pn_free(transport->remote_properties);
> >    pn_free(transport->disp_data);
> >    pn_condition_tini(&transport->remote_condition);
> > +  pn_condition_tini(&transport->condition);
> >    pn_free(transport->local_channels);
> >    pn_free(transport->remote_channels);
> >    if (transport->input_buf) free(transport->input_buf);
> > @@ -350,6 +352,12 @@ pn_error_t *pn_transport_error(pn_transp
> >    return NULL;
> >  }
> >
> > +pn_condition_t *pn_transport_condition(pn_transport_t *transport)
> > +{
> > +  assert(transport);
> > +  return &transport->condition;
> > +}
> > +
> >  static void pni_map_remote_handle(pn_link_t *link, uint32_t handle)
> >  {
> >    link->state.remote_handle = handle;
> > @@ -456,9 +464,11 @@ int pn_do_error(pn_transport_t *transpor
> >      transport->close_sent = true;
> >    }
> >    transport->disp->halt = true;
> > -  pn_transport_logf(transport, "ERROR %s %s", condition, buf);
> > +  pn_condition_set_name(&transport->condition, condition);
> > +  pn_condition_set_description(&transport->condition, buf);
> >    pn_collector_t *collector = pni_transport_collector(transport);
> >    pn_collector_put(collector, PN_OBJECT, transport, PN_TRANSPORT_ERROR);
> > +  pn_transport_logf(transport, "ERROR %s %s", condition, buf);
> >    return PN_ERR;
> >  }
> >
> >
> > Modified:
> >
> qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/Transport.java
> > URL:
> >
> http://svn.apache.org/viewvc/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/Transport.java?rev=1627444&r1=1627443&r2=1627444&view=diff
> >
> ==============================================================================
> > ---
> >
> qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/Transport.java
> > (original)
> > +++
> >
> qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/Transport.java
> > Thu Sep 25 02:08:13 2014
> > @@ -22,6 +22,7 @@ package org.apache.qpid.proton.engine;
> >
> >  import java.nio.ByteBuffer;
> >
> > +import org.apache.qpid.proton.amqp.transport.ErrorCondition;
> >  import org.apache.qpid.proton.engine.impl.TransportImpl;
> >
> >
> > @@ -221,4 +222,6 @@ public interface Transport extends Endpo
> >
> >      int getRemoteChannelMax();
> >
> > +    ErrorCondition getCondition();
> > +
> >  }
> >
> > Modified:
> >
> qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java
> > URL:
> >
> http://svn.apache.org/viewvc/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java?rev=1627444&r1=1627443&r2=1627444&view=diff
> >
> ==============================================================================
> > ---
> >
> qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java
> > (original)
> > +++
> >
> qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java
> > Thu Sep 25 02:08:13 2014
> > @@ -121,7 +121,7 @@ public class TransportImpl extends Endpo
> >
> >      private FrameHandler _frameHandler = this;
> >      private boolean _head_closed = false;
> > -    private TransportException _tail_error = null;
> > +    private ErrorCondition _condition = null;
> >
> >      private boolean postedHeadClosed = false;
> >      private boolean postedTailClosed = false;
> > @@ -211,6 +211,12 @@ public class TransportImpl extends Endpo
> >      }
> >
> >      @Override
> > +    public ErrorCondition getCondition()
> > +    {
> > +        return _condition;
> > +    }
> > +
> > +    @Override
> >      public void bind(Connection conn)
> >      {
> >          // TODO - check if already bound
> > @@ -752,7 +758,7 @@ public class TransportImpl extends Endpo
> >
> >      private void processOpen()
> >      {
> > -        if ((_tail_error != null ||
> > +        if ((_condition != null ||
> >               (_connectionEndpoint != null &&
> >                _connectionEndpoint.getLocalState() !=
> >                EndpointState.UNINITIALIZED)) &&
> >              !_isOpenSent) {
> > @@ -929,7 +935,7 @@ public class TransportImpl extends Endpo
> >
> >      private void processClose()
> >      {
> > -        if ((_tail_error != null ||
> > +        if ((_condition != null ||
> >               (_connectionEndpoint != null &&
> >                _connectionEndpoint.getLocalState() ==
> EndpointState.CLOSED))
> >                &&
> >              !_isCloseSent) {
> > @@ -940,8 +946,7 @@ public class TransportImpl extends Endpo
> >                  ErrorCondition localError;
> >
> >                  if (_connectionEndpoint == null) {
> > -                    localError = new
> > ErrorCondition(ConnectionError.FRAMING_ERROR,
> > -
> _tail_error.toString());
> > +                    localError = _condition;
> >                  } else {
> >                      localError =  _connectionEndpoint.getCondition();
> >                  }
> > @@ -1263,13 +1268,15 @@ public class TransportImpl extends Endpo
> >      {
> >          if (!_closeReceived || error != null) {
> >              if (error == null) {
> > -                _tail_error = new TransportException("connection
> aborted");
> > +                _condition = new
> > ErrorCondition(ConnectionError.FRAMING_ERROR,
> > +                                               "connection aborted");
> >              } else {
> > -                _tail_error = error;
> > +                _condition = new
> > ErrorCondition(ConnectionError.FRAMING_ERROR,
> > +                                                error.toString());
> >              }
> >              _head_closed = true;
> >          }
> > -        if (_tail_error != null) {
> > +        if (_condition != null) {
> >              put(Event.Type.TRANSPORT_ERROR, this);
> >          }
> >          if (!postedTailClosed) {
> >
> > Modified: qpid/proton/trunk/proton-j/src/main/resources/cengine.py
> > URL:
> >
> http://svn.apache.org/viewvc/qpid/proton/trunk/proton-j/src/main/resources/cengine.py?rev=1627444&r1=1627443&r2=1627444&view=diff
> >
> ==============================================================================
> > --- qpid/proton/trunk/proton-j/src/main/resources/cengine.py (original)
> > +++ qpid/proton/trunk/proton-j/src/main/resources/cengine.py Thu Sep 25
> > 02:08:13 2014
> > @@ -867,6 +867,7 @@ class pn_transport_wrapper:
> >
> >    def __init__(self, impl):
> >      self.impl = impl
> > +    self.condition = pn_condition()
> >
> >  def pn_transport():
> >    return wrap(Proton.transport(), pn_transport_wrapper)
> > @@ -944,6 +945,10 @@ def pn_transport_close_tail(trans):
> >  def pn_transport_closed(trans):
> >    return trans.impl.isClosed()
> >
> > +def pn_transport_condition(trans):
> > +  trans.condition.decode(trans.impl.getCondition())
> > +  return trans.condition
> > +
> >  from org.apache.qpid.proton.engine import Event
> >
> >  PN_CONNECTION_INIT = Event.Type.CONNECTION_INIT
> >
> > Modified: qpid/proton/trunk/tests/python/proton_tests/engine.py
> > URL:
> >
> http://svn.apache.org/viewvc/qpid/proton/trunk/tests/python/proton_tests/engine.py?rev=1627444&r1=1627443&r2=1627444&view=diff
> >
> ==============================================================================
> > --- qpid/proton/trunk/tests/python/proton_tests/engine.py (original)
> > +++ qpid/proton/trunk/tests/python/proton_tests/engine.py Thu Sep 25
> 02:08:13
> > 2014
> > @@ -2257,12 +2257,14 @@ class EventTest(CollectorTest):
> >      t = Transport()
> >      t.bind(c)
> >      self.expect(Event.CONNECTION_BOUND)
> > +    assert t.condition is None
> >      t.push("asdf")
> >      self.expect(Event.TRANSPORT_ERROR, Event.TRANSPORT_TAIL_CLOSED)
> > +    assert t.condition is not None
> > +    assert t.condition.name == "amqp:connection:framing-error"
> > +    assert "AMQP header mismatch" in t.condition.description
> >      p = t.pending()
> >      assert p > 0
> > -    # XXX: can't include this because java behaviour is different
> > -    #assert "AMQP header mismatch" in t.peek(p), repr(t.peek(p))
> >      t.pop(p)
> >      self.expect(Event.TRANSPORT_HEAD_CLOSED, Event.TRANSPORT_CLOSED)
> >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
> > For additional commands, e-mail: commits-help@qpid.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
> For additional commands, e-mail: dev-help@qpid.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message