Merge lp:~michihenning/unity-scopes-api/what-for-exceptions into lp:unity-scopes-api

[Michi Henning]

Made changes required by new API for exceptions in unity-api: https://code.launchpad.net/~michihenning/unity-api/what-fix/+merge/198871.

Made changes required by new API for exceptions in unity-api: https://code.launchpad.net/~michihenning/unity-api/what-fix/+merge/198871.

Note that merging this branch depends on https://code.launchpad.net/~michihenning/unity-api/what-fix/+merge/198871 being merged first. Because that branch is in a different project, it can't be made a dependent branch.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:90
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-ci/105/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-trusty-amd64-ci/105/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-trusty-armhf-ci/105/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-trusty-i386-ci/105/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-ci/105/rebuild

review: Needs Fixing (continuous-integration)

[Michi Henning (michihenning)]

Need to kick Jenkins once the dependent branch is merged.

[lp:~michihenning/unity-scopes-api/what-for-exceptions]

91. By Michi Henning on 2013-12-17

* Remove factory_configfile() API as well as the need to define it in
  the configuration files.
[ Michal Hruby ]
* Bump version because of the ResultItem rename.
[ Pawel Stolowski ]
* Added API for annotations.
* Added Result::metadata() getter. Create null Variant with default ctor.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:91
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-ci/128/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-trusty-amd64-ci/128
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-trusty-armhf-ci/128
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-trusty-armhf-ci/128/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-trusty-i386-ci/128

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-ci/128/rebuild

review: Approve (continuous-integration)

[James Henstridge (jamesh)]

Looks good. Removing the extra catch clauses is a nice simplification too.

review: Approve

[Michal Hruby (mhr3)]

Please bump library version requirements as well as debian/control when adapting to API changes in underlying libraries next time.

[Michi Henning (michihenning)]

Ah, mea culpa. Thanks for the heads-up! Is it OK to leave as is now, or do I need to push another MR?

[Michal Hruby (mhr3)]

Pushed it myself to https://code.launchpad.net/~mhr3/unity-scopes-api/bump-required-versions/+merge/199247

=== modified file 'include/scopes/ScopeExceptions.h'
--- include/scopes/ScopeExceptions.h	2013-11-21 21:44:00 +0000
+++ include/scopes/ScopeExceptions.h	2013-12-17 01:40:22 +0000
@@ -30,11 +30,6 @@
 namespace scopes
 {
 
-namespace internal
-{
-class MiddlewareExceptionImpl;
-}
-
 /**
 \brief Exception to indicate that something went wrong with the middleware layer.
 */
@@ -54,21 +49,11 @@
     //! @endcond
 
     /**
-    \brief Returns the fully-qualified name of the exception.
-    */
-    virtual char const* what() const noexcept override;
-
-    /**
     \brief Returns a <code>std::exception_ptr</code> to <code>this</code>.
     */
     virtual std::exception_ptr self() const override;
 };
 
-namespace internal
-{
-class ConfigExceptionImpl;
-}
-
 /**
 \brief Exception to indicate that something went wrong with the contents of configuration files.
 */
@@ -88,21 +73,11 @@
     //! @endcond
 
     /**
-    \brief Returns the fully-qualified name of the exception.
-    */
-    virtual char const* what() const noexcept override;
-
-    /**
     \brief Returns a <code>std::exception_ptr</code> to <code>this</code>.
     */
     virtual std::exception_ptr self() const override;
 };
 
-namespace internal
-{
-class NotFoundExceptionImpl;
-}
-
 /**
 \brief Exception to indicate that an object wasn't found by a lookup function.
 */
@@ -123,11 +98,6 @@
     //! @endcond
 
     /**
-    \brief Returns the fully-qualified name of the exception.
-    */
-    virtual char const* what() const noexcept override;
-
-    /**
     \brief Returns a <code>std::exception_ptr</code> to <code>this</code>.
     */
     virtual std::exception_ptr self() const override;
@@ -138,7 +108,7 @@
     virtual std::string name() const;
 
 private:
-    std::shared_ptr<internal::NotFoundExceptionImpl> p;
+    std::string name_;
 };
 
 } // namespace scopes

=== modified file 'src/ScopeExceptions.cpp'
--- src/ScopeExceptions.cpp	2013-11-21 21:44:00 +0000
+++ src/ScopeExceptions.cpp	2013-12-17 01:40:22 +0000
@@ -18,8 +18,6 @@
 
 #include <scopes/ScopeExceptions.h>
 
-#include <unity/ExceptionImplBase.h>
-
 using namespace std;
 
 namespace unity
@@ -32,7 +30,7 @@
 {
 
 MiddlewareException::MiddlewareException(string const& reason) :
-    Exception(make_shared<unity::ExceptionImplBase>(this, reason))
+    Exception("unity::api::scopes::MiddlewareException", reason)
 {
 }
 
@@ -47,18 +45,13 @@
 
 //! @endcond
 
-char const* MiddlewareException::what() const noexcept
-{
-    return "unity::api::scopes::MiddlewareException";
-}
-
 exception_ptr MiddlewareException::self() const
 {
     return make_exception_ptr(*this);
 }
 
 ConfigException::ConfigException(string const& reason) :
-    Exception(make_shared<unity::ExceptionImplBase>(this, reason))
+    Exception("unity::api::scopes::ConfigException", reason)
 {
 }
 
@@ -73,12 +66,6 @@
 
 //! @endcond
 
-char const*
-ConfigException::what() const noexcept
-{
-    return "unity::api::scopes::ConfigException";
-}
-
 exception_ptr
 ConfigException::
 self() const
@@ -86,24 +73,10 @@
     return make_exception_ptr(*this);
 }
 
-namespace internal
-{
-
-class NotFoundExceptionImpl : public unity::ExceptionImplBase
-{
-public:
-    NotFoundExceptionImpl(NotFoundException const* owner, string const& reason, string const& name) :
-        ExceptionImplBase(owner, reason + " (name = " + name + ")"),
-        name_(name)
-    {
-    }
-    string name_;
-};
-
-} // namespace internal
-
 NotFoundException::NotFoundException(string const& reason, string const& name) :
-    Exception(make_shared<internal::NotFoundExceptionImpl>(this, reason, name))
+    Exception("unity::api::scopes::NotFoundException",
+              reason + (reason.empty() ? "" : " ") + "(name = " + name + ")"),
+    name_(name)
 {
 }
 
@@ -118,11 +91,6 @@
 
 //! @endcond
 
-char const* NotFoundException::what() const noexcept
-{
-    return "unity::api::scopes::NotFoundException";
-}
-
 exception_ptr NotFoundException::self() const
 {
     return make_exception_ptr(*this);
@@ -130,7 +98,7 @@
 
 string NotFoundException::name() const
 {
-    return dynamic_cast<const internal::NotFoundExceptionImpl*>(pimpl())->name_;
+    return name_;
 }
 
 } // namespace scopes

=== modified file 'src/internal/QueryObject.cpp'
--- src/internal/QueryObject.cpp	2013-12-11 16:20:02 +0000
+++ src/internal/QueryObject.cpp	2013-12-17 01:40:22 +0000
@@ -91,19 +91,19 @@
     {
         query_base_->run(reply_proxy);
     }
-    catch (unity::Exception const& e)
+    catch (std::exception const& e)
     {
         pushable_ = false;
         reply_->finished(ReceiverBase::Error);     // Oneway, can't block
         // TODO: log error
-        cerr << "An error occurred in ScopeBase::run(): " << e.to_string() << endl;
+        cerr << "ScopeBase::run(): " << e.what() << endl;
     }
     catch (...)
     {
         pushable_ = false;
         reply_->finished(ReceiverBase::Error);     // Oneway, can't block
         // TODO: log error
-        cerr << "An unknown error occurred in ScopeBase::run()" << endl;
+        cerr << "ScopeBase::run(): unknown exception" << endl;
     }
 }
 

=== modified file 'src/internal/ReplyObject.cpp'
--- src/internal/ReplyObject.cpp	2013-12-06 11:22:12 +0000
+++ src/internal/ReplyObject.cpp	2013-12-17 01:40:22 +0000
@@ -25,6 +25,7 @@
 #include <unity/Exception.h>
 
 #include <cassert>
+#include <iostream> // TODO: remove this once logging is added
 
 using namespace std;
 using namespace unity::api::scopes::internal;
@@ -125,16 +126,18 @@
                 CategorisedResult result(*cat_registry_, result_var);
                 receiver_base_->push(std::move(result));
             }
-            catch (unity::Exception const& e)
+            catch (std::exception const& e)
             {
                 // TODO: this is an internal error; log error
+                cerr << "ReplyObject::receiver_base_->push(): " << e.what() << endl;
                 finished(ReceiverBase::Error);
             }
         }
     }
-    catch (unity::Exception const& e)
+    catch (std::exception const& e)
     {
         // TODO: log error
+        cerr << "ReplyObject::push(VariantMap): " << e.what() << endl;
         try
         {
             finished(ReceiverBase::Error);
@@ -146,6 +149,7 @@
     catch (...)
     {
         // TODO: log error
+        cerr << "ReplyObject::push(VariantMap): unknown exception" << endl;
         try
         {
             finished(ReceiverBase::Error);
@@ -186,12 +190,14 @@
     {
         receiver_base_->finished(r);
     }
-    catch (unity::Exception const& e)
+    catch (std::exception const& e)
     {
+        cerr << "ReplyObject::finished(): " << e.what() << endl;
         // TODO: log error
     }
     catch (...)
     {
+        cerr << "ReplyObject::finished(): unknown exception" << endl;
         // TODO: log error
     }
 }

=== modified file 'src/internal/RuntimeImpl.cpp'
--- src/internal/RuntimeImpl.cpp	2013-12-06 11:18:35 +0000
+++ src/internal/RuntimeImpl.cpp	2013-12-17 01:40:22 +0000
@@ -28,6 +28,7 @@
 
 #include <cassert>
 #include <future>
+#include <iostream> // TODO: remove this once logging is added
 
 #include <config.h>
 
@@ -93,12 +94,14 @@
     {
         destroy();
     }
-    catch (unity::Exception const& e) // LCOV_EXCL_LINE
+    catch (std::exception const& e) // LCOV_EXCL_LINE
     {
+        cerr << "~RuntimeImpl(): " << e.what() << endl;
         // TODO: log error
     }
     catch (...) // LCOV_EXCL_LINE
     {
+        cerr << "~RuntimeImpl(): unknown exception" << endl;
         // TODO: log error
     }
 }

=== modified file 'src/internal/ScopeImpl.cpp'
--- src/internal/ScopeImpl.cpp	2013-11-26 04:27:32 +0000
+++ src/internal/ScopeImpl.cpp	2013-12-17 01:40:22 +0000
@@ -25,6 +25,7 @@
 #include <unity/Exception.h>
 
 #include <cassert>
+#include <iostream> // TODO: remove this once logging is added
 
 using namespace std;
 
@@ -70,9 +71,10 @@
         ctrl = fwd()->create_query(q, hints, rp);
         assert(ctrl);
     }
-    catch (unity::Exception const& e)
+    catch (std::exception const& e)
     {
         // TODO: log error
+        cerr << "create_query(): " << e.what() << endl;
         try
         {
             // TODO: if things go wrong, we need to make sure that the reply object
@@ -82,6 +84,7 @@
         }
         catch (...)
         {
+            cerr << "create_query(): unknown exception" << endl;
         }
         throw;
     }

=== modified file 'src/internal/ScopeLoader.cpp'
--- src/internal/ScopeLoader.cpp	2013-11-25 19:05:41 +0000
+++ src/internal/ScopeLoader.cpp	2013-12-17 01:40:22 +0000
@@ -23,6 +23,7 @@
 #include <unity/util/ResourcePtr.h>
 
 #include <cassert>
+#include <iostream> // TODO: remove this once logging is added
 
 using namespace std;
 using namespace unity::api::scopes;
@@ -96,16 +97,14 @@
     {
         unload();
     }
-    catch (unity::Exception const& e)
-    {
-        // TODO: log error
-    }
     catch (std::exception const& e) // LCOV_EXCL_LINE
     {
+        cerr << "~ScopeLoader(): " << e.what() << endl;
         // TODO: log error
     }
     catch (...) // LCOV_EXCL_LINE
     {
+        cerr << "~ScopeLoader(): unknown exception" << endl;
         // TODO: log error
     }
 }

=== modified file 'src/internal/ScopeObject.cpp'
--- src/internal/ScopeObject.cpp	2013-11-25 19:05:41 +0000
+++ src/internal/ScopeObject.cpp	2013-12-17 01:40:22 +0000
@@ -28,6 +28,7 @@
 #include <unity/UnityExceptions.h>
 
 #include <cassert>
+#include <iostream> // TODO: remove this once logging is added
 #include <sstream>
 
 using namespace std;
@@ -118,7 +119,7 @@
 
         query_proxy->run(reply);
     }
-    catch (unity::Exception const& e)
+    catch (std::exception const& e)
     {
         try
         {
@@ -127,6 +128,7 @@
         catch (...)
         {
         }
+        cerr << "create_query(): " << e.what() << endl;
         // TODO: log error
         throw;
     }
@@ -139,6 +141,7 @@
         catch (...)
         {
         }
+        cerr << "create_query(): unknown exception" << endl;
         // TODO: log error
         throw;
     }

=== modified file 'src/internal/zmq_middleware/ObjectAdapter.cpp'
--- src/internal/zmq_middleware/ObjectAdapter.cpp	2013-11-21 21:44:00 +0000
+++ src/internal/zmq_middleware/ObjectAdapter.cpp	2013-12-17 01:40:22 +0000
@@ -380,12 +380,6 @@
             }
         }
     }
-    catch (unity::Exception const& e)
-    {
-        // TODO: log this
-        cerr << "OA broker thread exception: " << e.to_string() << endl;
-        return;
-    }
     catch (std::exception const& e)
     {
         // TODO: log this
@@ -541,12 +535,6 @@
             }
         }
     }
-    catch (unity::Exception const& e)
-    {
-        // TODO: log this
-        cerr << "OA worker thread exception: " << e.to_string() << endl;
-        return;
-    }
     catch (std::exception const& e)
     {
         // TODO: log this

=== modified file 'src/internal/zmq_middleware/ServantBase.cpp'
--- src/internal/zmq_middleware/ServantBase.cpp	2013-11-21 21:44:00 +0000
+++ src/internal/zmq_middleware/ServantBase.cpp	2013-12-17 01:40:22 +0000
@@ -82,10 +82,6 @@
         dispatch_(current, in_params, r);
         return;
     }
-    catch (unity::Exception const& e)
-    {
-        error = e.to_string();
-    }
     catch (std::exception const& e)
     {
         error = e.what();

=== modified file 'test/gtest/scopes/ScopeExceptions/ScopeExceptions_test.cpp'
--- test/gtest/scopes/ScopeExceptions/ScopeExceptions_test.cpp	2013-11-01 12:44:48 +0000
+++ test/gtest/scopes/ScopeExceptions/ScopeExceptions_test.cpp	2013-12-17 01:40:22 +0000
@@ -27,7 +27,7 @@
 {
     {
         MiddlewareException e("some error");
-        EXPECT_STREQ("unity::api::scopes::MiddlewareException", e.what());
+        EXPECT_STREQ("unity::api::scopes::MiddlewareException: some error", e.what());
         EXPECT_THROW(rethrow_exception(e.self()), MiddlewareException);
         MiddlewareException e2("blah");
         e2 = e;
@@ -39,7 +39,7 @@
 {
     {
         ConfigException e("some error");
-        EXPECT_STREQ("unity::api::scopes::ConfigException", e.what());
+        EXPECT_STREQ("unity::api::scopes::ConfigException: some error", e.what());
         EXPECT_THROW(rethrow_exception(e.self()), ConfigException);
         ConfigException e2("blah");
         e2 = e;
@@ -51,7 +51,7 @@
 {
     {
         NotFoundException e("some error", "name");
-        EXPECT_STREQ("unity::api::scopes::NotFoundException", e.what());
+        EXPECT_STREQ("unity::api::scopes::NotFoundException: some error (name = name)", e.what());
         EXPECT_EQ("name", e.name());
         EXPECT_THROW(rethrow_exception(e.self()), NotFoundException);
         NotFoundException e2("blah", "name");