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

Timeout not working when having packet losses #694

Closed
johan-bjareholt opened this issue Dec 21, 2023 · 1 comment
Closed

Timeout not working when having packet losses #694

johan-bjareholt opened this issue Dec 21, 2023 · 1 comment

Comments

@johan-bjareholt
Copy link
Contributor

I have a case where the timeout is not respected.

First, the examples

server

#!/usr/bin/env python3

from flask import request, Flask
from time import sleep

app = Flask(__name__)

@app.route('/get', methods=['GET'])
def get():
    #sleep(60)
    return "GET OK"

if __name__ == "__main__":
    app.run(host="0.0.0.0", port="1234")

client

use std::time::{Duration, Instant};

fn main() -> Result<(), ureq::Error> {
    let start = Instant::now();
    let resp = ureq::get("http://192.168.0.1:1234/get")
        .timeout(Duration::from_secs(5))
        .call();
    println!("request took: {}s", start.elapsed().as_secs());
    match resp {
        Ok(body) => {
            let body = body.into_string()?;
            println!("body: {}", body);
        }
        Err(err) => {
            println!("request failed: {}", err);
        }
    }
    Ok(())
}

Just running it like this works fine, we get the following output:

request took: 0s
body: GET OK

However, when adding insane synthetic packetloss on the server side like this, we get the following

$ sudo tc qdisc replace dev eth0 root netem loss 90%

request took: 30s
request failed: http://192.168.0.1:1234/get: Connection Failed: Connect error: connection timed out

Considering that the client has "timeout" set to 5s and the documentation for timeout states "Sets overall timeout for the request", I assume that this is a bug?

If i just uncomment the "sleep" in the server code without packet loss however, we do get the correct timeout of 5 seconds. So it seems like it only occurs when the network itself is slow, but not when the server is slow at handling the request.

request took: 5s
request failed: http://127.0.0.1:1234/get: Network Error: Network Error: Error encountered in the status line: timed out reading response
@johan-bjareholt
Copy link
Contributor Author

Okay, so the "issue" is that there is a default set to 30s for timeout_connect on the Agent, which is applied for the Request as well.
When you look at the documentation for the AgentBuilder, it is very clear exactly how this works

This takes precedence over .timeout_read() and .timeout_write(), but not .timeout_connect().

I'd consider this just to be hard to read docs, it's not really a bug. It was my fault for not reading the last part of the sentence in the documentation that i cited before

Sets overall timeout for the request, overriding agent’s configuration if any.

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

No branches or pull requests

1 participant