Proposal: Remove send and receive timeouts

[ueman]

This PR removes send and receive timeouts. It's meant as a basis for a discussion.

As you can see, the current implementation just adds send and receive timeouts itself. It's not a concept of the underlying APIs (xhr and HttpClient). The Http2 adapter doesn't support it at all.

Removing these timeouts could lead to a pretty nice amount of reduced complexity and code.

[kuhnroyal]

I agree with this, what do you think @wendux?

[jgoyvaerts]

I don't think this is a good idea, a lot of users of dio depend on this implementation and would have to reimplement this themselves. We can make them optional and improve them if they work incorrectly, but removing them seems like a bad idea

[ueman]

I don't think this is a good idea, a lot of users of dio depend on this implementation and would have to reimplement this themselves.

Do you have any data on that?

Also since you're against this, I'd really like to hear some use cases for these two timeouts.

[jgoyvaerts]

I don't think this is a good idea, a lot of users of dio depend on this implementation and would have to reimplement this themselves.

Do you have any data on that?

Also since you're against this, I'd really like to hear some use cases for these two timeouts.

I don't have any data on that, since it would be hard/impossible to get that data.

I assume (this is my main use case anyway) people use dio to consume their backend API in their app, so they only have to configure stuff once like auth tokens, custom headers on each request, etc.

Removing these timeouts would mean you either fall back to the OS default timeout (which can vary per platform, and can be longer than you want), or means users of this library have to implement their own wrapper if they want their requests to timeout after X seconds.

[ueman]

Removing these timeouts would mean you either fall back to the OS default timeout (which can vary per platform, and can be longer than you want), or means users of this library have to implement their own wrapper if they want their requests to timeout after X seconds.

The connectionTimeout does not get removed, only the send and receive timeouts gets removed if this code is merged.

[jgoyvaerts]

My bad, misunderstood then.

Still seems weird to remove the feature rather than fixing/implementing it properly. Once it's removed, a user of the library would have no way of implementing this himself because it can only be implemented on request level, which Dio is hiding, correct?

[kuhnroyal]

Well, the concept of receive/send timeout don't technically exist in the underlying clients.
They just start a timer and throw an error once the timer completes. The request doesn't get canceled and may still run and successfully complete but you wouldn't know.

Would really love to hear @wendux thoughts. What was the motivation for adding this?

[amondnet]

• https://api.dart.dev/stable/2.16.1/dart-io/HttpClientRequest/abort.html
• https://api.dart.dev/stable/2.16.1/dart-html/HttpRequest/abort.html

Doesn't abort cancel the request?

I think once we have a response, instead of request.abort() we need to use response.detachSocket().

https://pub.dev/documentation/http/latest/io_client/IOStreamedResponse/detachSocket.html

There is a PR that support for request timeout.
dart-lang/http#521

I think this is worth doing because timeouts are a common case for wanting to abort requests, and they are easier to get wrong than to get right. The naive approach of .timeout doesn't clean up resources and the entire result is still going to get read. Even with a general mechanism to abort requests a timeout argument is handy because we can handle the timer cancellation which would otherwise be boilerplate. We also likely can't arrive at a solid design for request aborting in general, but the API design with timeout: argument is fairly straightforward.

[ueman]

• https://api.dart.dev/stable/2.16.1/dart-io/HttpClientRequest/abort.html
• https://api.dart.dev/stable/2.16.1/dart-html/HttpRequest/abort.html

Doesn't abort cancel the request?

I believe DIO doesn't yet make use of it.

[amondnet]

@ueman abort is used.

[ueman]

Oh, thanks for clarifying 👍

[wendux]

Thanks this PR, but I prefer to agree with @amondnet "timeouts are a common case for wanting to abort requests".

[ueman]

Thanks for your feedback. I'm closing this as there seems to be some use case for this.