Sdk: Issues with dart:io HttpClient reusing connections

[DartBot]

_This issue was originally filed by [e-mail]_

---

What steps will reproduce the problem?

import 'package:http/http.dart' as http;

main() {
  String url = "http://is.muni.cz";
  http.Client httpClient = new http.Client();
  httpClient.get(url).then((http.Response r) {
    print(r.statusCode);
    httpClient.get(url).then((http.Response r) {
        print(r.statusCode);
    });
  });
}

What is the expected output? What do you see instead?

Observatory listening on http://127.0.0.1:43785
200
Unhandled exception:
Uncaught Error: Connection closed before full header was received
Stack Trace:

­0 IOClient.send.<anonymous closure> (package:http/src/io_client.dart:78:7)

­1 _RootZone.runUnary (dart:async/zone.dart:1155)

­2 _Future._propagateToListeners.handleError (dart:async/future_impl.dart:515)

­3 _Future._propagateToListeners (dart:async/future_impl.dart:570)

­4 _Future._completeError (dart:async/future_impl.dart:366)

­5 Future.wait.handleError (dart:async/future.dart:263)

­6 _RootZone.runBinary (dart:async/zone.dart:1160)

­7 _Future._propagateToListeners.handleError (dart:async/future_impl.dart:511)

­8 _Future._propagateToListeners (dart:async/future_impl.dart:570)

­9 _Future._completeError (dart:async/future_impl.dart:366)

­10 _Future._asyncCompleteError.<anonymous closure> (dart:async/future_impl.dart:421)

­11 _asyncRunCallbackLoop (dart:async/schedule_microtask.dart:41)

­12 _asyncRunCallback (dart:async/schedule_microtask.dart:48)

­13 _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:84)

­14 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:131)

­0 _rootHandleUncaughtError.<anonymous closure> (dart:async/zone.dart:886)

­1 _asyncRunCallbackLoop (dart:async/schedule_microtask.dart:41)

­2 _asyncRunCallback (dart:async/schedule_microtask.dart:48)

­3 _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:84)

­4 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:131)

What version of the product are you using?

Dart Editor version 1.9.0.dev_00_00 (DEV)
Dart SDK version 1.9.0-dev.0.0

On what operating system?

Linux arch 3.17.4-1-ARCH #­1 SMP PREEMPT Fri Nov 21 21:14:42 CET 2014 x86_64 GNU/Linux

Please provide any additional information below.

The testcase works with some urls and fails with others. For example, "http://www.muni.cz" does work without crashing.

It could have something to do with Keep-Alive.

[lrhn]

cc @skabet.
_Added Pkg-Http, Area-Pkg, Triaged labels._

[sgjesse]

This is a more general issue. The following dart:io code reproduces this:

import 'dart:async';
import 'dart:io';

main() {
  Url url = Uri.parse("http://is.muni.cz");
  var httpClient = new HttpClient();
  httpClient.getUrl(url).then((request1) {
    return request1.close();
  }).then((response1) {
    print(response1.statusCode);
    response1.drain().then((_) {
      scheduleMicrotask(() {
        httpClient.getUrl(url).then((request2) {
          return request2.close();
        }).then((response2) {
          print(response2.statusCode);
          response2.drain().whenComplete(() {
            print("Done");
            httpClient.close();
          });
        });
      });
    });
  });
}

What happens is that the server closes the connection after sending the body. However in dart:io the close event is not processed until the connection is being reused for the next request.

Before going into the details below, the bottom line is that this is a general issue with the server closing the connection when dart:io thinks that it is ready for another request. In this case it reproduces, but it could also happen sporadic when the client re-uses a connection just as the server decides to close it due to timeout, connection pressure etc.

There is not really good solution here. The general case is unavoidable, but there are a number of options:

  * Better error message (making it simpler to decide whether to retry
  * Automatically retry GET request
  * Quarantine connections added to connection pool for e.g. 100ms before reusing

The first we should just do. The second might make sense to do. The third seems wrong, and will cause multiple connections for the case where keep-alive is actually working.

Looking at IP traces in this particular case reveals that the FIN is a separate package and not piggy-bagged on the last data package.

The headers from the server does not indicate that the connection will not be keep-alive, these are the headers:

GET / HTTP/1.1
user-agent: Dart/1.9 (dart:io)
accept-encoding: gzip
content-length: 0
host: is.muni.cz

HTTP/1.1 200 OK
Date: Mon, 08 Dec 2014 14:52:56 GMT
Server: Apache
Cache-Control: no-cache
Set-Cookie: issession=zuiA8v7MZiIYL2vcg-PutSVv; path=/; expires=Tue, 08-Dec-2015 14:52:56 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 7189
Content-Type: text/html; charset=utf-8

I tried to explicitly add the 'Connection: Keep-Alive' header. It caused change if reply headers, but still the server closed after sending the response.

GET / HTTP/1.1
user-agent: Dart/1.9 (dart:io)
connection: Keep-Alive
accept-encoding: gzip
content-length: 0
host: is.muni.cz

HTTP/1.1 200 OK
Date: Mon, 08 Dec 2014 15:27:38 GMT
Server: Apache
Cache-Control: no-cache
Set-Cookie: issession=eIZBWqQl2V5708uUjyRP1vSB; path=/; expires=Tue, 08-Dec-2015 15:27:38 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 7216
Keep-Alive: timeout=2, max=100
Connection: Keep-Alive
Content-Type: text/html; charset=utf-8

The site http://www.muni.cz works, but that is a different server (Microsoft IIS 7.5), and it keeps the connection open for the second request.

GET / HTTP/1.1
user-agent: Dart/1.9 (dart:io)
accept-encoding: gzip
content-length: 0
host: www.muni.cz

HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Type: text/html; charset=utf-8
Expires: -1
Server: Microsoft-IIS/7.5
X-AspNet-Version: 4.0.30319
Set-Cookie: w3mu.global=cid=e1ba8263-a4b5-4dfa-a0b7-7428947e7e85-2e94fe1f&leftMenu-=; expires=Thu, 08-Dec-2016 14:07:12 GMT; path=/
Set-Cookie: w3mu.prev-page=instance=MU$~$/$~$e1ba8263-a4b5-4dfa-a0b7-7428947e7e85-2e94fe1f$~$$pageId=MU$$~$fixed$~$en$~$normal; path=/
X-Powered-By: ASP.NET
Date: Mon, 08 Dec 2014 14:07:11 GMT
Content-Length: 22323

---

cc @nex3.
_Set owner to @sgjesse._
_Added this to the 1.9 milestone._
_Removed Pkg-Http, Area-Pkg labels._
_Added Area-Library, Library-IO, Accepted labels._

[kevmoo]

_Changed the title to: "Issues with dart:io HttpClient reusing connections"._

[sgjesse]

Punting this to 1.10.

---

_Removed this from the 1.9 milestone._
_Added this to the 1.10 milestone._

[sgjesse]

There is an intermediate fix for parts of this issue in r45127.

This will handle the case where the connection close is received when the new request is made, but was not processed by the event loop.

A more general mechanism where at least GET requests (considered idempotent) are retried in a more general manner. Possible by forcing a new connection and not a connection from the pool if there is a failure.

[kevmoo]

Soren: will we get to this in 1.10 or should we push to 1.11?

[sgjesse]

The fix from r45127 (comment #­6) was reverted in r45129, and have been re-applied in r45313 (with a fix to a broken test).

This will be merged to 1.10.

Keeping the bug open for 1.11.

[sgjesse]

_Removed this from the 1.10 milestone._
_Added this to the 1.11 milestone._

[mit-mit]

Removing milestone, this is not scheduled for a release.

[slightfoot]

Been receiving this a lot recently especially with NetworkImage provider in Flutter using a Amazon S3 bucket. I feel this might be connected to connection keep-alive.

[winwisely99]

this is killing us too.
we pull from S§ and also minio s3

[kevmoo]

@a-siva @dgrove @zanderso – could we get some love on this?

[mraleph]

Note that there is no universal solution to this problem. Users of the HttpClient are expected to write their own retry logic.

See this breaking change announcement for an example of how to do that.

[a-siva]

One of the later comments from @slightfoot seems to be around using NetworkImage provider from Flutter and not HttpClient directly, maybe the NetworkImage provider code in Flutter needs to add the retry logic.

[slightfoot]

When a underlying connection has chosen to be reused by the HttpClient and the socket is closed unexpectedly before the request is sent then surely this is a fault in the library choosing a connection, rather than a data transit error.

It seems a bit ridiculous that every implementation of the library will need to implement connect retries. I understand that the library cannot do this without a parameter from the developer. But connection retries should be baked into the HttpClient. Otherwise we will see every developer that uses the HttpClient implement their own retry/backoff logic.

Every Flutter app is going to require it. Perhaps its time to consider adding this as a feature of the HttpClient.

cc @a-siva @dgrove @zanderso

[cfchris]

Been receiving this a lot recently especially with NetworkImage provider in Flutter using a Amazon S3 bucket. I feel this might be connected to connection keep-alive.

@slightfoot @winwisely99, if either of you find a solution, please post it.

We are building a companion application to our web-based CRM. One of the features is sms/mms. Because the information might be private, we are serving the images behind an authenticated API that redirects (302) to an S3 short term URL (5 minutes).

Anecdotally, I can say NetworkImage seems fine on conversations that only have a few images (like 2 or 3). On a conversation thread that I am using that has 9 images, frequently one or two of them will have this issue immediately. (I will get the error Connection closed before full header was received, uri = ... in the "debug console" and those images will not load.

(A coworker pointed out that if you copy the address from the error in the debug console and paste it in a browser, it loads and is valid.)

It's going to be very embarassing if randomly, sometimes, some of the images won't show up. :-(

[cfchris]

I finally stumbled on a StackOverflow that mentioned fixing this with cached_network_image package. I tried it out and it did fix the issue. It also had the added benefit of making the screens faster on subsequent views (because images are cached locally now).

So, although the issue still remains for others, I'm happy that I was forced to find CachedNetworkImage().

It was a very easy drop-in replacement for NetworkImage, here are the two places I had to change (the second being only a tiny bit more complex).

// BEFORE:
          child: ClipRRect(
            borderRadius: borderRadius,
            child: Image.network(imageUrl),
          ),
// AFTER:
          child: ClipRRect(
            borderRadius: borderRadius,
            child: CachedNetworkImage(
              imageUrl: imageUrl,
              errorWidget: (context, url, error) => Icon(Icons.error),
            ),

// BEFORE:
            Expanded(
              child: PhotoView(
                imageProvider: NetworkImage(images[currentIndex].imageUrl),
              ),
            ),
// AFTER
            Expanded(
              child: CachedNetworkImage(
                imageUrl: images[currentIndex].imageUrl,
                imageBuilder: (context, imageProvider) =>
                    PhotoView(imageProvider: imageProvider),
              ),
            ),

[cfchris]

Been receiving this a lot recently especially with NetworkImage provider in Flutter using a Amazon S3 bucket. I feel this might be connected to connection keep-alive.

@slightfoot Please see my above comment about cached_network_image.

[yogurtpopss]

Note that there is no universal solution to this problem. Users of the HttpClient are expected to write their own retry logic.

See this breaking change announcement for an example of how to do that.

Can you give example for this retry logic? i tried adding retry using https://pub.dev/packages/retry in http.get function but it doesnt get rid the error _Connection closed before full header was received_.

I suffer this error when I use image library, when i show a list of hundred of image from server, bunch of errors will show up before the device will crash with message lost connection to device. This make me wonder if flutter not ready for marketplace type of application. Please somebody prove this wrong :(

[mraleph]

@harasadina there is a example in the breaking change I have linked to:

Future<List<String>> getUrlWithRetry(HttpClient httpClient, Uri url, {int maxRetries = 5}) async {
  for (var attempt = 0; attempt < maxRetries; attempt++) {
    try {
      final request = await httpClient.openUrl('GET', url);
      final response = await request.close();
      return await response
          .transform(new Utf8Decoder(allowMalformed: true)).toList();
    } catch (e) {
      // ...
    }
  }
}

You might want to be more nuanced and only retry on certain exceptions.

i tried adding retry using https://pub.dev/packages/retry in http.get function but it doesnt get rid the error Connection closed before full header was received.

Can you post your code and the stack trace you are getting?

It can be that we fail to properly propagate error from connection to the Future returned from get somewhere. Which is something that we should certainly fix.

[yogurtpopss]

@harasadina there is a example in the breaking change I have linked to:

Future<List<String>> getUrlWithRetry(HttpClient httpClient, Uri url, {int maxRetries = 5}) async {
  for (var attempt = 0; attempt < maxRetries; attempt++) {
    try {
      final request = await httpClient.openUrl('GET', url);
      final response = await request.close();
      return await response
          .transform(new Utf8Decoder(allowMalformed: true)).toList();
    } catch (e) {
      // ...
    }
  }
}

You might want to be more nuanced and only retry on certain exceptions.

i tried adding retry using https://pub.dev/packages/retry in http.get function but it doesnt get rid the error Connection closed before full header was received.

Can you post your code and the stack trace you are getting?

It can be that we fail to properly propagate error from connection to the Future returned from get somewhere. Which is something that we should certainly fix.

I just tried to edit the _network_image_io.dart by adding this :

Future<HttpClientResponse> getUrlWithRetry(HttpClient httpClient, Uri url, {int maxRetries = 5}) async {
    for (var attempt = 0; attempt < maxRetries; attempt++) {
      try {
        final request = await httpClient.openUrl('GET', url);
        final response = await request.close();
        return await response;
      } 
     ...
    }
  }

Where this piece of code is being called when I use FadeInImage.memoryNetwork, and now the network error is gone. Thanks a lot for your assistance, it means a lot!

Still got this _Lost connection to device._ , but I think this is caused by inefficiency somewhere in my code or API. Thanks to you, now I can actually do the optimization. My doubts on flutter are cleared after this quick response and helpful assistance :)