fixed a couple of todos, make sure that onError() is always called

include/amqpcpp/connection.h

@@ -161,7 +161,7 @@ public:
      *  called.
      * 
      *  @param  message     the message that has to be passed to all error handlers
-     *  @return bool
+     *  @return bool        false if the connection already was failed
      */
     bool fail(const char *message)
     {

include/amqpcpp/libev.h

@@ -233,7 +233,6 @@ private:
             if (now >= _expire)
             {
                 // close the connection with immediate effect (this will destruct the connection)
-                // @todo do we want to report an error too?
                 _connection->close(true);
             }
             else
@@ -281,6 +280,9 @@ private:
             // initialize the libev structure
             ev_timer_init(&_timer, callback, timeout, timeout);
 
+            // start the timer (this is the time that we reserve for setting up the connection)
+            ev_timer_start(_loop, &_timer);
+
             // the timer should not keep the event loop active
             ev_unref(_loop);
         }
@@ -311,10 +313,7 @@ private:
          */
         uint16_t start()
         {
-            // start the timer
+            // expose the interval (the timer is already running, so we do not have to explicitly start it)
-            ev_timer_start(_loop, &_timer);
-            
-            // expose the interval
             return _interval;
         }
         

include/amqpcpp/linux_tcp/tcpconnection.h

@@ -230,7 +230,8 @@ public:
      *  TCP connection. Note that the connection is not immediately closed: first all
      *  pending operations are completed, and then an AMQP closing-handshake is
      *  performed. If you pass a parameter "immediate=true" the connection is 
-     *  immediately closed, without waiting for earlier commands
+     *  immediately closed, without waiting for earlier commands (and your handler's
+     *  onError() method is called about the premature close)
      *  @return bool
      */
     bool close(bool immediate = false);

src/connectionimpl.cpp

@@ -197,7 +197,7 @@ bool ConnectionImpl::fail(const Monitor &monitor, const char *message)
         // leap out if no longer valid
         if (!monitor.valid()) return false;
     }
-    
+
     // done
     return true;
 }

src/linux_tcp/poll.h

@@ -29,7 +29,7 @@ private:
     fd_set _set;
 
     /**
-     *  The current socket // @todo what is it exactly?
+     *  The socket filedescriptor
      *  @var int
      */
     int _socket;

src/linux_tcp/sslhandshake.h

@@ -105,9 +105,6 @@ private:
 public:
     /**
      *  Constructor
-     * 
-     *  @todo catch the exception!  
-     * 
      *  @param  state       Earlier state
      *  @param  hostname    The hostname to connect to
      *  @param  context     SSL context

src/linux_tcp/tcpconnection.cpp

@@ -70,6 +70,9 @@ void TcpConnection::process(int fd, int flags)
 {
     // monitor the object for destruction, because you never know what the user
     Monitor monitor(this);
+
+    // remember the old state
+    auto *oldstate = _state.get();
         
     // pass on the the state, that returns a new impl
     auto *newstate = _state->process(monitor, fd, flags);
@@ -77,8 +80,7 @@ void TcpConnection::process(int fd, int flags)
     // if the state did not change, we do not have to update a member,
     // when the newstate is nullptr, the object is (being) destructed
     // and we do not have to do anything else either
-    // @todo what if close(true) is called that install the new TcpClosed() ????
+    if (newstate == nullptr || newstate == oldstate) return;
-    if (newstate == nullptr || newstate == _state.get()) return;
 
     // wrap the new state in a unique-ptr so that so that the old state
     // is not destructed before the new one is assigned
@@ -97,17 +99,21 @@ void TcpConnection::process(int fd, int flags)
  */
 bool TcpConnection::close(bool immediate)
 {
-    // @todo what if not yet connected / still doing a lookup / already disconnected?
-    
     // if no immediate disconnect is needed, we can simply start the closing handshake
     if (!immediate) return _connection.close();
-    
+
     // failing the connection could destruct "this"
     Monitor monitor(this);
     
     // fail the connection / report the error to user-space
-    _connection.fail("connection prematurely closed by client");
+    bool failed = _connection.fail("connection prematurely closed by client");
     
+    // stop if object was destructed
+    if (!monitor.valid()) return true;
+
+    // tell the handler that the connection was closed
+    if (failed) _handler->onError(this, "connection prematurely closed by client");
+
     // stop if object was destructed
     if (!monitor.valid()) return true;
     
@@ -203,20 +209,17 @@ void TcpConnection::onError(TcpState *state, const char *message, bool connected
     Monitor monitor(this);
     
     // if there are still pending operations, they should be reported as error
-    _connection.fail(message);
+    bool failed = _connection.fail(message);
     
     // stop if object was destructed
     if (!monitor.valid()) return;
 
+    // tell the handler
+    if (failed) _handler->onError(this, message);
+
     // if the object is still connected, we only have to report the error and
     // we wait for the subsequent call to the onLost() method
-    if (connected) return _handler->onError(this, message);
+    if (connected || !monitor.valid()) return;
-
-    // tell the handler
-    _handler->onError(this, message);
-    
-    // leap out if object was destructed
-    if (!monitor.valid()) return;
     
     // tell the handler that no further events will be fired
     _handler->onDetached(this);