changeset 170:590b1a1fd955

More work on error and exception handling
author Chris Cannam <cannam@all-day-breakfast.com>
date Tue, 31 Jan 2017 14:53:24 +0000
parents f13dc1db2229
children 8d2b12442903 cfa115746cb3
files vamp-capnp/VampnProto.h vamp-client/CapnpRRClient.h vamp-client/Exceptions.h vamp-client/PluginStub.h vamp-client/SynchronousTransport.h vamp-client/qt/AutoPlugin.h vamp-client/qt/ProcessQtTransport.h
diffstat 7 files changed, 149 insertions(+), 63 deletions(-) [+]
line wrap: on
line diff
--- a/vamp-capnp/VampnProto.h	Tue Jan 31 11:26:07 2017 +0000
+++ b/vamp-capnp/VampnProto.h	Tue Jan 31 14:53:24 2017 +0000
@@ -912,7 +912,7 @@
     getRequestResponseType(const piper::RpcResponse::Reader &r) {
         switch (r.getResponse().which()) {
         case piper::RpcResponse::Response::Which::ERROR:
-            return RRType::NotValid; //!!! or error type? test this
+            return RRType::NotValid;
         case piper::RpcResponse::Response::Which::LIST:
             return RRType::List;
         case piper::RpcResponse::Response::Which::LOAD:
--- a/vamp-client/CapnpRRClient.h	Tue Jan 31 11:26:07 2017 +0000
+++ b/vamp-client/CapnpRRClient.h	Tue Jan 31 14:53:24 2017 +0000
@@ -143,11 +143,8 @@
 
         LOG_E("CapnpRRClient::listPluginData called");
         
-        if (!m_transport->isOK()) {
-            log("Piper server crashed or failed to start (caller should have checked this)");
-            throw std::runtime_error("Piper server crashed or failed to start");
-        }
-
+        checkServerOK();
+        
         capnp::MallocMessageBuilder message;
         piper::RpcRequest::Builder builder = message.initRoot<piper::RpcRequest>();
         VampnProto::buildRpcRequest_List(builder, req);
@@ -174,11 +171,8 @@
 
         LOG_E("CapnpRRClient::loadPlugin called");
         
-        if (!m_transport->isOK()) {
-            log("Piper server crashed or failed to start (caller should have checked this)");
-            throw std::runtime_error("Piper server crashed or failed to start");
-        }
-
+        checkServerOK();
+        
         LoadResponse resp;
         PluginHandleMapper::Handle handle = serverLoad(req.pluginKey,
                                                        req.inputSampleRate,
@@ -210,12 +204,9 @@
               PluginConfiguration config) override {
 
         LOG_E("CapnpRRClient::configure called");
+
+        checkServerOK();
         
-        if (!m_transport->isOK()) {
-            log("Piper server crashed or failed to start (caller should have checked this)");
-            throw std::runtime_error("Piper server crashed or failed to start");
-        }
-
         ConfigurationRequest request;
         request.plugin = plugin;
         request.configuration = config;
@@ -232,8 +223,6 @@
         capnp::FlatArrayMessageReader responseMessage(karr);
         piper::RpcResponse::Reader reader = responseMessage.getRoot<piper::RpcResponse>();
 
-        //!!! handle (explicit) error case
-
         checkResponseType(reader, piper::RpcResponse::Response::Which::CONFIGURE, id);
 
         ConfigurationResponse cr;
@@ -254,11 +243,8 @@
 
         LOG_E("CapnpRRClient::process called");
         
-        if (!m_transport->isOK()) {
-            log("Piper server crashed or failed to start (caller should have checked this)");
-            throw std::runtime_error("Piper server crashed or failed to start");
-        }
-
+        checkServerOK();
+        
         ProcessRequest request;
         request.plugin = plugin;
         request.inputBuffers = inputBuffers;
@@ -275,8 +261,6 @@
         capnp::FlatArrayMessageReader responseMessage(karr);
         piper::RpcResponse::Reader reader = responseMessage.getRoot<piper::RpcResponse>();
 
-        //!!! handle (explicit) error case
-
         checkResponseType(reader, piper::RpcResponse::Response::Which::PROCESS, id);
 
         ProcessResponse pr;
@@ -294,11 +278,8 @@
 
         LOG_E("CapnpRRClient::finish called");
         
-        if (!m_transport->isOK()) {
-            log("Piper server crashed or failed to start (caller should have checked this)");
-            throw std::runtime_error("Piper server crashed or failed to start");
-        }
-
+        checkServerOK();
+        
         FinishRequest request;
         request.plugin = plugin;
         
@@ -314,8 +295,6 @@
         capnp::FlatArrayMessageReader responseMessage(karr);
         piper::RpcResponse::Reader reader = responseMessage.getRoot<piper::RpcResponse>();
 
-        //!!! handle (explicit) error case
-
         checkResponseType(reader, piper::RpcResponse::Response::Which::FINISH, id);
 
         FinishResponse pr;
@@ -341,11 +320,8 @@
 
         log("CapnpRRClient: reset() called, plugin will be closed and reloaded");
         
-        if (!m_transport->isOK()) {
-            log("Piper server crashed or failed to start (caller should have checked this)");
-            throw std::runtime_error("Piper server crashed or failed to start");
-        }
-
+        checkServerOK();
+        
         if (m_mapper.havePlugin(plugin)) {
             (void)finish(plugin); // server-side unload
         }
@@ -384,23 +360,58 @@
     }
 
     void
+    checkServerOK() {
+        if (!m_transport->isOK()) {
+            log("Piper server crashed or failed to start (caller should have checked this)");
+            throw ServerCrashed();
+        }
+    }
+    
+    /**
+     * Check (i) that the response has the same id as supplied (which
+     * presumably is the corresponding request id) and (ii) that the
+     * response has the expected type.
+     *
+     * Return only if both of these things are the case.
+     * 
+     * If the response has the right id but is an error response,
+     * throw a ServiceError exception with the error response's
+     * message in it.
+     *
+     * If the response has the wrong id, or if it has the wrong type
+     * and is not an error response, throw ProtocolError. (i.e. for
+     * cases having errors that are not conveyed through our official
+     * error response.)
+     */
+    void
     checkResponseType(const piper::RpcResponse::Reader &r,
                       piper::RpcResponse::Response::Which type,
                       ReqId id) {
         
-        if (r.getResponse().which() != type) {
-            std::ostringstream s;
-            s << "checkResponseType: wrong response type (received "
-              << int(r.getResponse().which()) << ", expected " << int(type) << ")";
-            log(s.str());
-            throw std::runtime_error("Wrong response type");
-        }
         if (ReqId(r.getId().getNumber()) != id) {
             std::ostringstream s;
             s << "checkResponseType: wrong response id (received "
               << r.getId().getNumber() << ", expected " << id << ")";
             log(s.str());
-            throw std::runtime_error("Wrong response id");
+            throw ProtocolError("Wrong response id");
+        }
+        if (r.getResponse().which() != type) {
+            if (r.getResponse().which() == piper::RpcResponse::Response::Which::ERROR) {
+                int code;
+                std::string message;
+                VampnProto::readRpcResponse_Error(code, message, r);
+                std::ostringstream s;
+                s << "checkResponseType: received an error with message: "
+                  << message;
+                log(s.str());
+                throw ServiceError(message);
+            } else {
+                std::ostringstream s;
+                s << "checkResponseType: wrong response type (received "
+                  << int(r.getResponse().which()) << ", expected " << int(type) << ")";
+                log(s.str());
+                throw ProtocolError("Wrong response type");
+            }
         }
     }
 
@@ -433,14 +444,9 @@
 
         auto karr = call(message, "load", false);
 
-        //!!! ... --> will also need some way to kill this process
-        //!!! (from another thread)
-
         capnp::FlatArrayMessageReader responseMessage(karr);
         piper::RpcResponse::Reader reader = responseMessage.getRoot<piper::RpcResponse>();
 
-        //!!! handle (explicit) error case
-
         checkResponseType(reader, piper::RpcResponse::Response::Which::LOAD, id);
         
         const piper::LoadResponse::Reader &lr = reader.getResponse().getLoad();
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/vamp-client/Exceptions.h	Tue Jan 31 14:53:24 2017 +0000
@@ -0,0 +1,70 @@
+/* -*- c-basic-offset: 4 indent-tabs-mode: nil -*-  vi:set ts=8 sts=4 sw=4: */
+/*
+  Piper C++
+
+  An API for audio analysis and feature extraction plugins.
+
+  Centre for Digital Music, Queen Mary, University of London.
+  Copyright 2006-2016 Chris Cannam and QMUL.
+  
+  Permission is hereby granted, free of charge, to any person
+  obtaining a copy of this software and associated documentation
+  files (the "Software"), to deal in the Software without
+  restriction, including without limitation the rights to use, copy,
+  modify, merge, publish, distribute, sublicense, and/or sell copies
+  of the Software, and to permit persons to whom the Software is
+  furnished to do so, subject to the following conditions:
+
+  The above copyright notice and this permission notice shall be
+  included in all copies or substantial portions of the Software.
+
+  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR
+  ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF
+  CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+  WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+  Except as contained in this notice, the names of the Centre for
+  Digital Music; Queen Mary, University of London; and Chris Cannam
+  shall not be used in advertising or otherwise to promote the sale,
+  use or other dealings in this Software without prior written
+  authorization.
+*/
+
+#ifndef PIPER_CLIENT_EXCEPTIONS_H
+#define PIPER_CLIENT_EXCEPTIONS_H
+
+namespace piper_vamp {
+namespace client {
+
+class ServerCrashed : public std::runtime_error
+{
+public:
+    ServerCrashed() : std::runtime_error("Piper server exited unexpectedly") {}
+};
+
+class RequestTimedOut : public std::runtime_error
+{
+public:
+    RequestTimedOut() : std::runtime_error("Piper request timed out") {}
+};
+
+class ProtocolError : public std::runtime_error
+{
+public:
+    ProtocolError() : std::runtime_error("Piper protocol error") {}
+    ProtocolError(const char *msg) : std::runtime_error(msg) {}
+};
+
+class ServiceError : public std::runtime_error
+{
+public:
+    ServiceError(std::string msg) : std::runtime_error(msg) {}
+};
+
+}
+}
+
+#endif
--- a/vamp-client/PluginStub.h	Tue Jan 31 11:26:07 2017 +0000
+++ b/vamp-client/PluginStub.h	Tue Jan 31 14:53:24 2017 +0000
@@ -261,7 +261,6 @@
             throw std::logic_error("Plugin has already been disposed of");
         }
 
-        //!!! ew
         std::vector<std::vector<float> > vecbuf;
         for (int c = 0; c < m_config.channelCount; ++c) {
             vecbuf.push_back(std::vector<float>
--- a/vamp-client/SynchronousTransport.h	Tue Jan 31 11:26:07 2017 +0000
+++ b/vamp-client/SynchronousTransport.h	Tue Jan 31 14:53:24 2017 +0000
@@ -52,12 +52,6 @@
     virtual State check(const std::vector<char> &message) const = 0;
 };
 
-class ServerCrashed : public std::runtime_error
-{
-public:
-    ServerCrashed() : std::runtime_error("Piper server exited unexpectedly") {}
-};
-
 class LogCallback
 {
 public:
--- a/vamp-client/qt/AutoPlugin.h	Tue Jan 31 11:26:07 2017 +0000
+++ b/vamp-client/qt/AutoPlugin.h	Tue Jan 31 14:53:24 2017 +0000
@@ -38,12 +38,21 @@
 
 #include "ProcessQtTransport.h"
 #include "../CapnpRRClient.h"
+#include "../Exceptions.h"
 
 #include <cstdint>
 
 namespace piper_vamp {
 namespace client {
 
+/**
+ * This "plugin" make the Piper client abstraction behave like a local
+ * Vamp plugin, with its own server that lasts only for the lifetime
+ * of this plugin and serves only it.
+ *
+ * Note that any method may throw ServerCrashed, RequestTimedOut or
+ * ProtocolError exceptions.
+ */
 class AutoPlugin : public Vamp::Plugin
 {
 public:
@@ -129,7 +138,14 @@
     virtual bool initialise(size_t inputChannels,
                             size_t stepSize,
                             size_t blockSize) {
-        return getPlugin()->initialise(inputChannels, stepSize, blockSize);
+        try {
+            return getPlugin()->initialise(inputChannels, stepSize, blockSize);
+        } catch (const ServiceError &e) {
+            // Sadly, the Vamp API has taught hosts to try to divine
+            // initialisation problems from a bool return value alone
+            log(std::string("AutoPlugin: initialise failed: ") + e.what());
+            return false;
+        }
     }
 
     virtual void reset() {
--- a/vamp-client/qt/ProcessQtTransport.h	Tue Jan 31 11:26:07 2017 +0000
+++ b/vamp-client/qt/ProcessQtTransport.h	Tue Jan 31 14:53:24 2017 +0000
@@ -37,6 +37,7 @@
 #define PIPER_PROCESS_QT_TRANSPORT_H
 
 #include "../SynchronousTransport.h"
+#include "../Exceptions.h"
 
 #include <QProcess>
 #include <QString>
@@ -181,12 +182,14 @@
                 if (responseStarted) {
                     if (duringResponseTimeout > 0 && ms > duringResponseTimeout) {
                         log("Server timed out during response");
-                        throw std::runtime_error("Request timed out");
+                        m_crashed = true;
+                        throw RequestTimedOut();
                     }
                 } else {
                     if (beforeResponseTimeout > 0 && ms > beforeResponseTimeout) {
                         log("Server timed out before response");
-                        throw std::runtime_error("Request timed out");
+                        m_crashed = true;
+                        throw RequestTimedOut();
                     }
                 }
                 
@@ -230,9 +233,7 @@
                 switch (m_completenessChecker->check(buffer)) {
                 case MessageCompletenessChecker::Complete: complete = true; break;
                 case MessageCompletenessChecker::Incomplete: break;
-                case MessageCompletenessChecker::Invalid:
-                    throw std::runtime_error
-                        ("Invalid message received: corrupt stream from server?");
+                case MessageCompletenessChecker::Invalid: throw ProtocolError();
                 }
                 (void)t.restart(); // reset timeout when we read anything
             }