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

TransportReceiverT::receive should return an Option to indicate a dropped connection #1461

Open
AArnott opened this issue Sep 19, 2024 · 4 comments

Comments

@AArnott
Copy link

AArnott commented Sep 19, 2024

TransportReceiverT today declares this method:

async fn receive(&mut self) -> Result<ReceivedMessage, Self::Error>;

But this doesn't permit the implementation to indicate a dropped connection. Can we change it to the following:

async fn receive(&mut self) -> Result<Option<ReceivedMessage>, Self::Error>;

Then the caller should treat a None result the same way it would treat a closed transport.

This helps toward #907.

@niklasad1
Copy link
Member

I think it would be better to replace TransportSender/TransportReceiver abstraction around AsyncRead/AsyncWrite to support other transports etc. The current abstraction is not great but does the job for now

@AArnott
Copy link
Author

AArnott commented Sep 19, 2024

You've got me interested. How does the existing TransportReceiverT trait fall short? What would you replace it with?

@niklasad1
Copy link
Member

It's opinionated, contain optional close messages and ideally we would simplify to just sending and receiving messages without any extra stuff.

I think it will work fine with the change you propose but if I would re-write I would do it differently.

@AArnott
Copy link
Author

AArnott commented Sep 19, 2024

Interesting. I know that web sockets have a special close frame, but I never understood why. IMO all transports should be prepared for an unexpected close, which is what reading 0 bytes (or None) indicates. What value might a special close frame add?

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

No branches or pull requests

2 participants