fix: websocket outbound transport #1788
Conversation
Signed-off-by: Martin Auer <[email protected]>
| // We can close the socket as it shouldn't return messages anymore | ||
| // make sure to use the socket in a manner that is compliant with the https://developer.mozilla.org/en-US/docs/Web/API/WebSocket | ||
| // (React Native) and https://github.com/websockets/ws (NodeJs) | ||
| socket.send(Buffer.from(JSON.stringify(payload))) |
There was a problem hiding this comment.
We technically could send just a string instead of a buffer with both these interfaces, but it should not matter that much.
There was a problem hiding this comment.
Is the method sync in RN? And if we don't provide a callback, is it sync in NodeJS as well?
There was a problem hiding this comment.
Haven't checked it, but there is likely an overload to not provide a callback as you don't have to handle the res/err if you don't want. But yeah it would be good to do a test run with this in both environments.
There was a problem hiding this comment.
I think this should be fine. It's also the behavior that was there up until 2-3 weeks ago.
|
Oh that API difference is unfortunate! This error handling was added recently to improve the test suites. Not sure if removing it will affect a lot (because there were other improvements done by @TimoGlastra at the same time), but if it does we can create a That being said, if WebSocket API differs in each platform, probably we should look at making a dedicated implementation for each (like we do for inbound transports). The good thing about such idea is that the infamous |
Signed-off-by: Martin Auer <[email protected]>
The previous approach was only possible in Node.
The websocket implementation of RN follows this specification. https://developer.mozilla.org/en-US/docs/Web/API/WebSocket.
The Promise was therefore never resolved as the callback does not exist.