DesktopStreamer: fix uncaught exception after #177

apps/DesktopStreamer/MainWindow.cpp

@@ -214,16 +214,15 @@ void MainWindow::_showMultiWindowMode()
 
     _listView->setVisible(true);
 
-    // select 'Desktop' item as initial default stream item
-    _listView->setCurrentIndex(_listView->model()->index(0, 0));
+    const auto desktopIndex = _listView->model()->index(0, 0);
+    _listView->setCurrentIndex(desktopIndex);
     _streamButton->setText(streamSelected);
 
+    const auto itemsCount = _listView->model()->rowCount();
     const int itemsHorizontal =
-        std::min(3.f,
-                 std::ceil(std::sqrt(float(_listView->model()->rowCount()))));
+        std::min(3.f, std::ceil(std::sqrt(float(itemsCount))));
     const int itemsVertical =
-        std::min(3.f, std::ceil(float(_listView->model()->rowCount()) /
-                                itemsHorizontal));
+        std::min(3.f, std::ceil(float(itemsCount) / itemsHorizontal));
 
     layout()->setSizeConstraint(QLayout::SetDefaultConstraint);
     setFixedSize(QWIDGETSIZE_MAX, QWIDGETSIZE_MAX);
@@ -267,45 +266,30 @@ void MainWindow::_updateStreams()
 void MainWindow::_updateMultipleStreams()
 {
 #ifdef DEFLECT_USE_QT5MACEXTRAS
-    const QModelIndexList windowIndices =
-        _listView->selectionModel()->selectedIndexes();
+    const auto windowIndices = _listView->selectionModel()->selectedIndexes();
 
     StreamMap streams;
-    for (const QPersistentModelIndex& index : windowIndices)
+    for (const auto& index : windowIndices)
     {
         if (_streams.count(index))
         {
             streams[index] = _streams[index];
             continue;
         }
 
-        const std::string appName = index.isValid()
-                                        ? _listView->model()
-                                              ->data(index, Qt::DisplayRole)
-                                              .toString()
-                                              .toStdString()
-                                        : std::string();
-        const std::string streamId = std::to_string(++_streamID) + " " +
-                                     appName + " - " +
-                                     _streamIdLineEdit->text().toStdString();
-        const int pid = index.isValid()
-                            ? _listView->model()
-                                  ->data(index, DesktopWindowsModel::ROLE_PID)
-                                  .toInt()
-                            : 0;
-        const std::string host = _getStreamHost();
-        StreamPtr stream(new Stream(*this, index, streamId, host, pid));
-
-        if (!stream->isConnected())
+        const auto appName = _getAppName(index);
+        const auto streamId = _getFormattedStreamId(++_streamID, appName);
+        const auto host = _getStreamHost();
+        const auto pid = _getAppPid(index);
+
+        try
         {
-            _showConnectionErrorStatus();
-            continue;
+            streams.emplace(index, _makeStream(index, streamId, host, pid));
+        }
+        catch (const std::runtime_error& e)
+        {
+            _showConnectionErrorStatus(e.what());
         }
-
-        if (_remoteControlCheckBox->isChecked())
-            stream->registerForEvents();
-
-        streams[index] = stream;
     }
     _streams.swap(streams);
 
@@ -325,28 +309,41 @@ void MainWindow::_updateSingleStream()
         return;
     }
 
-    if (_streams.empty())
+    if (!_streams.empty())
+        return;
+
+    const auto index = QPersistentModelIndex(); // default == use desktop
+    const auto streamId = _getStreamId();
+    const auto host = _getStreamHost();
+    const auto pid = 0;
+
+    try
     {
-        const QPersistentModelIndex index; // default == use desktop
-        StreamPtr stream(new Stream(*this, index,
-                                    _streamIdLineEdit->text().toStdString(),
-                                    _getStreamHost()));
-        if (stream->isConnected())
-        {
-            if (_remoteControlCheckBox->isChecked())
-                stream->registerForEvents();
-            _streams[index] = stream;
-            _startStreaming();
-        }
-        else
-            _showConnectionErrorStatus();
+        _streams.emplace(index, _makeStream(index, streamId, host, pid));
+        _startStreaming();
+    }
+    catch (const std::runtime_error& e)
+    {
+        _showConnectionErrorStatus(e.what());
     }
 }
 
-void MainWindow::_showConnectionErrorStatus()
+MainWindow::StreamPtr MainWindow::_makeStream(const QPersistentModelIndex index,
+                                              const std::string& id,
+                                              const std::string& host,
+                                              const int pid) const
 {
-    _statusbar->showMessage(
-        QString("Cannot connect to host: '%1'").arg(_getStreamHost().c_str()));
+    auto stream = std::make_shared<Stream>(*this, index, id, host, pid);
+
+    if (_remoteControlCheckBox->isChecked())
+        stream->registerForEvents();
+
+    return stream;
+}
+
+void MainWindow::_showConnectionErrorStatus(const QString& message)
+{
+    _statusbar->showMessage(message);
 }
 
 void MainWindow::_processStreamEvents()
@@ -365,7 +362,7 @@ void MainWindow::_processStreamEvents()
         }
     }
 
-    for (ConstStreamPtr stream : closedStreams)
+    for (auto stream : closedStreams)
         _deselect(stream);
 }
 
@@ -428,6 +425,39 @@ void MainWindow::_regulateFrameRate()
     }
 }
 
+std::string MainWindow::_getAppName(const QModelIndex& appIndex) const
+{
+    if (!appIndex.isValid())
+        return std::string();
+
+    const auto name = _listView->model()->data(appIndex, Qt::DisplayRole);
+    return name.toString().toStdString();
+}
+
+int MainWindow::_getAppPid(const QModelIndex& appIndex) const
+{
+    if (!appIndex.isValid())
+        return 0;
+
+#ifdef DEFLECT_USE_QT5MACEXTRAS
+    const auto pidRole = DesktopWindowsModel::ROLE_PID;
+    return _listView->model()->data(appIndex, pidRole).toInt();
+#else
+    return 0;
+#endif
+}
+
+std::string MainWindow::_getFormattedStreamId(const uint32_t id,
+                                              const std::string& appName) const
+{
+    return std::to_string(id) + " " + appName + " - " + _getStreamId();
+}
+
+std::string MainWindow::_getStreamId() const
+{
+    return _streamIdLineEdit->text().toStdString();
+}
+
 std::string MainWindow::_getStreamHost() const
 {
     if (_hostComboBox->findText(_hostComboBox->currentText()) != -1 &&

apps/DesktopStreamer/MainWindow.h

@@ -104,12 +104,20 @@ private slots:
     void _updateStreams();
     void _updateMultipleStreams();
     void _updateSingleStream();
-    void _showConnectionErrorStatus();
+    void _showConnectionErrorStatus(const QString& message);
+    StreamPtr _makeStream(QPersistentModelIndex index, const std::string& id,
+                          const std::string& host, int pid) const;
 
     void _deselect(ConstStreamPtr stream);
     void _processStreamEvents();
     void _shareDesktopUpdate();
     void _regulateFrameRate();
+
+    std::string _getAppName(const QModelIndex& appIndex) const;
+    int _getAppPid(const QModelIndex& appIndex) const;
+    std::string _getFormattedStreamId(uint32_t id,
+                                      const std::string& appName) const;
+    std::string _getStreamId() const;
     std::string _getStreamHost() const;
     deflect::ChromaSubsampling _getSubsampling() const;
 };

deflect/Socket.cpp

@@ -47,7 +47,8 @@
 #include <QDataStream>
 #include <QLoggingCategory>
 #include <QTcpSocket>
-#include <iostream>
+
+#include <sstream>
 
 namespace
 {
@@ -67,10 +68,7 @@ Socket::Socket(const std::string& host, const unsigned short port)
     // _connect(): QObject::connect: Cannot connect (null)::destroyed() to
     // QHostInfoLookupManager::waitForThreadPoolDone()
     if (!qApp)
-    {
-        QLoggingCategory* log = QLoggingCategory::defaultCategory();
-        log->setEnabled(QtWarningMsg, false);
-    }
+        QLoggingCategory::defaultCategory()->setEnabled(QtWarningMsg, false);
 
     _connect(host, port);
 
@@ -178,33 +176,30 @@ bool Socket::_receiveHeader(MessageHeader& messageHeader)
     return stream.status() == QDataStream::Ok;
 }
 
-bool Socket::_connect(const std::string& host, const unsigned short port)
+void Socket::_connect(const std::string& host, const unsigned short port)
 {
     _socket->connectToHost(host.c_str(), port);
     if (!_socket->waitForConnected(RECEIVE_TIMEOUT_MS))
     {
-        std::cerr << "could not connect to " << host << ":" << port
-                  << std::endl;
-        return false;
+        std::stringstream ss;
+        ss << "could not connect to " << host << ":" << port;
+        throw std::runtime_error(ss.str());
     }
 
     if (!_receiveProtocolVersion())
     {
-        std::cerr << "server protocol version was not received" << std::endl;
         _socket->disconnectFromHost();
-        return false;
+        throw std::runtime_error("server protocol version was not received");
     }
 
     if (_serverProtocolVersion < NETWORK_PROTOCOL_VERSION)
     {
-        std::cerr << "server uses unsupported protocol: "
-                  << _serverProtocolVersion << " < " << NETWORK_PROTOCOL_VERSION
-                  << std::endl;
         _socket->disconnectFromHost();
-        return false;
+        std::stringstream ss;
+        ss << "server uses unsupported protocol: " << _serverProtocolVersion
+           << " < " << NETWORK_PROTOCOL_VERSION;
+        throw std::runtime_error(ss.str());
     }
-
-    return true;
 }
 
 bool Socket::_receiveProtocolVersion()

deflect/Socket.h

@@ -70,6 +70,7 @@ class Socket : public QObject
      * Construct a Socket and connect to host.
      * @param host The target host (IP address or hostname)
      * @param port The target port
+     * @throw std::runtime_error if the socket could not connect
      */
     DEFLECT_API Socket(const std::string& host, unsigned short port);
 
@@ -128,7 +129,7 @@ class Socket : public QObject
     int32_t _serverProtocolVersion;
 
     bool _receiveHeader(MessageHeader& messageHeader);
-    bool _connect(const std::string& host, const unsigned short port);
+    void _connect(const std::string& host, const unsigned short port);
     bool _receiveProtocolVersion();
     bool _write(const QByteArray& data);
 };

deflect/Stream.h

@@ -114,7 +114,7 @@ class Stream : public Observer
      * @param image The image to send. Note that the image is not copied, so the
      *              referenced must remain valid until the send is finished.
      * @return true if the image data could be sent, false otherwise
-     * @throw std::invalid_argument if RGBA and uncompressed
+     * @throw std::invalid_argument if not RGBA and uncompressed
      * @throw std::invalid_argument if invalid JPEG compression arguments
      * @throw std::runtime_error if pending finishFrame() has not been completed
      * @throw std::runtime_error if JPEG compression failed

deflect/StreamPrivate.cpp

@@ -85,10 +85,6 @@ StreamPrivate::StreamPrivate(const std::string& id_, const std::string& host,
     , socket{_getStreamHost(host), port}
     , sendWorker{socket, id}
 {
-    if (!socket.isConnected())
-        throw std::runtime_error(
-            "Connection to deflect server could not be established");
-
     socket.connect(&socket, &Socket::disconnected, [this]() {
         if (disconnectedCallback)
             disconnectedCallback();

tests/cpp/SocketTests.cpp

@@ -1,6 +1,6 @@
 /*********************************************************************/
-/* Copyright (c) 2013, EPFL/Blue Brain Project                       */
-/*                     Raphael Dumusc <raphael.dumusc@epfl.ch>       */
+/* Copyright (c) 2013-2017, EPFL/Blue Brain Project                  */
+/*                          Raphael Dumusc <raphael.dumusc@epfl.ch>  */
 /* All rights reserved.                                              */
 /*                                                                   */
 /* Redistribution and use in source and binary forms, with or        */
@@ -52,9 +52,15 @@ void testSocketConnect(const int32_t versionOffset)
 {
     MinimalDeflectServer server(versionOffset);
 
-    deflect::Socket socket("localhost", server.serverPort());
-
-    BOOST_CHECK(socket.isConnected() == (versionOffset >= 0));
+    try
+    {
+        deflect::Socket socket("localhost", server.serverPort());
+        BOOST_CHECK(socket.isConnected());
+    }
+    catch (const std::runtime_error&)
+    {
+        BOOST_CHECK(versionOffset < 0);
+    }
 }
 
 BOOST_AUTO_TEST_CASE(