Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove send-error message #149

Open
lgrahl opened this issue Jul 10, 2018 · 4 comments
Open

Remove send-error message #149

lgrahl opened this issue Jul 10, 2018 · 4 comments
Labels

Comments

@lgrahl
Copy link
Member

lgrahl commented Jul 10, 2018

The send-error message is not really helpful since it cannot reliably tell whether a message has been relayed. See this comment for a detailed explanation.

I propose to remove the send-error message. We could add a new field to the disconnected message that indicates whether the client disconnected (or has been disconnected by the server) gracefully with a close code or the connection has been lost (either with a ping timeout or a TCP timeout/error). But even that may be prone to error and the value is questionable.

@lgrahl lgrahl added the bug label Jul 10, 2018
@lgrahl lgrahl modified the milestone: Protocol Polishing Jul 10, 2018
@ovalseven8
Copy link
Contributor

ovalseven8 commented Oct 21, 2019

May I ask what's the current state on this? What's currently happening in such a case you mentioned in this comment?

On the Websocket layer, the only solution is to send back a "acknowledge" message. Since SaltyRTC is based on websockets, it's up to the applications to ack messages. So, removing send-error makes sense, I would note in the docs though that websockets don't work like TCP in this case.

Like you said, I do not see much value to add a new field to the disconnected message. In the end, it does not matter and the client cannot really conclude if a message was received or not.

@lgrahl
Copy link
Member Author

lgrahl commented Oct 22, 2019

It's currently implemented but implementations should not rely on it. We still need to remove it.

@ovalseven8
Copy link
Contributor

It's currently implemented but implementations should not rely on it. We still need to remove it.

You mean applications who use SaltyRTC should not rely on send-error, but implement their own ack protocol if needed?

@lgrahl
Copy link
Member Author

lgrahl commented Oct 22, 2019

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants