Naios / continuable

C++14 asynchronous allocation aware futures (supporting then, exception handling, coroutines and connections)
https://naios.github.io/continuable/
MIT License
815 stars 44 forks source link

How to get error code when using use_continuable with asio #32

Closed Gei0r closed 4 years ago

Gei0r commented 4 years ago

@Naios

Commit Hash

61826817c7716cec5e476a68f679d9347891bba7

Expected Behavior

std::exception_ptr parameter to .fail() captures the error when using use_continuable with boost asio

Actual Behavior

The std::exception_ptr is null.

Steps to Reproduce

#include <boost/asio.hpp>
#include <boost/asio/steady_timer.hpp>
#include <iostream>
#include <iomanip>
#include <chrono>
#include <memory>

#include <continuable/continuable.hpp>
#include <continuable/external/asio.hpp>

namespace asio = boost::asio;
namespace sys = boost::system;

int main()
{
    asio::io_service io;

    auto time = std::chrono::seconds{1};
    asio::steady_timer timer{io, time};

    timer.async_wait(cti::use_continuable)
        .then([time]() {
            std::cout << time.count() << " s elapsed\n"; })
        .fail([](std::exception_ptr pe) {
            // !!!! Here I expect pe to point to operation_aborted !!!!
            std::cerr << "fail! pe: " << std::boolalpha << (bool)pe << '\n';
            if(!pe) return;
            try{std::rethrow_exception(pe);}
            catch(std::exception &e){std::cerr << e.what();}
            catch(...) {std::cerr << "something else";}});

    // This function forces the completion of any pending asynchronous wait
    // operations against the timer.
    // The handler for each cancelled operation will be invoked with the
    // boost::asio::error::operation_aborted error code.
    timer.cancel();

    io.run();
}

Output: fail! pe: false

Your Environment

Naios commented 4 years ago

continuable maps asio::error::basic_errors::operation_aborted to a default constructed exception_t (std::exception_ptr in your case) which signals cancellation of one of the previous tasks as shown below:

https://github.com/Naios/continuable/blob/61826817c7716cec5e476a68f679d9347891bba7/include/continuable/detail/external/asio.hpp#L118-L129

The reasons for this behaviour is that:

  1. The default exception handler should not treat asio::error::basic_errors::operation_aborted as an unhandled error.
  2. The behaviour of timer_.cancel() should align with promise.set_canceled(), or return cancel()
  3. It is much cheaper in the overall system to signal cancellation through a default constructed exception type than through a special runtime exception type. Also this behaviour wouldn't be portable when using error_codes or custom exception types instead of exceptions.

All other asio exceptions can be cought through:

try {
  std::rethrow_exception(pe);
} catch (boost::system::error_code const& ec) [
 // ...
}

I'm open for a discussion about this behaviour and probably it needs to be stated more clearly in the documentation of continuable_base::fail and use_continuable_t.

Gei0r commented 4 years ago

I think it's important to know "what happened" in the fail() handler. If a default-constructed exception_ptr as fail() parameter always means "operation aborted" or other "cancelled by the user / by the program itself" than I think it's OK. But it needs to be clearly documented.

That was the first thing I did to try out the fail() handler and the result was unexpected 😉

Gei0r commented 4 years ago

I don't think it can stay the way it currently is, because this leads to disaster:

#include <boost/asio.hpp>
#include <boost/asio/steady_timer.hpp>
#include <iostream>
#include <chrono>

#define CONTINUABLE_WITH_EXPERIMENTAL_COROUTINE
#include <continuable/continuable.hpp>
#include <continuable/external/asio.hpp>

namespace asio = boost::asio;

asio::io_service io;
asio::steady_timer *timer = nullptr;

cti::continuable<> coro()
{
    std::cout << "creating timer object..." << std::endl;
    timer = new asio::steady_timer(io, std::chrono::seconds{1});

    // ----------- Adding try/catch doesn't help --------------
    try
    {
        co_await timer->async_wait(cti::use_continuable);
        std::cout << "Wait done\n";
    }
    catch(...)
    {
        std::cerr << "fail" << std::endl;
    }
}

int main()
{
    coro()
        .then([](){std::cout << "all done!\n";})
        .fail([](){std::cout << "fail\n";});

    timer->cancel();

    io.run();
}

Output:

creating timer object...
terminating
Aborted (core dumped)
Naios commented 4 years ago

Thanks for your feedback, the co_await expressions when applied to a continuable_base wasn't able to handle the default constructed exception type as of now. I bridged this through throwing a await_canceled_exception which gets converted to a default constructed exception type again if being left unhandled in the coroutine.

Through that your code fully succeeds correctly:

#include <chrono>
#include <iostream>
#include <memory>
#include <asio.hpp>

#define CONTINUABLE_WITH_EXPERIMENTAL_COROUTINE
#include <continuable/continuable.hpp>
#include <continuable/external/asio.hpp>

using namespace cti;

asio::io_context io;
std::unique_ptr<asio::steady_timer> timer;

continuable<> coro() {
  std::cout << "creating timer object..." << std::endl;
  timer = std::make_unique<asio::steady_timer>(io, std::chrono::seconds{1});

  // ----------- Adding try/catch doesn't help --------------
  try {
    co_await timer->async_wait(use_continuable);
    std::cout << "Wait done\n";
  } catch (...) {
    std::cerr << "fail" << std::endl;
    std::rethrow_exception(std::current_exception());
  }

  co_return;
}

continuable<> coro2() {
  co_await make_cancelling_continuable<void>();

  co_return;
}

int main() {
  coro()
      .then([]() {
        std::cout << "all done!\n";
      })
      .fail([](exception_t e) {
        if (e) {
          std::cout << "all fail\n";
        } else {
          std::cout << "all canceled\n";
        }
      });

  timer->cancel();

  coro2().fail([](exception_t e) {
    assert(bool(e) == false);
  });

  io.run();
}
Naios commented 4 years ago

I addressed this issue in multiple bugfix commits: 744809d8f and 8187c16ede for instance, if you have any furter issues with the completion token open a new issue please.

Also I've added the use_continuable_raw token which passes the asio result without any mapping to the user.