Remove pplx::task from public API #8747
Conversation
BrennanConroy
requested review from
halter73,
davidfowl,
mikaelm12,
analogrelay and
jkotalik
BrennanConroy
force-pushed
the
brecon/lowapi
branch
from
b1bf056
to
0cffafa
Compare
BrennanConroy
force-pushed
the
brecon/publicapi
branch
from
ed46001
to
d4915e8
Compare
src/SignalR/clients/cpp/samples/HubConnectionSample/HubConnectionSample.cpp
Show resolved
Hide resolved
src/SignalR/clients/cpp/samples/HubConnectionSample/HubConnectionSample.cpp
Show resolved
Hide resolved
src/SignalR/clients/cpp/src/signalrclient/hub_connection_impl.cpp
Outdated
Show resolved
Hide resolved
src/SignalR/clients/cpp/src/signalrclient/hub_connection_impl.cpp
Outdated
Show resolved
Hide resolved
| { | ||
| return m_pImpl->start(); | ||
| m_pImpl->start(callback); |
There was a problem hiding this comment.
Do you need checks for the implementation object here as well?
| } | ||
|
|
||
| ucout << U("Enter your message:"); | ||
| for (;;) |
|
|
||
| SIGNALRCLIENT_API pplx::task<void> __cdecl send(const std::string& data); | ||
| SIGNALRCLIENT_API void __cdecl send(const std::string& data, std::function<void(std::exception_ptr)> callback) noexcept; | ||
|
|
||
| SIGNALRCLIENT_API void __cdecl set_message_received(const message_received_handler& message_received_callback); | ||
| SIGNALRCLIENT_API void __cdecl set_disconnected(const std::function<void __cdecl()>& disconnected_callback); |
There was a problem hiding this comment.
Would it make sense to have set_disconnected's completion callback take an exception as well?
There was a problem hiding this comment.
Probably, I'll track that in our public API review issue #8717
| @@ -29,16 +28,16 @@ namespace signalr | |||
|
|
|||
| connection& operator=(const connection&) = delete; | |||
|
|
|||
| SIGNALRCLIENT_API pplx::task<void> __cdecl start(); | |||
| SIGNALRCLIENT_API void __cdecl start(std::function<void(std::exception_ptr)> callback) noexcept; | |||
There was a problem hiding this comment.
Can these callbacks be marked noexcept too? std::function<void(std::exception_ptr) noexcept>
There was a problem hiding this comment.
I thought about it, but I don't think it works
https://godbolt.org/z/dI2GuJ
| { | ||
| if (!m_pImpl) | ||
| { | ||
| throw signalr_exception("invoke() cannot be called on uninitialized hub_connection instance"); | ||
| callback(web::json::value(), std::make_exception_ptr(signalr_exception("invoke() cannot be called on destructed hub_connection instance"))); |
There was a problem hiding this comment.
Is web::json::value() equivalent to null in this case?
| auto handshake_exception = std::current_exception(); | ||
| connection->m_connection->stop([callback, handshake_exception](std::exception_ptr) | ||
| { | ||
| callback(handshake_exception); |
There was a problem hiding this comment.
Can we merge or log the stop exception if there is one?
There was a problem hiding this comment.
We log the exception in connection_impl::stop
| set_result(json::value::null()); | ||
| else | ||
| { | ||
| set_result(json::value::null()); |
There was a problem hiding this comment.
Should this be web::json::value() instead? The docs seem to indicate that value() is just a null value and null() is "A JSON null value".
|
This comment was made automatically. If there is a problem contact ryanbrandenburg. I've triaged the above build. |
Built on top of #8420Part of #8718