Better error exposure around JobHandle (aka QFuture<std::expected<>>)

Quotient/avatar.cpp

@@ -63,13 +63,13 @@ QImage Avatar::get(int width, int height, get_callback_t callback) const
 QFuture<QUrl> Avatar::upload(const QString& fileName) const
 {
     d->uploadRequest = d->connection->uploadFile(fileName);
-    return d->uploadRequest.responseFuture();
+    return d->uploadRequest.toFuture();
 }
 
 QFuture<QUrl> Avatar::upload(QIODevice* source) const
 {
     d->uploadRequest = d->connection->uploadContent(source);
-    return d->uploadRequest.responseFuture();
+    return d->uploadRequest.toFuture();
 }
 
 bool Avatar::isEmpty() const { return d->_url.isEmpty(); }

Quotient/connection.cpp

@@ -672,10 +672,11 @@ JobHandle<JoinRoomJob> Connection::joinRoom(const QString& roomAlias, const QStr
         .then([this](const QString& roomId) { provideRoom(roomId); });
 }
 
-QFuture<Room*> Connection::joinAndGetRoom(const QString& roomAlias, const QStringList& serverNames)
+QFuture<JobResult<Room *>> Connection::joinAndGetRoom(const QString &roomAlias,
+                                                      const QStringList &serverNames)
 {
     return callApi<JoinRoomJob>(roomAlias, serverNames, serverNames)
-        .then([this](const QString& roomId) { return provideRoom(roomId); });
+        .then([this](const QString &roomId) { return provideRoom(roomId); }, &BaseJob::status);
 }
 
 QFuture<Room *> Connection::waitForNewRoom(const QString &roomId)
@@ -839,10 +840,20 @@ JobHandle<CreateRoomJob> Connection::createRoom(
 
 void Connection::requestDirectChat(const QString& userId)
 {
-    getDirectChat(userId).then([this](Room* r) { emit directChatAvailable(r); });
+    tryGetDirectChat(userId).then([this](const JobResult<Room *> &expRoom) {
+        if (expRoom)
+            emit directChatAvailable(*expRoom);
+    });
+}
+
+QFuture<Room *> Connection::getDirectChat(const QString &otherUserId)
+{
+    return tryGetDirectChat(otherUserId).then([](JobResult<Room *> expRoom) {
+        return expRoom.value_or(nullptr);
+    });
 }
 
-QFuture<Room*> Connection::getDirectChat(const QString& otherUserId)
+QFuture<JobResult<Room*>> Connection::tryGetDirectChat(const QString& otherUserId)
 {
     auto* u = user(otherUserId);
     if (QUO_ALARM_X(!u, u"Couldn't get a user object for" % otherUserId))
@@ -861,7 +872,7 @@ QFuture<Room*> Connection::getDirectChat(const QString& otherUserId)
                 continue;
             qCDebug(MAIN) << "Requested direct chat with" << otherUserId
                           << "is already available as" << r->id();
-            return QtFuture::makeReadyValueFuture(r);
+            return QtFuture::makeReadyValueFuture<JobResult<Room *>>(r);
         }
         if (auto ir = invitation(roomId)) {
             Q_ASSERT(ir->id() == roomId);
@@ -889,9 +900,9 @@ QFuture<Room*> Connection::getDirectChat(const QString& otherUserId)
         emit directChatsListChanged({}, removals);
     }
 
-    return createDirectChat(otherUserId).then([this](const QString& roomId) {
+    return createDirectChat(otherUserId).then([this](const QString &roomId) {
         return room(roomId, JoinState::Join);
-    });
+    }, &BaseJob::status);
 }
 
 JobHandle<CreateRoomJob> Connection::createDirectChat(const QString& userId, const QString& topic,
@@ -1463,7 +1474,7 @@ QFuture<QList<LoginFlow>> Connection::setHomeserver(const QUrl& baseUrl)
             d->loginFlows.clear();
         emit loginFlowsChanged();
     });
-    return d->loginFlowsJob.responseFuture();
+    return d->loginFlowsJob.toFuture();
 }
 
 void Connection::saveRoomState(Room* r) const

Quotient/connection.h

@@ -615,9 +615,12 @@ class QUOTIENT_API Connection : public QObject {
     //! \sa LoginFlowsJob, loginFlows, loginFlowsChanged, homeserverChanged
     Q_INVOKABLE QFuture<QList<LoginFlow> > setHomeserver(const QUrl& baseUrl);
 
-    //! \brief Get a future to a direct chat with the user
+    [[deprecated("Use tryGetDirectChat() instead")]]
     Q_INVOKABLE QFuture<Room*> getDirectChat(const QString& otherUserId);
 
+    //! \brief Get a future to a direct chat with the user
+    Q_INVOKABLE QFuture<JobResult<Room *>> tryGetDirectChat(const QString &otherUserId);
+
     //! Create a direct chat with a single user, optional name and topic
     //!
     //! A room will always be created, unlike in requestDirectChat.
@@ -631,8 +634,8 @@ class QUOTIENT_API Connection : public QObject {
     Q_INVOKABLE JobHandle<JoinRoomJob> joinRoom(const QString& roomAlias,
                                                 const QStringList& serverNames = {});
 
-    Q_INVOKABLE QFuture<Room*> joinAndGetRoom(const QString& roomAlias,
-                                              const QStringList& serverNames = {});
+    Q_INVOKABLE QFuture<JobResult<Room *>> joinAndGetRoom(const QString &roomAlias,
+                                                          const QStringList &serverNames = {});
 
     Q_INVOKABLE QFuture<Room *> waitForNewRoom(const QString &roomId);
 

Quotient/jobs/jobhandle.h

@@ -5,6 +5,8 @@
 #include <QtCore/QFuture>
 #include <QtCore/QPointer>
 
+#include <expected>
+
 namespace Quotient {
 
 template <typename FnT, typename JobT>
@@ -191,9 +193,19 @@ class QUOTIENT_API JobHandle : public QPointer<JobT>, public QFuture<JobT*> {
     }
 
     //! Get a QFuture for the value returned by `collectResponse()` called on the underlying job
-    auto responseFuture()
+    auto toFuture()
+    {
+        return future_type::then([](future_type ft) mutable {
+            auto *const job = ft.result();
+            if (!job->status().good())
+                ft.cancel();
+            return collectResponse(job);
+        });
+    }
+
+    auto toFutureExpected()
     {
-        return future_type::then([](auto* j) { return collectResponse(j); });
+        return then([] (JobT* j) { return collectResponse(j); }, &BaseJob::status);
     }
 
     //! \brief Abandon the underlying job, if there's one pending
@@ -222,17 +234,17 @@ class QUOTIENT_API JobHandle : public QPointer<JobT>, public QFuture<JobT*> {
         auto callFn(future_value_type job)
         {
             if constexpr (std::invocable<FnT>) {
-                return std::forward<FnT>(fn)();
+                return std::invoke(std::forward<FnT>(fn));
             } else {
                 static_assert(AllowJobArg, "onCanceled continuations should not accept arguments");
-                if constexpr (requires { fn(job); })
-                    return fn(job);
+                if constexpr (std::invocable<FnT, future_value_type>)
+                    return std::invoke(std::forward<FnT>(fn), job);
                 else if constexpr (requires { collectResponse(job); }) {
                     static_assert(
-                        requires { fn(collectResponse(job)); },
+                        std::invocable<FnT, decltype(collectResponse(job))>,
                         "The continuation function must accept either of: 1) no arguments; "
                         "2) the job pointer itself; 3) the value returned by collectResponse(job)");
-                    return fn(collectResponse(job));
+                    return std::invoke(std::forward<FnT>(fn), collectResponse(job));
                 }
             }
         }
@@ -291,8 +303,12 @@ class QUOTIENT_API JobHandle : public QPointer<JobT>, public QFuture<JobT*> {
             else if constexpr (std::is_same_v<FailureFnT, Skip>) {
                 // Still call fFn to suppress unused lambda warning
                 return job->status().good() ? sFn(job) : (fFn(job), sType{});
-            } else
+            } else if constexpr (std::is_same_v<sType, fType>)
                 return job->status().good() ? sFn(job) : fFn(job);
+            else {
+                using result_t = std::expected<sType, fType>;
+                return job->status().good() ? result_t(sFn(job)) : std::unexpected(fFn(job));
+            }
         };
     }
 
@@ -339,6 +355,9 @@ class QUOTIENT_API JobHandle : public QPointer<JobT>, public QFuture<JobT*> {
 template <std::derived_from<BaseJob> JobT>
 JobHandle(JobT*) -> JobHandle<JobT>;
 
+template <typename ResultT>
+using JobResult = std::expected<ResultT, BaseJob::Status>;
+
 } // namespace Quotient
 
 Q_DECLARE_SMART_POINTER_METATYPE(Quotient::JobHandle)

Quotient/room.cpp

@@ -1325,14 +1325,13 @@ inline namespace v16 {
 }
 }
 
-QFuture<std::expected<Room *, BaseJob::Status>> Room::upgrade(QString newVersion,
-                                                              const QStringList &additionalCreators)
+QFuture<JobResult<Room *>> Room::upgrade(QString newVersion, const QStringList &additionalCreators)
 {
     if (!successorId().isEmpty()) {
         Q_ASSERT(!successorId().isEmpty());
         emit upgradeFailed(tr("The room is already upgraded"));
     }
-    using future_t = std::expected<Room*, BaseJob::Status>;
+    using future_t = JobResult<Room*>;
     return connection()
         ->callApi<CSAPI::v16::UpgradeRoomJob>(id(), newVersion, additionalCreators)
         .then(

Quotient/room.h

@@ -820,9 +820,10 @@ class QUOTIENT_API Room : public QObject {
     //! a new room of the specified version. It is possible to specify \p additionalCreators for
     //! room versions that support those (unfortunately it is only possible to find out whether
     //! a given room version supports additional creators by attempting to upgrade a room).
-    //! \return a future eventually holding a new room once it arrives via sync
-    QFuture<std::expected<Room *, BaseJob::Status>> upgrade(
-        QString newVersion, const QStringList &additionalCreators = {});
+    //! \return a future eventually holding either a new room once it arrives via sync,
+    //!         or the failed upgrade job status if the upgrade wasn't successful
+    QFuture<JobResult<Room *>> upgrade(QString newVersion,
+                                         const QStringList &additionalCreators = {});
 
 public Q_SLOTS:
     /** Check whether the room should be upgraded */

Quotient/util.h

@@ -384,4 +384,12 @@ struct QUOTIENT_API HomeserverData {
 
     bool checkMatrixSpecVersion(QStringView targetVersion) const;
 };
+
+//! Basic concept for all specialisations of std::expected
+template <typename T>
+concept Expected_Class = requires(T exp) {
+    exp.value();
+    exp.error();
+};
+
 } // namespace Quotient

quotest/quotest.cpp

@@ -313,15 +313,18 @@ void TestManager::setupAndRun(const QString& targetRoomAlias)
     c->setLazyLoading(true);
 
     qInfo() << "Joining" << targetRoomAlias;
-    c->joinAndGetRoom(targetRoomAlias).then(this, [this](Room* room) {
-        if (!room) {
-            qCritical() << "Failed to join the test room";
+    c->joinAndGetRoom(targetRoomAlias)
+        .then(this, [this](const JobResult<Room *> &expectedRoom) {
+        if (!expectedRoom) {
+            auto logLine = qCritical();
+            logLine << "Failed to join the test room: ";
+            expectedRoom.error().dumpToLog(logLine);
             finalize();
             return;
         }
         // Ensure that the room has been joined and filled with some events
         // so that other tests could use that
-        testSuite = new TestSuite(room, origin, this);
+        testSuite = new TestSuite(*expectedRoom, origin, this);
         // Only start the sync after joining, to make sure the room just
         // joined is in it
         c->syncLoop();