Improved tracing of failures, user exceptions are now tracked

with a userException member.

9 files changed, 221 insertions, 99 deletions

diff --git a/cpp/src/Ice/Incoming.cpp b/cpp/src/Ice/Incoming.cpp
index f31d3b19e88..9c0fc36f509 100644
--- a/cpp/src/Ice/Incoming.cpp
+++ b/cpp/src/Ice/Incoming.cpp
@@ -107,6 +107,11 @@ IncomingBase::__startWriteParams(FormatType format)
 void 
 IncomingBase::__endWriteParams(bool ok)
 {
+    if(!ok)
+    {
+        _observer.userException();
+    }
+
     if(_response)
     {
         *(_os.b.begin() + headerSize + 4) = ok ? replyOK : replyUserException; // Reply status position.
@@ -129,6 +134,11 @@ IncomingBase::__writeEmptyParams()
 void 
 IncomingBase::__writeParamEncaps(const Byte* v, Ice::Int sz, bool ok)
 {
+    if(!ok)
+    {
+        _observer.userException();
+    }
+
     if(_response)
     {
         assert(_os.b.size() == headerSize + 4); // Reply status position.
@@ -148,11 +158,6 @@ IncomingBase::__writeParamEncaps(const Byte* v, Ice::Int sz, bool ok)
 void 
 IncomingBase::__writeUserException(const Ice::UserException& ex, Ice::FormatType format)
 {
-    if(_observer)
-    {
-        _observer.failed(ex.ice_name());
-    }
-
     ::IceInternal::BasicStream* __os = __startWriteParams(format);
     __os->write(ex);
     __endWriteParams(false);
@@ -215,7 +220,7 @@ IceInternal::IncomingBase::__servantLocatorFinished()
 
         if(_observer)
         {
-            _observer.failed(ex.ice_name());
+            _observer.userException();
         }
 
         //
@@ -613,7 +618,7 @@ IceInternal::Incoming::invoke(const ServantManagerPtr& servantManager, BasicStre
 
                     if(_observer)
                     {
-                        _observer.failed(ex.ice_name());
+                        _observer.userException();
                     }
 
                     if(_response)
diff --git a/cpp/src/Ice/InstrumentationI.cpp b/cpp/src/Ice/InstrumentationI.cpp
index e07d378d7cc..3776c447d28 100644
--- a/cpp/src/Ice/InstrumentationI.cpp
+++ b/cpp/src/Ice/InstrumentationI.cpp
@@ -222,7 +222,7 @@ private:
 
 ConnectionHelper::Attributes ConnectionHelper::attributes;
 
-class DispatchHelper : public MetricsHelperT<Metrics>
+class DispatchHelper : public MetricsHelperT<DispatchMetrics>
 {
 public:
 
@@ -721,11 +721,23 @@ ThreadObserverI::stateChanged(ThreadState oldState, ThreadState newState)
 }
 
 void
+DispatchObserverI::userException()
+{
+    forEach(inc(&DispatchMetrics::userException));
+}
+
+void
 InvocationObserverI::retried()
 {
     forEach(inc(&InvocationMetrics::retry));
 }
 
+void
+InvocationObserverI::userException()
+{
+    forEach(inc(&InvocationMetrics::userException));
+}
+
 ObserverPtr
 InvocationObserverI::getRemoteObserver(const ConnectionInfoPtr& connection, const EndpointPtr& endpoint)
 {
@@ -854,7 +866,7 @@ CommunicatorObserverI::getInvocationObserver(const ObjectPrx& proxy, const strin
     return 0;
 }
 
-ObserverPtr 
+DispatchObserverPtr 
 CommunicatorObserverI::getDispatchObserver(const Current& current)
 {
     if(_dispatch.isEnabled())
diff --git a/cpp/src/Ice/InstrumentationI.h b/cpp/src/Ice/InstrumentationI.h
index 9f363016a00..87d238cfc4b 100644
--- a/cpp/src/Ice/InstrumentationI.h
+++ b/cpp/src/Ice/InstrumentationI.h
@@ -30,12 +30,21 @@ public:
     virtual void stateChanged(Ice::Instrumentation::ThreadState, Ice::Instrumentation::ThreadState);
 };
 
+class DispatchObserverI : public Ice::Instrumentation::DispatchObserver, public ObserverT<DispatchMetrics>
+{
+public:
+
+    virtual void userException();
+};
+
 class InvocationObserverI : public Ice::Instrumentation::InvocationObserver, public ObserverT<InvocationMetrics>
 {
 public:
 
     virtual void retried();
 
+    virtual void userException();
+
     virtual Ice::Instrumentation::ObserverPtr getRemoteObserver(const Ice::ConnectionInfoPtr&, const Ice::EndpointPtr&);
 };
 
@@ -66,7 +75,7 @@ public:
                                                                               const std::string&,
                                                                               const Ice::Context&);
 
-    virtual Ice::Instrumentation::ObserverPtr getDispatchObserver(const Ice::Current&);
+    virtual Ice::Instrumentation::DispatchObserverPtr getDispatchObserver(const Ice::Current&);
 
     const IceInternal::MetricsAdminIPtr& getMetricsAdmin() const;
 
@@ -75,7 +84,7 @@ private:
     const IceInternal::MetricsAdminIPtr _metrics;
 
     ObserverFactoryT<ConnectionObserverI> _connections;
-    ObserverFactoryT<ObserverI> _dispatch;
+    ObserverFactoryT<DispatchObserverI> _dispatch;
     ObserverFactoryT<InvocationObserverI> _invocations;
     ObserverFactoryT<ThreadObserverI> _threads;
     ObserverFactoryT<ObserverI> _connects;
diff --git a/cpp/src/Ice/Outgoing.cpp b/cpp/src/Ice/Outgoing.cpp
index 53e50da60d1..e02e28063a4 100644
--- a/cpp/src/Ice/Outgoing.cpp
+++ b/cpp/src/Ice/Outgoing.cpp
@@ -387,6 +387,7 @@ IceInternal::Outgoing::finished(BasicStream& is)
         
         case replyUserException:
         {
+            _observer.userException();
             _state = StateUserException; // The state must be set last, in case there is an exception.
             break;
         }
@@ -523,6 +524,7 @@ IceInternal::Outgoing::finished(const LocalException& ex, bool sent)
 {
     IceUtil::Monitor<IceUtil::Mutex>::Lock sync(_monitor);
     assert(_state <= StateInProgress);
+    _remoteObserver.failed(ex.ice_name());
     _remoteObserver.detach();
 
     _state = StateFailed;
@@ -539,12 +541,8 @@ IceInternal::Outgoing::throwUserException()
         _is.startReadEncaps();
         _is.throwException();
     }
-    catch(const Ice::UserException& ex)
+    catch(const Ice::UserException&)
     {
-        if(_observer)
-        {
-            _observer.failed(ex.ice_name());
-        }
         _is.endReadEncaps();
         throw;
     }
@@ -579,7 +577,6 @@ IceInternal::BatchOutgoing::invoke()
         {
             _monitor.wait();
         }
-        _remoteObserver.detach();
         if(_exception.get())
         {
             _exception->ice_throw();
@@ -600,12 +597,15 @@ IceInternal::BatchOutgoing::sent(bool notify)
     {
         _sent = true;
     }
+    _remoteObserver.detach();
 }
 
 void
 IceInternal::BatchOutgoing::finished(const Ice::LocalException& ex, bool)
 {
     IceUtil::Monitor<IceUtil::Mutex>::Lock sync(_monitor);
+    _remoteObserver.failed(ex.ice_name());
+    _remoteObserver.detach();
     _exception.reset(ex.ice_clone());
     _monitor.notify();
 }
diff --git a/cpp/src/Ice/OutgoingAsync.cpp b/cpp/src/Ice/OutgoingAsync.cpp
index a1f5c9ef77e..efbc752c897 100644
--- a/cpp/src/Ice/OutgoingAsync.cpp
+++ b/cpp/src/Ice/OutgoingAsync.cpp
@@ -241,6 +241,15 @@ Ice::AsyncResult::__sent()
             __warning();
         }
     }
+
+    if(_observer)
+    {
+        Ice::ObjectPrx proxy = getProxy();
+        if(!proxy || !proxy->ice_isTwoway())
+        {
+            _observer.detach();
+        }
+    }
 }
 
 void
@@ -527,10 +536,6 @@ void
 IceInternal::OutgoingAsync::__sent()
 {
     ::Ice::AsyncResult::__sent();
-    if(!_proxy->ice_isTwoway())
-    {
-        _observer.detach();
-    }
 }
 
 void
@@ -539,6 +544,7 @@ IceInternal::OutgoingAsync::__finished(const Ice::LocalException& exc, bool sent
     {
         IceUtil::Monitor<IceUtil::Mutex>::Lock sync(_monitor);
         assert(!(_state & Done));
+        _remoteObserver.failed(exc.ice_name());
         _remoteObserver.detach();
         if(_timerTaskConnection)
         {
@@ -578,6 +584,7 @@ IceInternal::OutgoingAsync::__finished(const LocalExceptionWrapper& exc)
     // calling on the callback. The LocalExceptionWrapper exception is only called
     // before the invocation is sent.
     //
+    _remoteObserver.failed(exc.get()->ice_name());
     _remoteObserver.detach();
 
     try
@@ -622,8 +629,12 @@ IceInternal::OutgoingAsync::__finished(BasicStream& is)
         switch(replyStatus)
         {
             case replyOK:
+            {
+                break;
+            }
             case replyUserException:
             {
+                _observer.userException();
                 break;
             }
 
@@ -889,15 +900,12 @@ IceInternal::BatchOutgoingAsync::__sent(Ice::ConnectionI* connection)
     _state |= Done | OK | Sent;
     _remoteObserver.detach();
     _monitor.notifyAll();
-    if(_callback && _callback->__hasSentCallback())
-    {
-        return true;
-    }
-    else
+    if(!_callback || !_callback->__hasSentCallback())
     {
         _observer.detach();
         return false;
     }
+    return true;
 }
 
 void
@@ -909,6 +917,7 @@ IceInternal::BatchOutgoingAsync::__sent()
 void
 IceInternal::BatchOutgoingAsync::__finished(const Ice::LocalException& exc, bool)
 {
+    _remoteObserver.failed(exc.ice_name());
     _remoteObserver.detach();
     __exception(exc);
 }
@@ -1030,8 +1039,9 @@ IceInternal::CommunicatorBatchOutgoingAsync::flushConnection(const ConnectionIPt
             return false;
         }
 
-        virtual void __finished(const Ice::LocalException&, bool)
+        virtual void __finished(const Ice::LocalException& ex, bool)
         {
+            _remoteObserver.failed(ex.ice_name());
             _remoteObserver.detach();
             _outAsync->check(false);
         }
@@ -1074,24 +1084,29 @@ IceInternal::CommunicatorBatchOutgoingAsync::check(bool userThread)
         if(--_useCount > 0)
         {
             return;
-        }
-         
-        _observer.detach();
+        }         
         _state |= Done | OK | Sent;
         _monitor.notifyAll();
     }
 
-    //
-    // _sentSynchronously is immutable here.
-    //
-    if(!_sentSynchronously && userThread)
+    if(!_callback || !_callback->__hasSentCallback())
     {
-        __sentAsync();
+        _observer.detach();
     }
     else
     {
-        assert(_sentSynchronously == userThread); // sentSynchronously && !userThread is impossible.
-        AsyncResult::__sent();
+        //
+        // _sentSynchronously is immutable here.
+        //
+        if(!_sentSynchronously && userThread)
+        {
+            __sentAsync();
+        }
+        else
+        {
+            assert(_sentSynchronously == userThread); // sentSynchronously && !userThread is impossible.
+            AsyncResult::__sent();
+        }
     }
 }
 
diff --git a/cpp/src/Ice/Proxy.cpp b/cpp/src/Ice/Proxy.cpp
index 59081974ffa..e2196926685 100644
--- a/cpp/src/Ice/Proxy.cpp
+++ b/cpp/src/Ice/Proxy.cpp
@@ -184,22 +184,31 @@ bool
 IceProxy::Ice::Object::end_ice_isA(const AsyncResultPtr& __result)
 {
     AsyncResult::__check(__result, this, ice_isA_name);
-    if(!__result->__wait())
+    bool __ok = __result->__wait();
+    try
     {
-        try
-        {
-            __result->__throwUserException();
-        }
-        catch(const UserException& __ex)
+        if(!__ok)
         {
-            throw UnknownUserException(__FILE__, __LINE__, __ex.ice_name());
+            try
+            {
+                __result->__throwUserException();
+            }
+            catch(const UserException& __ex)
+            {
+                throw UnknownUserException(__FILE__, __LINE__, __ex.ice_name());
+            }
         }
+        bool __ret;
+        IceInternal::BasicStream* __is = __result->__startReadParams();
+        __is->read(__ret);
+        __result->__endReadParams();
+        return __ret;
+    }
+    catch(const ::Ice::LocalException& ex)
+    {
+        __result->__getObserver().failed(ex.ice_name());
+        throw;
     }
-    bool __ret;
-    IceInternal::BasicStream* __is = __result->__startReadParams();
-    __is->read(__ret);
-    __result->__endReadParams();
-    return __ret;
 }
 
 void
@@ -326,22 +335,31 @@ vector<string>
 IceProxy::Ice::Object::end_ice_ids(const AsyncResultPtr& __result)
 {
     AsyncResult::__check(__result, this, ice_ids_name);
-    if(!__result->__wait())
+    bool __ok = __result->__wait();
+    try
     {
-        try
-        {
-            __result->__throwUserException();
-        }
-        catch(const UserException& __ex)
+        if(!__ok)
         {
-            throw UnknownUserException(__FILE__, __LINE__, __ex.ice_name());
+            try
+            {
+                __result->__throwUserException();
+            }
+            catch(const UserException& __ex)
+            {
+                throw UnknownUserException(__FILE__, __LINE__, __ex.ice_name());
+            }
         }
+        vector<string> __ret;
+        IceInternal::BasicStream* __is = __result->__startReadParams();
+        __is->read(__ret);
+        __result->__endReadParams();
+        return __ret;
+    }
+    catch(const ::Ice::LocalException& ex)
+    {
+        __result->__getObserver().failed(ex.ice_name());
+        throw;
     }
-    vector<string> __ret;
-    IceInternal::BasicStream* __is = __result->__startReadParams();
-    __is->read(__ret);
-    __result->__endReadParams();
-    return __ret;
 }
 
 AsyncResultPtr
@@ -368,22 +386,32 @@ string
 IceProxy::Ice::Object::end_ice_id(const AsyncResultPtr& __result)
 {
     AsyncResult::__check(__result, this, ice_id_name);
-    if(!__result->__wait())
+    bool __ok = __result->__wait();
+    try
     {
-        try
-        {
-            __result->__throwUserException();
-        }
-        catch(const UserException& __ex)
+        if(!__ok)
         {
-            throw UnknownUserException(__FILE__, __LINE__, __ex.ice_name());
+            try
+            {
+                __result->__throwUserException();
+            }
+            catch(const UserException& __ex)
+            {
+                throw UnknownUserException(__FILE__, __LINE__, __ex.ice_name());
+            }
         }
+        string __ret;
+        IceInternal::BasicStream* __is = __result->__startReadParams();
+        __is->read(__ret);
+        __result->__endReadParams();
+        return __ret;
+    
+    }
+    catch(const ::Ice::LocalException& ex)
+    {
+        __result->__getObserver().failed(ex.ice_name());
+        throw;
     }
-    string __ret;
-    IceInternal::BasicStream* __is = __result->__startReadParams();
-    __is->read(__ret);
-    __result->__endReadParams();
-    return __ret;
 }
 
 bool
@@ -484,10 +512,18 @@ IceProxy::Ice::Object::end_ice_invoke(vector<Byte>& outEncaps, const AsyncResult
     bool ok = __result->__wait();
     if(_reference->getMode() == Reference::ModeTwoway)
     {
-        const Byte* v;
-        Int sz;
-        __result->__readParamEncaps(v, sz);
-        vector<Byte>(v, v + sz).swap(outEncaps);
+        try
+        {
+            const Byte* v;
+            Int sz;
+            __result->__readParamEncaps(v, sz);
+            vector<Byte>(v, v + sz).swap(outEncaps);
+        }
+        catch(const ::Ice::LocalException& ex)
+        {
+            __result->__getObserver().failed(ex.ice_name());
+            throw;
+        }
     }
     return ok;
 }
@@ -606,9 +642,17 @@ IceProxy::Ice::Object::___end_ice_invoke(pair<const Byte*, const Byte*>& outEnca
     bool ok = __result->__wait();
     if(_reference->getMode() == Reference::ModeTwoway)
     {
-        Int sz;
-        __result->__readParamEncaps(outEncaps.first, sz);
-        outEncaps.second = outEncaps.first + sz;
+        try
+        {
+            Int sz;
+            __result->__readParamEncaps(outEncaps.first, sz);
+            outEncaps.second = outEncaps.first + sz;
+        }
+        catch(const ::Ice::LocalException& ex)
+        {
+            __result->__getObserver().failed(ex.ice_name());
+            throw;
+        }
     }
     return ok;
 }
@@ -1300,10 +1344,6 @@ IceProxy::Ice::Object::__handleExceptionWrapper(const ::IceInternal::Handle< ::I
         observer.failed(ex.get()->ice_name());
         ex.get()->ice_throw();
     }
-    else
-    {
-        observer.retried();
-    }
 
     return 0;
 }
@@ -1328,7 +1368,6 @@ IceProxy::Ice::Object::__handleExceptionWrapperRelaxed(const ::IceInternal::Hand
                 _delegate = 0;
             }
         }
-        observer.retried();
         return 0;
     }
 }
@@ -1365,21 +1404,29 @@ void
 IceProxy::Ice::Object::__end(const ::Ice::AsyncResultPtr& __result, const std::string& operation) const
 {
     AsyncResult::__check(__result, this, operation);
-    bool ok = __result->__wait();
+    bool __ok = __result->__wait();
     if(_reference->getMode() == Reference::ModeTwoway)
     {
-        if(!ok)
+        try
         {
-            try
-            {
-                __result->__throwUserException();
-            }
-            catch(const UserException& __ex)
+            if(!__ok)
             {
-                throw UnknownUserException(__FILE__, __LINE__, __ex.ice_name());
+                try
+                {
+                    __result->__throwUserException();
+                }
+                catch(const UserException& __ex)
+                {
+                    throw UnknownUserException(__FILE__, __LINE__, __ex.ice_name());
+                }
             }
+            __result->__readEmptyParams();
+        }
+        catch(const ::Ice::LocalException& ex)
+        {
+            __result->__getObserver().failed(ex.ice_name());
+            throw;
         }
-        __result->__readEmptyParams();
     }
 }
 
diff --git a/cpp/src/slice2cpp/Gen.cpp b/cpp/src/slice2cpp/Gen.cpp
index 72ac920e597..6738a68f10c 100644
--- a/cpp/src/slice2cpp/Gen.cpp
+++ b/cpp/src/slice2cpp/Gen.cpp
@@ -2263,8 +2263,10 @@ Slice::Gen::ProxyVisitor::visitOperation(const OperationPtr& p)
         // and Windows 64 bits when compiled with optimization (see bug 4400).
         //
         writeAllocateCode(C, ParamDeclList(), p, _useWstring | TypeContextAMIEnd);
-
-        C << nl << "if(!__result->__wait())";
+        C << nl << "bool __ok = __result->__wait();";
+        C << nl << "try";
+        C << sb;
+        C << nl << "if(!__ok)";
         C << sb;
         C << nl << "try";
         C << sb;
@@ -2309,6 +2311,12 @@ Slice::Gen::ProxyVisitor::visitOperation(const OperationPtr& p)
         {
             C << nl << "return __ret;";
         }
+        C << eb;
+        C << nl << "catch(const ::Ice::LocalException& ex)";
+        C << sb;
+        C << nl << "__result->__getObserver().failed(ex.ice_name());";
+        C << nl << "throw;";
+        C << eb;
     }
     else
     {
diff --git a/cpp/src/slice2cs/Gen.cpp b/cpp/src/slice2cs/Gen.cpp
index 623576819ce..121976a7433 100644
--- a/cpp/src/slice2cs/Gen.cpp
+++ b/cpp/src/slice2cs/Gen.cpp
@@ -4973,7 +4973,10 @@ Slice::Gen::HelperVisitor::visitClassDefStart(const ClassDefPtr& p)
         {
             _out << nl << "IceInternal.OutgoingAsync outAsync__ = (IceInternal.OutgoingAsync)r__;";
             _out << nl << "IceInternal.OutgoingAsync.check__(outAsync__, this, " << flatName << ");";
-            _out << nl << "if(!outAsync__.wait__())";
+            _out << nl << "bool ok__ = outAsync__.wait__();";
+            _out << nl << "try";
+            _out << sb;
+            _out << nl << "if(!ok__)";
             _out << sb;
 
             ExceptionList throws = op->throws();
@@ -5071,6 +5074,16 @@ Slice::Gen::HelperVisitor::visitClassDefStart(const ClassDefPtr& p)
             {
                 _out << nl << "outAsync__.readEmptyParams__();";
             }
+            _out << eb;
+            _out << nl << "catch(Ice.LocalException ex)";
+            _out << sb;
+            _out << nl << "Ice.Instrumentation.InvocationObserver obsv__ = outAsync__.getObserver__();";
+            _out << nl << "if(obsv__ != null)";
+            _out << sb;
+            _out << nl << "obsv__.failed(ex.ice_name());";
+            _out << eb;
+            _out << nl << "throw ex;";
+            _out << eb;
         }
         else
         {
diff --git a/cpp/src/slice2java/Gen.cpp b/cpp/src/slice2java/Gen.cpp
index 666e23f1104..c9f7f42e243 100644
--- a/cpp/src/slice2java/Gen.cpp
+++ b/cpp/src/slice2java/Gen.cpp
@@ -4524,8 +4524,10 @@ Slice::Gen::HelperVisitor::visitClassDefStart(const ClassDefPtr& p)
             if(op->returnsData())
             {
                 out << nl << "Ice.AsyncResult.__check(__result, this, __" << op->name() << "_name);";
-                out << nl;
-                out << "if(!__result.__wait())";
+                out << nl << "boolean __ok = __result.__wait();";
+                out << nl << "try";
+                out << sb;
+                out << nl << "if(!__ok)";
                 out << sb;
                 out << nl << "try";
                 out << sb;
@@ -4587,6 +4589,17 @@ Slice::Gen::HelperVisitor::visitClassDefStart(const ClassDefPtr& p)
                         out << nl << "return __ret;";
                     }
                 }
+                
+                out << eb;
+                out << nl << "catch(Ice.LocalException ex)";
+                out << sb;
+                out << nl << "Ice.Instrumentation.InvocationObserver __obsv = __result.__getObserver();";
+                out << nl << "if(__obsv != null)";
+                out << sb;
+                out << nl << "__obsv.failed(ex.ice_name());";
+                out << eb;
+                out << nl << "throw ex;";
+                out << eb;
             }
             else
             {