hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.47k stars 1.6k forks source link

Code block for "Reading a Request Body" #1137

Closed serayuzgur closed 7 years ago

serayuzgur commented 7 years ago

Hi, If you can provide a simple code block, I can document it with an example project.

hjr3 commented 7 years ago

Looking for something like this?

req.body().fold(Vec::new(), |mut v, chunk| {
    v.extend(&chunk[..]);
    future::ok::<_, hyper::Error>(v)
}).and_then(move |chunks| {
    let body = String::from_utf8(chunks).unwrap();
    future::ok(body)
})

That will return a Future<Item=String, Error=hyper::Error>. You can wait() on that future if you want to block until the body is completely read into the String.

tesaguri commented 7 years ago

Or you can write like this (since 78512bdb184903061ea02f1101c99a097483cb69):

use hyper::client::Response;
use std::str;

let future_body = future_res.map(Response::body).flatten_stream().concat();
let body = future_body.wait().unwrap(); // : hyper::Chunk
let as_str = str::from_utf8(&body).unwrap();
serayuzgur commented 7 years ago

Hi, thanks for quick response. wait() blocks the operation for eternity. It can never complete reading req.body. I lived something like that before when I was working with tokio-proto. If a Decoder returns Ok(Some(...)It feels like it never finishes. I passed that with checking Content-length and returning Ok(None) if the size matches. So what I tried is something like that. It again stuck with wait(). What do you think?

let chunks = req.body().fold(Vec::new(), |mut v, chunk| {
    v.extend(&chunk[..]);
    futures::future::ok::<_, hyper::Error>(v)
}).wait().unwrap();
let mut payload_mut = deserialize(chunks.as_slice());
match db.insert(table,payload_mut) {
    Some(record) => {db.flush(); return self.success(response, &record)}
    None => return self.error(response, StatusCode::Conflict),
 }
seanmonstar commented 7 years ago

Definitely do not use wait() in most circumstances. If you have to ask if it's a good idea, then it isn't. It has very specific cases where it's useful.

Instead, you should just return the future chain.

req.body().concat().and_then(|body| {
    let payload = deserialize(&body);
    // you should make `db.insert` async and return a future too
    db.insert(table, payload).then(|result| {
        match result {
            // ...
        }
    })
})
serayuzgur commented 7 years ago

Hi @seanmonstar , You closed the issue so I believe, you are sure what you are saying but forgive me I have to ask. Collecting all chunks may block the current thread. I am sure it is not about db because I commented it out and tried. When I take a look at tokio's samples I realized threat_pool usage and implemented it (self.thread_pool.spawn_fn). This resolved all my problems. Can you please take a look at this code. Is this could be the solution? It seems waiting for the stream blocks the polling (or reading?) and system never executes one more line.

seanmonstar commented 7 years ago

Collecting the chunks into 1 doesn't block the thread, only the use of .wait() on the end of it does. Otherwise, it just returns a Future, which you can then chain on. Absolutely remove the .wait(). The definition of .wait is: block this thread until the future is ready. That is certainly the wrong behavior, since that exact same thread is the one that would be reading the data from the socket in an event loop.

The database operations wouldn't necessarily deadlock the server, but it would hurt its performance, as you're doing likely synchronous (network and/or file) IO, which defeats the purpose of using asynchronous IO with your HTTP server.

Using a thread pool may resolve those issues, but you'd have better success using asynchronous database operations as well.

serayuzgur commented 7 years ago

Thank for the detailed explanation. Since I promised a documentation about it, I will fix my usage and create an example about it. Thanks.

seanmonstar commented 7 years ago

Documentation is almost ready to publish that includes examples on how to read the body.

shesek commented 5 years ago

Documentation is almost ready to publish that includes examples on how to read the body.

Did that happen?

lf94 commented 5 years ago

I second this. Hyper has actually been really nice to use on its own. I'm currently stuck with this error:

cannot move out of data in a '&' reference: cannot move

...with nearly anything, such as request.body().collect(), request.body().concat2() and so on.

I know why but I'm not sure how to fix it.


For anyone reaching here by Google:

      request.into_body().collect().and_then(|chunks| {
        let file_name = Path::new(original_path).file_name().unwrap();
        let mut file_handle = File::create(file_name).unwrap();
        let bytes: Vec<u8> = chunks.into_iter().flat_map(|chunk| chunk.into_bytes()).collect();
        file_handle.write_all(&bytes).unwrap();
        Ok::<_, _>(())
      });

into_body() is the key here. It consumes the body, returning Body. body() returns just a reference, &Body, which causes all sorts of problems.

david415 commented 5 years ago

What's the conclusion of this github issue? Where can I find the code solution to "i want my http server to read the entire request body" ?

jedisct1 commented 5 years ago

Calling into_parts() or into_body() returns something that resembles a Future iterator, that can be used to wait for body chunks:

let chunk = body.next().await.unwrap().unwrap();

Concatenating all these chunks reconstructs the entire request body.

Now, if you want both to read the body chunks and return a streamed response simultaneously, it's certainly possible, but good luck with that.