C API for custom websocket #6168
Conversation
There was a problem hiding this comment.
Would you mind adding a unit test for these interfaces? You don't need to use an actual client or websocket implementation - just verify the functions work like we expect. Take a look at some of the tests under the realm_config_t test case in test/object-store/c_api/c_api.cpp
| RLM_API void realm_sync_socket_callback_complete(realm_sync_socket_callback* realm_callback, | ||
| status_error_code_e status, const char* reason) | ||
| { | ||
| (*(realm_callback->get()))(Status{static_cast<ErrorCodes::Error>(status), reason}); |
There was a problem hiding this comment.
For status_error_code_e of OK, we'll need to pass Status::OK() instead of creating a Status object.
I was just looking at it. I was thinking to actually inject the default implementations and make sure everything works as expected. I will have a look at the test you mentioned. |
|
@michael-wb I was thinking that as part of this PR I could also fix the TODOs that use the default implementation instead of the custom one from the config. |
Created #6169 to handle a fully implemented c_api test - you can add some unit testing now if you like, or we can fully vet this interface later. If not tested, add a comment that indicates this interface is a prototype only. |
I see you did that in 6171. |
| typedef enum status_error_code { | ||
| STATUS_OK = 0, | ||
| STATUS_UNKNOWN_ERROR = 1, | ||
| STATUS_RUNTIME_ERROR = 2, | ||
| STATUS_LOGIC_ERROR = 3, | ||
| STATUS_BROKEN_PROMISE = 4, | ||
| STATUS_OPERATION_ABORTED = 5, | ||
|
|
||
| /// WEBSOCKET ERRORS | ||
| // STATUS_WEBSOCKET_OK = 1000 IS NOT USED, JUST USE OK INSTEAD | ||
| STATUS_WEBSOCKET_GOING_AWAY = 1001, | ||
| STATUS_WEBSOCKET_PROTOCOL_ERROR = 1002, | ||
| STATUS_WEBSOCKET_UNSUPPORTED_DATA = 1003, | ||
| STATUS_WEBSOCKET_RESERVED = 1004, | ||
| STATUS_WEBSOCKET_NO_STATUS_RECEIVED = 1005, | ||
| STATUS_WEBSOCKET_ABNORMAL_CLOSURE = 1006, | ||
| STATUS_WEBSOCKET_INVALID_PAYLOAD_DATA = 1007, | ||
| STATUS_WEBSOCKET_POLICY_VIOLATION = 1008, | ||
| STATUS_WEBSOCKET_MESSAGE_TOO_BIG = 1009, | ||
| STATUS_WEBSOCKET_INAVALID_EXTENSION = 1010, | ||
| STATUS_WEBSOCKET_INTERNAL_SERVER_ERROR = 1011, | ||
| STATUS_WEBSOCKET_TLS_HANDSHAKE_FAILED = 1015, // USED BY DEFAULT WEBSOCKET | ||
| } status_error_code_e; |
There was a problem hiding this comment.
Either now or when the end to end test is created, we should probably move these to a separate file since this list is going to get quite long (esp. with the other error unification changes coming in). We will also need some asserts to verify the C_API status_error_code values match the ErrorCodes::Error values. See https://github.com/realm/realm-core/blob/master/src/realm/object-store/c_api/sync.cpp#L46 for an example.
There was a problem hiding this comment.
Good point. I will do it in RCORE-1359 since we need to add new errors anyway.
| void post(FunctionHandler&& handler) final | ||
| { | ||
| auto shared_handler = std::make_shared<FunctionHandler>(std::move(handler)); | ||
| m_post(m_userdata, new realm_sync_socket_callback_t(std::move(shared_handler))); |
There was a problem hiding this comment.
where does the pointer we allocate here get free'd? does the post function call realm_release() on it?
There was a problem hiding this comment.
realm_sync_socket_callback_complete should free the pointer. I will update it.
There was a problem hiding this comment.
I had a look at the pointers that are allocated by this api. realm_sync_socket_callback_t* is free'd by realm_sync_socket_callback_complete. realm_websocket_observer_t* is now free'd when the c api socket is free'd.
| { | ||
| auto shared_handler = std::make_shared<FunctionHandler>(std::move(handler)); | ||
| return std::make_unique<CAPITimer>(m_userdata, delay.count(), | ||
| new realm_sync_socket_callback_t(std::move(shared_handler)), |
There was a problem hiding this comment.
same question about the lifetime of this callback.
There was a problem hiding this comment.
Since this is a timer, it will eventually call post and realm_sync_socket_callback_complete, so the pointer will be free'd (as per above update)
What, How & Why?
Extend the C API so SDK's can provider their own platform specific implementation of websockets and event loop.
Fixes #5917.
☑️ ToDos
🚦 Tests (or not relevant)To be added in Create end to end test using the C_API and default websocket #6169