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

Update example echo - ready to merge #1855

Merged
merged 3 commits into from
Jul 12, 2019

Conversation

liufuyang
Copy link
Contributor

I tried to update this as well but not sure how to deal with that req.into_body().map(....

Perhaps you have some example to how me how to update this example?

examples/echo.rs Outdated Show resolved Hide resolved
examples/echo.rs Outdated Show resolved Hide resolved
@liufuyang liufuyang force-pushed the update-example-echo branch from 5fddaa2 to a392686 Compare July 10, 2019 21:27
examples/echo.rs Outdated
@@ -56,7 +48,7 @@ fn echo(req: Request<Body>) -> BoxFut {
response
});

return Box::new(reversed);
return Ok(reversed);
Copy link
Contributor Author

@liufuyang liufuyang Jul 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how to deal with concat2 above, it also seems to be a function that is missing

"no method named concat2 found for type hyper::Body in the current scope"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like that was renamed to TryStreamExt::try_concat.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm.. I tried to go with it, but not really sure how :P
Got some errors like this with my last commit c04d92f

error[E0277]: `(dyn hyper::rt::Stream<Item = std::result::Result<hyper::Chunk, std::boxed::Box<(dyn std::error::Error + std::marker::Send + std::marker::Sync + 'static)>>> + std::marker::Send + 'static)` cannot be shared between threads safely
  --> examples/echo.rs:78:10
   |
78 |         .serve(service);
   |          ^^^^^ `(dyn hyper::rt::Stream<Item = std::result::Result<hyper::Chunk, std::boxed::Box<(dyn std::error::Error + std::marker::Send + std::marker::Sync + 'static)>>> + std::marker::Send + 'static)` cannot be shared between threads safely
   |

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I checked out the branch, and finally understand the giant error messages. They're very confusing, but ultimately, since there was an await inside the match req.uri(), the await was forcing one of the state machine steps to own a borrow of &Request. Because of that, it was noticing the &Request is not Sync, thanks to the wrapped stream variant inside Body.

This will be very easy for users to bump into, and so probably I'll need to fix Body to require Sync...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, here's the issue in the rust-lang repo: rust-lang/rust#57017

Copy link
Contributor Author

@liufuyang liufuyang Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that sounds complicated. Not sure how much I could really understand but it's nice that you helping to solve it out :)

I rebased onto master, now I seem to have this issue:

error[E0599]: no method named `into_future` found for type `tokio_sync::mpsc::bounded::Receiver<_>` in the current scope
   --> src/client/connect/dns.rs:134:14
    |
134 |             .into_future()
    |              ^^^^^^^^^^^
    |
    = note: the method `into_future` exists but the following trait bounds were not satisfied:
            `&mut tokio_sync::mpsc::bounded::Receiver<_> : futures_util::stream::StreamExt`
            `&tokio_sync::mpsc::bounded::Receiver<_> : futures_util::stream::StreamExt`
            `tokio_sync::mpsc::bounded::Receiver<_> : futures_util::stream::StreamExt`

error: aborting due to previous error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may need to run cargo update to get the latest changes to tokio.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... did that then I got this 😄

error[E0277]: `dyn std::error::Error` cannot be sent between threads safely
  --> examples/echo.rs:67:1
   |
67 | #[tokio::main]
   | ^^^^^^^^^^^^^^ `dyn std::error::Error` cannot be sent between threads safely
   |
   = help: the trait `std::marker::Send` is not implemented for `dyn std::error::Error`
   = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<dyn std::error::Error>`
   = note: required because it appears within the type `std::boxed::Box<dyn std::error::Error>`
   = note: required because it appears within the type `std::result::Result<(), std::boxed::Box<dyn std::error::Error>>`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh cool, looks like I just need to add
async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> on the main. Nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like it is working now, let me tide up the PR a bit. Thanks for the help 😄

@liufuyang liufuyang force-pushed the update-example-echo branch from c04d92f to f6bc0cc Compare July 11, 2019 19:12
@liufuyang liufuyang changed the title Try update example echo - WIP - Not working Try update example echo Jul 11, 2019
@liufuyang liufuyang changed the title Try update example echo Update example echo - ready to merge Jul 12, 2019
@liufuyang liufuyang force-pushed the update-example-echo branch from 1121f5e to b841ef6 Compare July 12, 2019 07:41
@liufuyang
Copy link
Contributor Author

@seanmonstar Hey Sean, I have made this example work and cleanup the code a bit. Would you like take a look at it again? :)

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for continuing despite the difficulties!

@seanmonstar seanmonstar merged commit 7ff8fcc into hyperium:master Jul 12, 2019
@liufuyang liufuyang deleted the update-example-echo branch July 13, 2019 08:04
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.

2 participants