Sdk: dart:io HttpClient.post throws uncatch-able exception

[Pacane]

I have this issue where I use dart:io's HttpClient to post data client.post and that the post throws the following exception:

      if (_nextResponseCompleter == null) {
        throw new HttpException(
            "Unexpected response (unsolicited response without request).",
            uri: _currentUri);
      }

(from http_impl.dart)

when I try to put the client.post in a try-catch block, the exception doesn't bubble up in my catch. Instead, it goes up until it's uncaught and make my program crash.

with the help of @floitschG I could successfully catch the error by issuing my client.post in a runZoned, but this shouldn't be necessary.

This bug affected our system in production and made our server crash.

[floitschG]

@zanderso : I looked a little bit into this.
The problem is that this exception is thrown from within the listen to a _httpParser. This means that the user can't ever catch this error:

    _subscription = _httpParser.listen((incoming) {
      // Only handle one incoming response at the time. Keep the
      // stream paused until the response have been processed.
      _subscription.pause();
      // We assume the response is not here, until we have send the request.
      if (_nextResponseCompleter == null) {
        throw new HttpException(
            "Unexpected response (unsolicited response without request).",
            uri: _currentUri);
      }

The big question is: how should this error be reported?. Just throwing isn't good enough, since it would never reach the user. The surrounding code doesn't give me enough information.

When can _nextResponseCompleter == null be true?
If it is null did we already report an error (or success), and should we then just ignore the error? (still better than crashing the VM).

@Pacane could you provide some feedback on how this was happening?
What client/server interaction seems to trigger this?

[Pacane]

So this happens when I do a post request to a remote server, and the server gives a 408 response (Request Timeout).

When the server returns the 408, _nextResponseCompleter == null is true, and the exception is thrown.

The production had this pattern

connect() async {
     var client = getSomeClient();
     try {
          HttpClientRequest request = await client.postUrl(connUri);
          // more code
     catch(e) {
          retryConnectWithSomeDelay();
     }
}

For some reason, when I do this

connect() async {
     var client = getSomeClient();
     try {
          HttpClientRequest request = await client.postUrl(connUri);
          // more code
     catch(e) {
          retryConnectWithSomeDelay();
     }
}

the error happens. But if I close the client before retrying:

connect() async {
     var client = getSomeClient();
     try {
          HttpClientRequest request = await client.postUrl(connUri);
          // more code
     catch(e) {
          __client.close();__
          retryConnectWithSomeDelay();
     }
}

I have no error.

Note that I can freely remove the retryConnectWithSomeDelay() and get essentially the same behaviour. In other words, the minimal code needed to reproduce this (omitting some details like certificates and [I think] other irrelevant stuff) is:

connect() async {
     var client = getSomeClient();
     HttpClientRequest request = await client.postUrl(connUri);
}

with a server returning a 408. When I debugged the _HttpClientConnection, when the exception was gonna be thrown, the incoming value from the _httpParser.listen() was:

Note that this server returning the 408 is a 3rd party server that I don't have access to unfortunately.

[Schwusch]

I get this when using the Dart AWS Lambda Runtime in the lambci/lambda:provided Docker image in the AWS SAM CLI tool:

Unhandled exception:
HttpException: Unexpected response (unsolicited response without request).

The server interacting with the Dart HTTP client is a Go server.
Specifically the postInvocationError request that is handled by the handleErrorRequest function gives the "Unexpected response" exception.

[zanderso]

@ZichangG @sortie

[zichangg]

      if (incoming.statusCode == 100) {
        // some codes
      } else {
        _nextResponseCompleter.complete(incoming);
        _nextResponseCompleter = null;
      }

          if (!closing &&
              !_dispose &&
              incoming.headers.persistentConnection &&
              request.persistentConnection) {
            // Return connection, now we are done.
            _httpClient._returnConnection(this);
            _subscription.resume();
          } else {
            destroy();
          }

I'm not confident whether this is a bug. In our implementation, if statusCode != 100, we complete with incoming and set _nextResponseCompleter to null. Because of persistent connection, it resumes the subscription so the next incoming arrived and _nextResponseCompleter is null.
Is this behavior correct?

[Schwusch]

I'm wondering whether it's a bug in the failing request or the request that came before.
Is there a possibility that the previous request is causing the unexpected response?
How is this handled in other languages?

[zichangg]

According to RFC,

A server SHOULD send the "close" connection option
(Section 6.1 of [RFC7230]) in the response, since 408 implies that
the server has decided to close the connection rather than continue
waiting.

This looks like the case where server didn't explicitly close.

@Schwusch or @Pacane Could you please try my patch(https://dart-review.googlesource.com/c/sdk/+/144346) to see whether it worked?

[Schwusch]

@zichangg I'll make an attempt to download the patchset and build the SDK today, or is there a build artifact or alike from the CI/CD?

[Schwusch]

@zichangg It worked when I tested it! You're awesome :1st_place_medal:

[zichangg]

@Schwusch Thanks! I'll try to get it landed asap.