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

Bump actix-codec version, Reexport ConnectionIo #2436

Closed
wants to merge 3 commits into from

Conversation

brockelmore
Copy link

@brockelmore brockelmore commented Nov 5, 2021

PR Type

Other

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

During the refactor of moving actix_http::client over to awc, ConnectionIo was not reexported. Users need this trait for websocket usage. Additionally, updates the actix-codec version so websocket clients don't silently drop/not send continuation frames when the data is large (see actix/actix#431 and actix/actix-net#409)

@fakeshadow
Copy link
Contributor

fakeshadow commented Nov 5, 2021

Can you show me how would you need the ConnectionIo trait? It's never intended to be exported as it's just a trait alias for writing bound easier.

@brockelmore
Copy link
Author

brockelmore commented Nov 5, 2021

Ah - I see i should use BoxedSocket. Previously, I used it as so:

/// A wrapper around a raw Websocket writer
pub type WsWriterRaw = SplitSink<Framed<Box<dyn ConnectionIo>, awc::ws::Codec>, awc::ws::Message>;

/// A wrapper around a raw Websocket stream
pub type WsStreamRaw = SplitStream<Framed<Box<dyn ConnectionIo>, awc::ws::Codec>>;

// ...
fn() -> Result<(WsWriterRaw, WsStreamRaw), ()> {
    let (_response, framed) = client_clone
        .ws(url)
        .connect()
        .await
        .map_err(|e| {
            tracing::error!("Websocket connection error: {}", e);
        })?;

    let (sink, stream) = framed.split();
    Ok((sink, stream))
}

But that can be replaced by BoxSocket i believe? But when i originally wrote the above function, the compiler said to use ConnectionIo in older versions of awc & actix-http

@fakeshadow
Copy link
Contributor

fakeshadow commented Nov 5, 2021

yea I believe you can use BoxedSocket. It's a shortcut explicitly exported.
You can also write it in a generic way as Framed<BoxedSocket, awc::ws::Codec> can be described as S: Stream + Sink<Message> or impl Stream + Sink<Message> where Stream is used to receive and Sink is used to send message This way you don't have to import the concrete types and keep the code cleaner since the only type that is awc specific is Message, Frame and maybe ProtocolError.

@fakeshadow
Copy link
Contributor

As for the compiler error I believe we should do better to hide the internal traits and types to reduce confusion.

@brockelmore
Copy link
Author

gonna close this and allow someone else to bump codec versions whenever they want to release a new version

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

Successfully merging this pull request may close these issues.

None yet

2 participants