Merge lp:~michihenning/unity-scopes-api/push-errors into lp:unity-scopes-api

[Michi Henning]

Added ability for scope to push an exception (as an exception_ptr). On the client side,
the exception is delivered as the what() string (if the exception is a std::exception) and
as "unknown exception", otherwise.

Added ability for scope to push an exception (as an exception_ptr). On the client side,
the exception is delivered as the what() string (if the exception is a std::exception) and
as "unknown exception", otherwise.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Michal Hruby (mhr3)]

Hm, thinking more about this, there is pretty much nothing a client can do with the message in the finished() method, so I'm wondering whether it should really be there.

A much nicer API would be if create_query() returned a future and you could get() the operation result from there - and it could even throw an error at that point.

[Michal Hruby (mhr3)]

Of course that doesn't change the fact the error should be tranferred on the protocol level.

[Michi Henning (michihenning)]

I could remove the message in the finished() method on the server side, but I think James wanted to have this for the Go binding (which is why I added it in the first place).

I agree, the client can't do all that much with it. But, in James's case, he needs the error for testing, to see whether Go errors are correctly propagated across to C++. (James, correct me please if I got that wrong.)

Michal, if you recall, when you first pushed for this, it was *me* who argued that an error flag is good enough for the Dash because the Dash can't do much useful with the error message either :-)

It looks like we can't have our cake and eat it too. And I'd rather not pollute the public API with methods that are there purely for testing.

We could make a derived version of the interface for testing that provides access to the extra parameter, but it adds yet complexity.

Does anyone else have a bright idea?

I think I could return a future in addition to the CtrlProxy. But I'm not sure that this would necessarily work better. Results can arrive at any time (push, cancel, finished, or error), and there can be multiple pushes. A future is a one-shot API, effectively saying "wait for finish or exception".

Hmmm... Michal, you are sitting at the receiving end of the API. Would providing a future make more sense than the current finished() and error() callbacks? If an error is recorded, the future would return something like a ScopeException with an error string, regardless of the actual exception that was caught on the scope side. That's because I can't marshal arbitrary exceptions back over the wire.

I can add a future, but I'd like to do this only if there is a compelling reason. Doing so will make create_query() more complicated because it then would have two return values. (Or I overload create_query() to make the return of the future optional, but that's also more complex.)

The current callback API has the advantage that it is very simple and that C++ programmers of moderate skill level will have no problem understanding it. On the other hand, there are many C++ programmers whose eyes glaze over as soon as you mention std::future...

[Michal Hruby (mhr3)]

Well, for UI-programming futures suck, cause they block the thread :) Plus having a dedicated thread just to allow it to run each future's get sounds expensive. Moreover we do want to get the partial resultsets as soon as possible - so the virtual push() is nice. That being said, with all the threading in place, you could actually end up processing finished() before processing last push(), and that'd be bad. But I'm getting sidetracked.

I can see that futures would simplify aggregating scopes, and being able to start the query and wait for it to finish + get potential exception in the same place is really nice, and ultimately will be what you end up doing "manually" if there's no future.

So bottomline, I don't see a silver bullet, both approaches have their pros and cons.

[Michi Henning (michihenning)]

I think it's probably better, on balance, to leave it as is. It's easy enough for the scope author to use a promise/future or a condvar if they want to wait for completion.

Michal, if you can live with that, can you approve please?

[lp:~michihenning/unity-scopes-api/push-errors]

99. By Michi Henning on 2014-01-02

Merged trunk and resolved conflict.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Michal Hruby (mhr3)]

18 +unity-scopes-api (0.1.7+14.04.20131219.1-0ubuntu2) UNRELEASED; urgency=low

No need to change the -0ubuntuX suffix.

20 + [ michi ]
25 + -- michi <michi@djembefola> Thu, 02 Jan 2014 16:10:06 +1000

Could you please create ~/.devscripts with the content:
<email address hidden>
DEBFULLNAME=Your name

With that dch will produce a proper changelog entry. Also there's no need for the [Name] line, dch will do that automatically when there are multiple authors.

537 +#include <iostream> // TODO: remove this
555 +cerr << "sending error reply: " << error_message << endl;

Forgotten debug?

review: Needs Fixing

[lp:~michihenning/unity-scopes-api/push-errors]

100. By Michi Henning on 2014-01-02

Removed left-over debug trace.

[Michi Henning (michihenning)]

> 18 +unity-scopes-api (0.1.7+14.04.20131219.1-0ubuntu2) UNRELEASED;
> urgency=low
>
> No need to change the -0ubuntuX suffix.

OK, thanks for that. So, what should I do when I commit something? (Sorry, but the whole debian thing is still a bit of mystery to me.)

> 20 + [ michi ]
> 25 + -- michi <michi@djembefola> Thu, 02 Jan 2014 16:10:06 +1000
>
> Could you please create ~/.devscripts with the content:
> <email address hidden>
> DEBFULLNAME=Your name
>
> With that dch will produce a proper changelog entry. Also there's no need for
> the [Name] line, dch will do that automatically when there are multiple
> authors.

OK, I just did that, thanks!

> 537 +#include <iostream> // TODO: remove this
> 555 +cerr << "sending error reply: " << error_message << endl;
>
> Forgotten debug?

Yes, thanks for spotting this! I've just committed with this removed. Hopefully, that'll be OK this time.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Michal Hruby (mhr3)]

> OK, thanks for that. So, what should I do when I commit something? (Sorry, but
> the whole debian thing is still a bit of mystery to me.)

So, when changing API, you bump the version number as you did:
8 -set(UNITY_SCOPES_MICRO 6)
9 +set(UNITY_SCOPES_MICRO 7)

Then you run `dch -v "[new-version]-0ubuntu1"`. Ie `dch -v "0.1.7-0ubuntu1"` in this case.

That will create a new changelog entry where you just add the message, nothing else. Plus with the correct DEB* variables as mentioned above, it'll use your real name and email, not just username@hostname as is visible here (michi@djembefola, is that a real DJ btw? :))

So can you please revert the changelog change and do it again to fix it?

[Michal Hruby (mhr3)]

> So can you please revert the changelog change and do it again to fix it?

Just to clarify, no need for VCS revert, just delete the new entry and run "dch ..." again.

[lp:~michihenning/unity-scopes-api/push-errors]

101. By Michi Henning on 2014-01-02

Fixed changelog yet again.

[Michi Henning (michihenning)]

> Then you run `dch -v "[new-version]-0ubuntu1"`. Ie `dch -v "0.1.7-0ubuntu1"`
> in this case.
>
> That will create a new changelog entry where you just add the message, nothing
> else. Plus with the correct DEB* variables as mentioned above, it'll use your
> real name and email, not just username@hostname as is visible here

Thanks for that! I've just pushed this. It looks like dch is ignoring what's in my .devscripts file. Here are the contents:

<email address hidden>
DEBFULLNAME="Michi Henning"

When I run

dch -v "0.1.7-0ubuntu1"

I get the following at the bottom:

-- Michi Henning <michi@djembefola> Fri, 03 Jan 2014 07:35:43 +1000

DEBEMAIL is being ignored, it seems. I've verified that .devscripts is being read by adding a syntax error. When I do that, I get an error message. So it's not that the file has the wrong name or is in the wrong place.

When I set this as an environment variable instead, it works, so I've set it up in my .bashrc now.

> (michi@djembefola, is that a real DJ btw? :))

:-)

"Djembefola" is a Malinke word. "Fola" means to "play an instrument". So a djembefola literally means "one who plays the djembe". It's the name of my desktop machine :-)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Michal Hruby (mhr3)]

> -- Michi Henning <michi@djembefola> Fri, 03 Jan 2014 07:35:43 +1000
>
> DEBEMAIL is being ignored, it seems. I've verified that .devscripts is being
> read by adding a syntax error. When I do that, I get an error message. So it's
> not that the file has the wrong name or is in the wrong place.
>
> When I set this as an environment variable instead, it works, so I've set it
> up in my .bashrc now.

Ah, you're right indeed, I have it also both in my ~/.devscripts and ~/.bashrc

> > (michi@djembefola, is that a real DJ btw? :))
>
> :-)
>
> "Djembefola" is a Malinke word. "Fola" means to "play an instrument". So a
> djembefola literally means "one who plays the djembe". It's the name of my
> desktop machine :-)

I see, so not exactly a DJ, but close :)

review: Approve

[Michi Henning (michihenning)]

Aargh... :-(

I think the version should now be 8 because that was an incompatible ABI change. That's a problem when there is more than one branch outstanding that touches the API.

Michal, could you check please and perform whatever requisite magic we need?

[Michal Hruby (mhr3)]

> Aargh... :-(
>
> I think the version should now be 8 because that was an incompatible ABI
> change. That's a problem when there is more than one branch outstanding that
> touches the API.
>
> Michal, could you check please and perform whatever requisite magic we need?

Right, but there are no other approved branches which would bump the version number, so it's fine (we don't care about strict ABI compatibility just yet). But of course other branches which again change the API will need to be bumped to 8.

[Michi Henning (michihenning)]

> Right, but there are no other approved branches which would bump the version
> number, so it's fine (we don't care about strict ABI compatibility just yet).

Phew! :-)

> But of course other branches which again change the API will need to be bumped
> to 8.

We have a more general problem here: suppose we are at version 7. I make an incompatible change in an MR and dutifully bump the version to 8. The MR hangs around for a few days (like some just did because of the Christmas break.) Then I fix something else that's incompatible and also dutifully bump the version to 8. (Or, more likely, someone else does.) Now we have two incompatible changes, but only one version increment :-(

Depending when the branches get merged, there is a window where someone will get bitten :-(

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-12-27 02:52:30 +0000
+++ CMakeLists.txt	2014-01-02 21:33:37 +0000
@@ -150,7 +150,7 @@
 # API version
 set(UNITY_SCOPES_MAJOR 0)
 set(UNITY_SCOPES_MINOR 1)
-set(UNITY_SCOPES_MICRO 6)
+set(UNITY_SCOPES_MICRO 7)
 
 # Scopes library
 set(UNITY_SCOPES_LIB unity-scopes)

=== modified file 'debian/changelog'
--- debian/changelog	2013-12-19 21:08:02 +0000
+++ debian/changelog	2014-01-02 21:33:37 +0000
@@ -1,3 +1,11 @@
+unity-scopes-api (0.1.7-0ubuntu1) UNRELEASED; urgency=low
+
+  * Added ability for scope to push an exception (as an exception_ptr). On the client side,
+    the exception is delivered as the what() string (if the exception is a std::exception) and
+    as "unknown exception", otherwise.
+
+ -- Michi Henning <[e-mail]>  Fri, 03 Jan 2014 07:31:02 +1000
+
 unity-scopes-api (0.1.6+14.04.20131219.1-0ubuntu1) trusty; urgency=low
 
   [ Pawel Stolowski ]

=== modified file 'demo/client.cpp'
--- demo/client.cpp	2013-12-16 07:05:08 +0000
+++ demo/client.cpp	2014-01-02 21:33:37 +0000
@@ -40,28 +40,42 @@
 
     virtual void push(Category::SCPtr category) override
     {
-        cout << "received category: id=" << category->id() << " title=" << category->title() << " icon=" << category->icon() << " template=" <<
-            category->renderer_template().data() << endl;
+        cout << "received category: id=" << category->id()
+             << " title=" << category->title()
+             << " icon=" << category->icon()
+             << " template=" << category->renderer_template().data()
+             << endl;
     }
 
     virtual void push(CategorisedResult result) override
     {
-        cout << "received result: uri=" << result.uri() << " title=" << result.title() << " category id: " << result.category()->id() << endl;
+        cout << "received result: uri=" << result.uri()
+             << " title=" << result.title()
+             << " category id: "
+             << result.category()->id()
+             << endl;
     }
 
     virtual void push(Annotation annotation) override
     {
         auto links = annotation.links();
-        cout << "received annotation of type " << annotation.annotation_type() << " with " << links.size() << " link(s):" << endl;
+        cout << "received annotation of type " << annotation.annotation_type()
+             << " with " << links.size() << " link(s):"
+             << endl;
         for (auto link: links)
         {
             cout << "  " << link->query().to_string() << endl;
         }
     }
 
-    virtual void finished(ReceiverBase::Reason reason) override
+    virtual void finished(ReceiverBase::Reason reason, string const& error_message) override
     {
-        cout << "query complete, status: " << to_string(reason) << endl;
+        cout << "query complete, status: " << to_string(reason);
+        if (reason == ReceiverBase::Error)
+        {
+            cout << ": " << error_message;
+        }
+        cout << endl;
         {
             unique_lock<decltype(mutex_)> lock(mutex_);
             query_complete_ = true;

=== modified file 'demo/scopes/scope-B/scope-B.cpp'
--- demo/scopes/scope-B/scope-B.cpp	2013-12-04 12:00:33 +0000
+++ demo/scopes/scope-B/scope-B.cpp	2014-01-02 21:33:37 +0000
@@ -57,9 +57,14 @@
         }
     }
 
-    virtual void finished(Reason reason) override
+    virtual void finished(Reason reason, string const& error_message) override
     {
-        cout << "query to " << scope_name_ << " complete, status: " << to_string(reason) << endl;
+        cout << "query to " << scope_name_ << " complete, status: " << to_string(reason);
+        if (reason == ReceiverBase::Error)
+        {
+            cout << ": " << error_message;
+        }
+        cout << endl;
     }
 
     Receiver(string const& scope_name, ReplyProxy const& upstream) :

=== modified file 'include/scopes/ReceiverBase.h'
--- include/scopes/ReceiverBase.h	2013-12-27 03:32:57 +0000
+++ include/scopes/ReceiverBase.h	2014-01-02 21:33:37 +0000
@@ -84,8 +84,8 @@
     /**
     \brief Indicates the cause of a call to finished().
     The Error enumerator indicates that a query terminated abnormally, for example,
-    because a scope could not be reached over the network or terminated an query
-    abnormally.
+    because a scope could not be reached over the network, a query terminated
+    abnormally, or explicitly reported an error.
     */
     enum Reason { Finished, Cancelled, Error };
 
@@ -93,8 +93,10 @@
     \brief Called once by the scopes run time after the final result for a query() was sent.
     Exceptions thrown from finished() are ignored.
     \param r Indicates the cause for the call to finished().
+    \param error_message If r is set to Reason::Error, error_message contains further details.
+           Otherwise, error_message is the empty string.
     */
-    virtual void finished(Reason r) = 0;
+    virtual void finished(Reason r, std::string const& error_message) = 0;
 
 protected:
     /// @cond

=== modified file 'include/scopes/Reply.h'
--- include/scopes/Reply.h	2013-12-05 12:20:08 +0000
+++ include/scopes/Reply.h	2014-01-02 21:33:37 +0000
@@ -55,13 +55,15 @@
     \brief Create and register a new Category. The category is automatically sent to the source of the query.
     \return Category instance
     */
-    Category::SCPtr register_category(std::string const& id, std::string const& title, std::string const &icon, CategoryRenderer const& renderer_template =
-            CategoryRenderer());
+    Category::SCPtr register_category(std::string const& id,
+                                      std::string const& title,
+                                      std::string const &icon,
+                                      CategoryRenderer const& renderer_template = CategoryRenderer());
 
     /**
     \brief Register an existing category instance and send it to the source of the query.
-    The purpose of this call is to register a category obtained via ReplyBase::push(Category::SCPtr) when aggregating results and categories from
-    other scope(s).
+    The purpose of this call is to register a category obtained via ReplyBase::push(Category::SCPtr) when aggregating
+    results and categories from other scope(s).
     */
     void register_category(Category::SCPtr category);
 
@@ -89,13 +91,22 @@
     was sent, that is, that the query is complete.
     The scope application code is responsible for calling finished() once it has sent the
     final result for a query.
-    Multiple calls to finished() are ignored.
+    Multiple calls to finished() and calls to error() after finished() was called are ignored.
     The destructor implicitly calls finished() if a Reply goes out of scope without
     a prior call to finished().
     */
     void finished() const;
 
     /**
+    \brief Informs the source of a query that the query was terminated due to an error.
+    Multiple calls to error() and calls to finished() after error() was called are ignored.
+    \param ex An exception_ptr indicating the cause of the error. If ex is a `std::exception`,
+              the return value of `what()` is made available to the query source. Otherwise,
+              the query source receives `"unknown exception"`.
+    */
+    void error(std::exception_ptr ex) const;
+
+    /**
     \brief Destroys a Reply.
     If a Reply goes out of scope without a prior call to finished(), the destructor implicitly calls finished().
     */

=== modified file 'include/scopes/internal/MWReply.h'
--- include/scopes/internal/MWReply.h	2013-11-25 19:05:41 +0000
+++ include/scopes/internal/MWReply.h	2014-01-02 21:33:37 +0000
@@ -44,7 +44,7 @@
     virtual ~MWReply() noexcept;
 
     virtual void push(VariantMap const& result) = 0;
-    virtual void finished(ReceiverBase::Reason reason) = 0;
+    virtual void finished(ReceiverBase::Reason reason, std::string const& error_message) = 0;
 
 protected:
     MWReply(MiddlewareBase* mw_base);

=== modified file 'include/scopes/internal/ReplyImpl.h'
--- include/scopes/internal/ReplyImpl.h	2013-12-05 12:20:08 +0000
+++ include/scopes/internal/ReplyImpl.h	2014-01-02 21:33:37 +0000
@@ -66,6 +66,7 @@
     bool push(unity::api::scopes::Annotation const& annotation);
     void finished();
     void finished(unity::api::scopes::ReceiverBase::Reason reason);
+    void error(std::exception_ptr ex);
 
     static ReplyProxy create(MWReplyProxy const& mw_proxy, std::shared_ptr<QueryObject> const& qo);
 

=== modified file 'include/scopes/internal/ReplyObject.h'
--- include/scopes/internal/ReplyObject.h	2013-11-25 19:05:41 +0000
+++ include/scopes/internal/ReplyObject.h	2014-01-02 21:33:37 +0000
@@ -56,7 +56,7 @@
 
     // Remote operation implementations
     void push(VariantMap const& result) noexcept;
-    void finished(ReceiverBase::Reason reason) noexcept;
+    void finished(ReceiverBase::Reason reason, std::string const& error_message) noexcept;
 
 private:
     ReceiverBase::SPtr const receiver_base_;

=== modified file 'include/scopes/internal/zmq_middleware/ZmqReply.h'
--- include/scopes/internal/zmq_middleware/ZmqReply.h	2013-11-25 19:05:41 +0000
+++ include/scopes/internal/zmq_middleware/ZmqReply.h	2014-01-02 21:33:37 +0000
@@ -45,7 +45,7 @@
     virtual ~ZmqReply() noexcept;
 
     virtual void push(VariantMap const& result) override;
-    virtual void finished(ReceiverBase::Reason reason) override;
+    virtual void finished(ReceiverBase::Reason reason, std::string const& error_message) override;
 };
 
 } // namespace zmq_middleware

=== modified file 'src/Reply.cpp'
--- src/Reply.cpp	2013-12-05 12:20:08 +0000
+++ src/Reply.cpp	2014-01-02 21:33:37 +0000
@@ -73,6 +73,10 @@
     return fwd()->finished();
 }
 
+void Reply::error(std::exception_ptr ex) const
+{
+    return fwd()->error(ex);
+}
 internal::ReplyImpl* Reply::fwd() const
 {
     return dynamic_cast<internal::ReplyImpl*>(pimpl());

=== modified file 'src/internal/QueryCtrlImpl.cpp'
--- src/internal/QueryCtrlImpl.cpp	2013-11-25 19:05:41 +0000
+++ src/internal/QueryCtrlImpl.cpp	2014-01-02 21:33:37 +0000
@@ -26,6 +26,7 @@
 #include <scopes/ScopeExceptions.h>
 
 #include <cassert>
+#include <iostream> // TODO: remove this once logging is added
 
 using namespace std;
 
@@ -65,10 +66,11 @@
         // Indicate (to ourselves) that this query is complete. Calling via the MWReplyProxy ensures
         // the finished() call will be processed by a seperate server-side thread,
         // so we cannot block here.
-        reply_proxy_->finished(ReceiverBase::Cancelled);
+        reply_proxy_->finished(ReceiverBase::Cancelled, "");
     }
-    catch (MiddlewareException const& e)
+    catch (std::exception const& e)
     {
+        cerr << e.what() << endl;
         // TODO: log error
     }
 }

=== modified file 'src/internal/QueryObject.cpp'
--- src/internal/QueryObject.cpp	2013-12-13 04:48:38 +0000
+++ src/internal/QueryObject.cpp	2014-01-02 21:33:37 +0000
@@ -94,15 +94,15 @@
     catch (std::exception const& e)
     {
         pushable_ = false;
-        reply_->finished(ReceiverBase::Error);     // Oneway, can't block
         // TODO: log error
+        reply_->finished(ReceiverBase::Error, e.what());     // Oneway, can't block
         cerr << "ScopeBase::run(): " << e.what() << endl;
     }
     catch (...)
     {
         pushable_ = false;
-        reply_->finished(ReceiverBase::Error);     // Oneway, can't block
         // TODO: log error
+        reply_->finished(ReceiverBase::Error, "unknown exception");     // Oneway, can't block
         cerr << "ScopeBase::run(): unknown exception" << endl;
     }
 }
@@ -116,7 +116,7 @@
         // Send finished() to up-stream client to tell him the query is done.
         // We send via the MWReplyProxy here because that allows passing
         // a ReceiverBase::Reason (whereas the public ReplyProxy does not).
-        reply_->finished(ReceiverBase::Cancelled);     // Oneway, can't block
+        reply_->finished(ReceiverBase::Cancelled, "");     // Oneway, can't block
     }
 
     // Forward the cancellation to the query base (which in turn will forward it to any subqueries).

=== modified file 'src/internal/ReplyImpl.cpp'
--- src/internal/ReplyImpl.cpp	2013-12-06 11:22:12 +0000
+++ src/internal/ReplyImpl.cpp	2014-01-02 21:33:37 +0000
@@ -30,6 +30,7 @@
 
 #include <sstream>
 #include <cassert>
+#include <iostream> // TODO: remove this once logging is added
 
 using namespace std;
 
@@ -73,7 +74,10 @@
     push(category);
 }
 
-Category::SCPtr ReplyImpl::register_category(std::string const& id, std::string const& title, std::string const &icon, CategoryRenderer const& renderer_template)
+Category::SCPtr ReplyImpl::register_category(std::string const& id,
+                                             std::string const& title,
+                                             std::string const &icon,
+                                             CategoryRenderer const& renderer_template)
 {
     auto cat = cat_registry_->register_category(id, title, icon, renderer_template); // will throw if adding same category again
 
@@ -139,7 +143,7 @@
         catch (MiddlewareException const& e)
         {
             // TODO: log error
-            finished(ReceiverBase::Error);
+            error(current_exception());
             return false;
         }
     }
@@ -157,15 +161,44 @@
     {
         try
         {
-            fwd()->finished(reason);
+            fwd()->finished(reason, "");
         }
         catch (MiddlewareException const& e)
         {
             // TODO: log error
+            cerr << e.what() << endl;
         }
     }
 }
 
+void ReplyImpl::error(exception_ptr ex)
+{
+    string error_message;
+    try
+    {
+        rethrow_exception(ex);
+    }
+    catch (std::exception const& e)
+    {
+        error_message = e.what();
+    }
+    catch (...)
+    {
+        error_message = "unknown exception";
+    }
+
+    try
+    {
+        fwd()->finished(ReceiverBase::Error, error_message);
+    }
+    catch (MiddlewareException const& e)
+    {
+        // TODO: log error
+        cerr << e.what() << endl;
+    }
+}
+
+
 ReplyProxy ReplyImpl::create(MWReplyProxy const& mw_proxy, std::shared_ptr<QueryObject> const& qo)
 {
     return ReplyProxy(new Reply((new ReplyImpl(mw_proxy, qo))));

=== modified file 'src/internal/ReplyObject.cpp'
--- src/internal/ReplyObject.cpp	2013-12-17 01:39:35 +0000
+++ src/internal/ReplyObject.cpp	2014-01-02 21:33:37 +0000
@@ -22,7 +22,6 @@
 #include <scopes/ReceiverBase.h>
 #include <scopes/Category.h>
 #include <scopes/CategorisedResult.h>
-#include <unity/Exception.h>
 
 #include <cassert>
 #include <iostream> // TODO: remove this once logging is added
@@ -57,7 +56,7 @@
 {
     try
     {
-        finished(ReceiverBase::Finished);
+        finished(ReceiverBase::Finished, "");
     }
     catch (...)
     {
@@ -110,10 +109,11 @@
                 Annotation annotation(new internal::AnnotationImpl(*cat_registry_, result_var));
                 receiver_base_->push(std::move(annotation));
             }
-            catch (unity::Exception const& e)
+            catch (std::exception const& e)
             {
-                // TODO: this is an internal error; log error
-                finished(ReceiverBase::Error);
+                // TODO: log this
+                cerr << "ReplyObject::receiver_base_->push(): " << e.what() << endl;
+                finished(ReceiverBase::Error, e.what());
             }
         }
 
@@ -128,9 +128,9 @@
             }
             catch (std::exception const& e)
             {
-                // TODO: this is an internal error; log error
+                // TODO: log this
                 cerr << "ReplyObject::receiver_base_->push(): " << e.what() << endl;
-                finished(ReceiverBase::Error);
+                finished(ReceiverBase::Error, e.what());
             }
         }
     }
@@ -140,7 +140,7 @@
         cerr << "ReplyObject::push(VariantMap): " << e.what() << endl;
         try
         {
-            finished(ReceiverBase::Error);
+            finished(ReceiverBase::Error, e.what());
         }
         catch (...)
         {
@@ -152,7 +152,7 @@
         cerr << "ReplyObject::push(VariantMap): unknown exception" << endl;
         try
         {
-            finished(ReceiverBase::Error);
+            finished(ReceiverBase::Error, "unknown exception");
         }
         catch (...)
         {
@@ -165,7 +165,7 @@
     }
 }
 
-void ReplyObject::finished(ReceiverBase::Reason r) noexcept
+void ReplyObject::finished(ReceiverBase::Reason r, string const& error_message) noexcept
 {
     // We permit exactly one finished() call for a query. This avoids
     // a race condition where the executing down-stream query invokes
@@ -188,7 +188,7 @@
     lock.unlock(); // Inform the application code that the query is complete outside synchronization.
     try
     {
-        receiver_base_->finished(r);
+        receiver_base_->finished(r, error_message);
     }
     catch (std::exception const& e)
     {

=== modified file 'src/internal/ScopeImpl.cpp'
--- src/internal/ScopeImpl.cpp	2013-12-13 04:48:38 +0000
+++ src/internal/ScopeImpl.cpp	2014-01-02 21:33:37 +0000
@@ -79,7 +79,7 @@
         {
             // TODO: if things go wrong, we need to make sure that the reply object
             // is disconnected from the middleware, so it gets deallocated.
-            reply->finished(ReceiverBase::Error);
+            reply->finished(ReceiverBase::Error, e.what());
             throw;
         }
         catch (...)

=== modified file 'src/internal/ScopeObject.cpp'
--- src/internal/ScopeObject.cpp	2013-12-13 04:48:38 +0000
+++ src/internal/ScopeObject.cpp	2014-01-02 21:33:37 +0000
@@ -123,7 +123,7 @@
     {
         try
         {
-            reply->finished(ReceiverBase::Error);
+            reply->finished(ReceiverBase::Error, e.what());
         }
         catch (...)
         {
@@ -136,7 +136,7 @@
     {
         try
         {
-            reply->finished(ReceiverBase::Error);
+            reply->finished(ReceiverBase::Error, "unknown exception");
         }
         catch (...)
         {

=== modified file 'src/internal/zmq_middleware/ReplyI.cpp'
--- src/internal/zmq_middleware/ReplyI.cpp	2013-12-12 16:02:06 +0000
+++ src/internal/zmq_middleware/ReplyI.cpp	2014-01-02 21:33:37 +0000
@@ -81,6 +81,7 @@
     auto req = in_params.getAs<capnproto::Reply::FinishedRequest>();
     auto r = req.getReason();
     ReceiverBase::Reason reason;
+    string err;
     switch (r)
     {
         case capnproto::Reply::FinishedReason::FINISHED:
@@ -96,6 +97,7 @@
         case capnproto::Reply::FinishedReason::ERROR:
         {
             reason = ReceiverBase::Error;
+            err = req.getError();
             break;
         }
         default:
@@ -104,7 +106,7 @@
             reason = ReceiverBase::Error; // LCOV_EXCL_LINE
         }
     }
-    delegate->finished(reason);
+    delegate->finished(reason, err);
 }
 
 } // namespace zmq_middleware

=== modified file 'src/internal/zmq_middleware/ZmqReply.cpp'
--- src/internal/zmq_middleware/ZmqReply.cpp	2013-12-02 18:10:46 +0000
+++ src/internal/zmq_middleware/ZmqReply.cpp	2014-01-02 21:33:37 +0000
@@ -71,7 +71,7 @@
     future.wait();
 }
 
-void ZmqReply::finished(ReceiverBase::Reason reason)
+void ZmqReply::finished(ReceiverBase::Reason reason, string const& error_message)
 {
     capnp::MallocMessageBuilder request_builder;
     auto request = make_request_(request_builder, "finished");
@@ -92,6 +92,7 @@
         case ReceiverBase::Error:
         {
             r = capnproto::Reply::FinishedReason::ERROR;
+            in_params.setError(error_message);
             break;
         }
         default:

=== modified file 'src/internal/zmq_middleware/capnproto/Reply.capnp'
--- src/internal/zmq_middleware/capnproto/Reply.capnp	2013-11-22 04:00:45 +0000
+++ src/internal/zmq_middleware/capnproto/Reply.capnp	2014-01-02 21:33:37 +0000
@@ -54,4 +54,5 @@
 struct FinishedRequest
 {
     reason @0 : FinishedReason;
+    error @1  : Text;               # Present only if reason is error
 }

=== modified file 'test/gtest/scopes/Runtime/Runtime_test.cpp'
--- test/gtest/scopes/Runtime/Runtime_test.cpp	2013-12-06 10:42:26 +0000
+++ test/gtest/scopes/Runtime/Runtime_test.cpp	2014-01-02 21:33:37 +0000
@@ -53,9 +53,10 @@
         EXPECT_EQ("dnd_uri", result.dnd_uri());
         count_++;
     }
-    virtual void finished(ReceiverBase::Reason reason) override
+    virtual void finished(ReceiverBase::Reason reason, string const& error_message) override
     {
         EXPECT_EQ(Finished, reason);
+        EXPECT_EQ("", error_message);
         EXPECT_EQ(1, count_);
         // Signal that the query has completed.
         unique_lock<mutex> lock(mutex_);