LibCore: Slightly rework the Core::Promise API

The previous iteration of this API was somewhat odd and rough in random
places, which degraded usability and made less than perfect sense.
This commit reworks the API to be a little closer to more
conventional promise APIs (a la javascript promises).

Also adds a test to ensure the class even works.

Tests/LibCore/CMakeLists.txt

@@ -1,10 +1,11 @@
 set(TEST_SOURCES
     TestLibCoreArgsParser.cpp
-    TestLibCoreFileWatcher.cpp
     TestLibCoreDeferredInvoke.cpp
-    TestLibCoreStream.cpp
     TestLibCoreFilePermissionsMask.cpp
+    TestLibCoreFileWatcher.cpp
+    TestLibCorePromise.cpp
     TestLibCoreSharedSingleProducerCircularQueue.cpp
+    TestLibCoreStream.cpp
 )
 
 foreach(source IN LISTS TEST_SOURCES)

Tests/LibCore/TestLibCorePromise.cpp

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2023, Ali Mohammad Pur <mpfard@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibCore/EventLoop.h>
+#include <LibCore/Promise.h>
+#include <LibTest/TestSuite.h>
+
+TEST_CASE(promise_await_async_event)
+{
+    Core::EventLoop loop;
+
+    auto promise = MUST(Core::Promise<int>::try_create());
+
+    loop.deferred_invoke([=] {
+        promise->resolve(42);
+    });
+
+    auto& result = promise->await();
+    EXPECT(!result.is_error());
+    EXPECT_EQ(result.value(), 42);
+}
+
+TEST_CASE(promise_await_async_event_rejection)
+{
+    Core::EventLoop loop;
+
+    auto promise = MUST(Core::Promise<int>::try_create());
+
+    loop.deferred_invoke([=] {
+        promise->reject(AK::Error::from_string_literal("lol no"));
+    });
+
+    auto& result = promise->await();
+    EXPECT(result.is_error());
+    EXPECT_EQ(result.error().string_literal(), "lol no"sv);
+}
+
+TEST_CASE(promise_chain_handlers)
+{
+    Core::EventLoop loop;
+
+    bool resolved = false;
+    bool rejected = false;
+
+    NonnullRefPtr<Core::Promise<int>> promise = MUST(Core::Promise<int>::try_create())
+                                                    ->when_resolved([&](int&) -> ErrorOr<void> { resolved = true; return {}; })
+                                                    .when_rejected([&](AK::Error const&) { rejected = true; });
+
+    loop.deferred_invoke([=] {
+        promise->resolve(42);
+    });
+
+    promise->await();
+    EXPECT(resolved);
+    EXPECT(!rejected);
+}

Userland/Applications/Browser/CookieJar.cpp

@@ -608,10 +608,10 @@ void CookieJar::select_all_cookies_from_database(OnSelectAllCookiesResult on_res
                         on_result(cookie.release_value());
                 },
                 [&]() {
-                    MUST(promise->resolve({}));
+                    promise->resolve({});
                 },
                 [&](auto) {
-                    MUST(promise->resolve({}));
+                    promise->resolve({});
                 });
 
             MUST(promise->await());

Userland/Libraries/LibCore/Forward.h

@@ -6,6 +6,8 @@
 
 #pragma once
 
+#include <AK/Forward.h>
+
 namespace Core {
 
 class AnonymousBuffer;
@@ -32,7 +34,7 @@ class Object;
 class ObjectClassRegistration;
 class ProcessStatisticsReader;
 class Socket;
-template<typename Result>
+template<typename Result, typename TError = AK::Error>
 class Promise;
 class SocketAddress;
 class TCPServer;

Userland/Libraries/LibCore/Promise.h

@@ -1,76 +1,127 @@
 /*
  * Copyright (c) 2021, Kyle Pereira <hey@xylepereira.me>
  * Copyright (c) 2022, kleines Filmröllchen <filmroellchen@serenityos.org>
+ * Copyright (c) 2021-2023, Ali Mohammad Pur <mpfard@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #pragma once
 
+#include <AK/Concepts.h>
 #include <LibCore/EventLoop.h>
 #include <LibCore/Object.h>
 
 namespace Core {
 
-template<typename Result>
+template<typename Result, typename TError>
 class Promise : public Object {
     C_OBJECT(Promise);
 
 public:
-    Function<ErrorOr<void>(Result&)> on_resolved;
+    using ErrorType = TError;
 
-    ErrorOr<void> resolve(Result&& result)
+    Function<ErrorOr<void>(Result&)> on_resolution;
+    Function<void(ErrorType&)> on_rejection;
+
+    void resolve(Result&& result)
     {
-        m_pending_or_error = move(result);
+        m_result_or_rejection = move(result);
 
-        if (on_resolved)
-            return on_resolved(m_pending_or_error->value());
-        return {};
+        if (on_resolution) {
+            auto handler_result = on_resolution(m_result_or_rejection->value());
+            possibly_handle_rejection(handler_result);
+        }
     }
 
-    void cancel(Error error)
+    void reject(ErrorType&& error)
     {
-        m_pending_or_error = move(error);
+        m_result_or_rejection = move(error);
+        possibly_handle_rejection(*m_result_or_rejection);
     }
 
-    bool is_canceled()
+    bool is_rejected()
     {
-        return m_pending_or_error.has_value() && m_pending_or_error->is_error();
+        return m_result_or_rejection.has_value() && m_result_or_rejection->is_error();
     }
 
     bool is_resolved() const
     {
-        return m_pending_or_error.has_value() && !m_pending_or_error->is_error();
+        return m_result_or_rejection.has_value() && !m_result_or_rejection->is_error();
     }
 
-    ErrorOr<Result> await()
+    ErrorOr<Result, ErrorType>& await()
     {
-        while (!m_pending_or_error.has_value())
+        while (!m_result_or_rejection.has_value())
             Core::EventLoop::current().pump();
 
-        return m_pending_or_error.release_value();
+        return *m_result_or_rejection;
     }
 
     // Converts a Promise<A> to a Promise<B> using a function func: A -> B
     template<typename T>
-    RefPtr<Promise<T>> map(T func(Result&))
+    RefPtr<Promise<T>> map(Function<T(Result&)> func)
     {
         RefPtr<Promise<T>> new_promise = Promise<T>::construct();
-        on_resolved = [new_promise, func](Result& result) {
-            auto t = func(result);
-            return new_promise->resolve(move(t));
+        on_resolution = [new_promise, func = move(func)](Result& result) -> ErrorOr<void> {
+            new_promise->resolve(func(result));
+            return {};
+        };
+        on_rejection = [new_promise](ErrorType& error) {
+            new_promise->reject(move(error));
         };
         return new_promise;
     }
 
+    template<CallableAs<void, Result&> F>
+    Promise& when_resolved(F handler)
+    {
+        on_resolution = [handler = move(handler)](Result& result) { return handler(result); };
+        if (is_resolved()) {
+            auto handler_result = on_resolution(m_result_or_rejection->value());
+            possibly_handle_rejection(handler_result);
+        }
+
+        return *this;
+    }
+
+    template<CallableAs<ErrorOr<void>, Result&> F>
+    Promise& when_resolved(F handler)
+    {
+        on_resolution = move(handler);
+        if (is_resolved()) {
+            auto handler_result = on_resolution(m_result_or_rejection->value());
+            possibly_handle_rejection(handler_result);
+        }
+
+        return *this;
+    }
+
+    template<CallableAs<void, ErrorType&> F>
+    Promise& when_rejected(F handler)
+    {
+        on_rejection = move(handler);
+        if (is_rejected())
+            on_rejection(m_result_or_rejection->error());
+
+        return *this;
+    }
+
 private:
+    template<typename T>
+    void possibly_handle_rejection(ErrorOr<T>& result)
+    {
+        if (result.is_error() && on_rejection)
+            on_rejection(result.error());
+    }
+
     Promise() = default;
     Promise(Object* parent)
         : Object(parent)
     {
     }
 
-    Optional<ErrorOr<Result>> m_pending_or_error;
+    Optional<ErrorOr<Result, ErrorType>> m_result_or_rejection;
 };
 
 }

Userland/Libraries/LibCore/ThreadEventQueue.cpp

@@ -76,7 +76,7 @@ void ThreadEventQueue::cancel_all_pending_jobs()
 {
     Threading::MutexLocker lock(m_private->mutex);
     for (auto const& promise : m_private->pending_promises)
-        promise->cancel(Error::from_errno(ECANCELED));
+        promise->reject(Error::from_errno(ECANCELED));
 
     m_private->pending_promises.clear();
 }
@@ -87,7 +87,7 @@ size_t ThreadEventQueue::process()
     {
         Threading::MutexLocker locker(m_private->mutex);
         events = move(m_private->queued_events);
-        m_private->pending_promises.remove_all_matching([](auto& job) { return job->is_resolved() || job->is_canceled(); });
+        m_private->pending_promises.remove_all_matching([](auto& job) { return job->is_resolved() || job->is_rejected(); });
     }
 
     size_t processed_events = 0;

Userland/Libraries/LibIMAP/Client.cpp

@@ -64,7 +64,7 @@ ErrorOr<void> Client::on_ready_to_receive()
 
     // Once we get server hello we can start sending.
     if (m_connect_pending) {
-        TRY(m_connect_pending->resolve({}));
+        m_connect_pending->resolve({});
         m_connect_pending.clear();
         m_buffer.clear();
         return {};
@@ -227,13 +227,13 @@ ErrorOr<void> Client::handle_parsed_response(ParseStatus&& parse_status)
         bool should_send_next = false;
         if (!parse_status.successful) {
             m_expecting_response = false;
-            TRY(m_pending_promises.first()->resolve({}));
+            m_pending_promises.first()->resolve({});
             m_pending_promises.remove(0);
         }
         if (parse_status.response.has_value()) {
             m_expecting_response = false;
             should_send_next = parse_status.response->has<SolidResponse>();
-            TRY(m_pending_promises.first()->resolve(move(parse_status.response)));
+            m_pending_promises.first()->resolve(move(parse_status.response));
             m_pending_promises.remove(0);
         }
 
@@ -386,7 +386,7 @@ RefPtr<Promise<Optional<SolidResponse>>> Client::append(StringView mailbox, Mess
     auto response_promise = Promise<Optional<Response>>::construct();
     m_pending_promises.append(response_promise);
 
-    continue_req->on_resolved = [this, message2 { move(message) }](auto& data) -> ErrorOr<void> {
+    continue_req->on_resolution = [this, message2 { move(message) }](auto& data) -> ErrorOr<void> {
         if (!data.has_value()) {
             TRY(handle_parsed_response({ .successful = false, .response = {} }));
         } else {

Userland/Libraries/LibThreading/BackgroundAction.h

@@ -61,7 +61,7 @@ private:
         , m_on_complete(move(on_complete))
     {
         if (m_on_complete) {
-            m_promise->on_resolved = [](NonnullRefPtr<Core::Object>& object) -> ErrorOr<void> {
+            m_promise->on_resolution = [](NonnullRefPtr<Core::Object>& object) -> ErrorOr<void> {
                 auto self = static_ptr_cast<BackgroundAction<Result>>(object);
                 VERIFY(self->m_result.has_value());
                 if (auto maybe_error = self->m_on_complete(self->m_result.value()); maybe_error.is_error())
@@ -78,7 +78,7 @@ private:
         enqueue_work([this, origin_event_loop = &Core::EventLoop::current()] {
             auto result = m_action(*this);
             // The event loop cancels the promise when it exits.
-            m_canceled |= m_promise->is_canceled();
+            m_canceled |= m_promise->is_rejected();
             auto callback_scheduled = false;
             // All of our work was successful and we weren't cancelled; resolve the event loop's promise.
             if (!m_canceled && !result.is_error()) {
@@ -99,7 +99,7 @@ private:
                 if (result.is_error())
                     error = result.release_error();
 
-                m_promise->cancel(Error::from_errno(ECANCELED));
+                m_promise->reject(Error::from_errno(ECANCELED));
                 if (!m_canceled && m_on_error) {
                     callback_scheduled = true;
                     origin_event_loop->deferred_invoke([this, error = move(error)]() mutable {

Userland/Services/WebDriver/Session.cpp

@@ -65,8 +65,7 @@ ErrorOr<NonnullRefPtr<Core::LocalServer>> Session::create_server(NonnullRefPtr<S
     server->on_accept = [this, promise](auto client_socket) {
         auto maybe_connection = adopt_nonnull_ref_or_enomem(new (nothrow) WebContentConnection(move(client_socket)));
         if (maybe_connection.is_error()) {
-            // Use of MUST in this function is safe, as our promise callback can never error out.
-            MUST(promise->resolve(maybe_connection.release_error()));
+            promise->resolve(maybe_connection.release_error());
             return;
         }
 
@@ -85,11 +84,11 @@ ErrorOr<NonnullRefPtr<Core::LocalServer>> Session::create_server(NonnullRefPtr<S
         if (m_current_window_handle.is_empty())
             m_current_window_handle = window_handle;
 
-        MUST(promise->resolve({}));
+        promise->resolve({});
     };
 
     server->on_accept_error = [promise](auto error) {
-        MUST(promise->resolve(move(error)));
+        promise->resolve(move(error));
     };
 
     return server;