Skip to content
This repository has been archived by the owner on Feb 14, 2023. It is now read-only.

Hyper client vs lazy Hyper client issue #82

Closed
sb89 opened this issue May 29, 2017 · 11 comments
Closed

Hyper client vs lazy Hyper client issue #82

sb89 opened this issue May 29, 2017 · 11 comments

Comments

@sb89
Copy link

sb89 commented May 29, 2017

I have the following 'lazy' code:

let ssl = NativeTlsClient::new().map_err(|e| ::hyper::Error::Ssl(Box::new(e)))?;
let connector = HttpsConnector::new(ssl);
let client = Client::with_connector(connector);

let mut res = Multipart::new().add_text("chat_id", "220614325")
   .add_file("document", "/home/steven/hello.jpg")
   .client_request(&client, &url)?;

println!("{:?}", &res);

which returns a 400 error.

And the following 'non-lazy' code:


let ssl = NativeTlsClient::new().map_err(|e| ::hyper::Error::Ssl(Box::new(e)))?;
let connector = HttpsConnector::new(ssl);

let request = hyper::client::Request::with_connector(Method::Post,
                                                             Url::parse(&url).unwrap(),
                                                             &connector)?;

let mut multipart = Multipart::from_request(request)?;

multipart.write_text("chat_id", "220614325")?;
multipart.write_file("document", "/home/steven/hello.jpg")?;

let mut res = multipart.send()?;

println!("{:?}", &res);

which works fine.

So what am I doing wrong with the 'lazy' version?

@drahnr
Copy link

drahnr commented May 29, 2017

can you also add the output? not sure if the lazy also populates ContentLength header which some web APIs request

@sb89
Copy link
Author

sb89 commented May 29, 2017

Response { status: BadRequest, headers: Headers { Server: nginx/1.10.0
, Date: Mon, 29 May 2017 13:01:40 GMT
, Content-Type: application/json
, Content-Length: 94
, Connection: keep-alive
, Access-Control-Allow-Origin: *
, Access-Control-Expose-Headers: Content-Length,Content-Type,Date,Server,Connection
, }, version: Http11, url: "https://api.telegram.org/..../sendDocument", status_raw: RawStatus(400, "Bad Request"), message: Http11Message { is_proxied: false, method: None, stream: Wrapper { obj: Some(Reading(SizedReader(remaining=94))) } } }

The telegram JSON error is

"Bad Request: there is no document in the request"

@sb89
Copy link
Author

sb89 commented May 29, 2017

I resized my image to be 1px x 1px and the 'lazy' method worked, could it be that the lazy method isn't handling larger files correctly?

Perhaps the following line?

const DEFAULT_BUFFER_THRESHOLD: u64 = 8 * 1024;

@drahnr
Copy link

drahnr commented May 29, 2017

This could be the same issue as #81

@sb89
Copy link
Author

sb89 commented May 29, 2017

The buffer threshold is definitely causing this.

@abonander
Copy link
Owner

abonander commented May 30, 2017

Admittedly, that codepath has not been very well tested. I'm open to suggestions, otherwise I'll try to see if I can reproduce the problem at least. I imagine PreparedField::Partial() isn't getting written out completely. I'll probably end up changing the logic here as I think I want the prepare-threshold to be the maximum amount of bytes buffered overall, not per field.

@sb89
Copy link
Author

sb89 commented May 30, 2017

I am not aware of the purpose of the buffer threshold (I haven't looked at it in any great detail) but if that is to remain, does it need to be configurable when sending a request?

To get around this issue for the moment, I have created a fork that allows for a threshold value to be configured when sending a request.

@abonander
Copy link
Owner

abonander commented May 30, 2017

You can use prepare_threshold() which changes how much gets buffered up-front. Having some amount of buffering up-front seemed like a good optimization at the time but now I don't know. Getting rid of it will probably fix this bug, though.

@sb89
Copy link
Author

sb89 commented May 30, 2017

Yes, my change is to use prepare_threshold() in client_request_mut() instead of prepare(), as that just uses the default threshold which isn't enough (for my use anyway).

@abonander
Copy link
Owner

If the code was working correctly, the size of the buffer shouldn't matter.

@abonander
Copy link
Owner

Publishing 0.13 which should have this bug fixed.

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

No branches or pull requests

3 participants