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

Could crates.io require updating with the master branch on GitHub? #58

Closed
DouglasGray opened this issue Apr 25, 2019 · 3 comments
Closed
Assignees
Labels

Comments

@DouglasGray
Copy link

DouglasGray commented Apr 25, 2019

Hello, I'm new to Rust so I'm probably just a bit confused but I was looking through the docs for what appears to be the latest version of tokio-tungstenite on crates.io (v0.6.0), specifically the source for WebSocketStream in lib.rs (https://docs.rs/tokio-tungstenite/0.6.0/src/tokio_tungstenite/lib.rs.html#162-164), and it appears that the code for WebSocketStream is a fair bit different to that on the GitHub master branch. For example, comparing the struct and the poll function in 'impl Stream for WebSocketStream':

(crates.io source)

pub struct WebSocketStream<S> {
    inner: WebSocket<S>,
}

(GitHub source)

pub struct WebSocketStream<S> {
    inner: WebSocket<S>,
    stream_ended: bool,
}

(crates.io source)

fn poll(&mut self) -> Poll<Option<Message>, WsError> {
        self.inner.read_message().map(|m| Some(m)).to_async()
}

(GitHub source)

fn poll(&mut self) -> Poll<Option<Message>, WsError> {
        if self.stream_ended {
            self.stream_ended = false;
            return Ok(Async::Ready(None))
        }

        self.inner.read_message().map(|m| {
            if m.is_close() {
                self.stream_ended = true;
            }
            Some(m)
        }).to_async()
}

At least for me I can confirm that when I add 'tokio-tungstenite = "*"' to my Cargo.toml it uses the code that I can see on crates.io. Is this the case for anyone else?

(Edit: changed format for code blocks)

@DouglasGray
Copy link
Author

Of course I understand that the master branch and what's officially out there can be out of sync, but it seems that the change to signal the end of the stream is a fairly important one? Unless I'm missing something in the source that I see on crates.io (which is very likely)

@daniel-abramov
Copy link
Member

Hi, yes, the master branch is the current (active) state, the latest versions of tokio-tungstenite and tungstenite has not been published to crates.io yet.

At least for me I can confirm that when I add 'tokio-tungstenite = "*"' to my Cargo.toml it uses the code that I can see on crates.io. Is this the case for anyone else?

Yes. The version from master branch has not yet been published. The changes you're referring to have been made to implement some improvements which were suggested to us, namely we changed the semantics for the Stream implementation so that it corresponds to the "expected" behavior of Tokio, i.e. a normal close will not result in an error on Stream anymore, but return a None. And the Message enum has been extended by an additional Close message, so you still have the possibility to analyze the close code in a newest version (master branch) and your Stream will return None when the connection is closed in a normal way (and not an Err like it did before).

Some people already mentioned in multiple comments that we have to publish tungstenite-rs and tokio-tungstenite to crates.io, I pinged @agalakhov to do that. Perhaps he has forgotten to do that.

@DouglasGray
Copy link
Author

Ok gotcha, thank you for explaining!

gregates pushed a commit to gregates/tokio-tungstenite that referenced this issue Feb 24, 2024
extract the context values from `WebSocket`
This issue was closed.
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