Implement addStream for HttpClientRequest/HttpResponse and propegate all write-errors from the sock…

Implement addStream for HttpClientRequest/HttpResponse and propegate all write-errors from the socket.


BUG=https://code.google.com/p/dart/issues/detail?id=8821

Committed: https://code.google.com/p/dart/source/detail?r=19213

[Søren Gjesse, 2013-02-27 15:07:43]

lgtm, with comments

https://codereview.chromium.org/12317147/diff/1/tests/standalone/io/http_serv...
File tests/standalone/io/http_server_response_test.dart (right):

https://codereview.chromium.org/12317147/diff/1/tests/standalone/io/http_serv...
tests/standalone/io/http_server_response_test.dart:48: void
testResponseAddStream() {
Please add a test where you add more than one stream.

Please add a test where you add an empty stream.

Maybe we should check that the client actually received the concattenated
streams.

https://codereview.chromium.org/12317147/diff/1/tests/standalone/io/http_shut...
File tests/standalone/io/http_shutdown_test.dart (right):

https://codereview.chromium.org/12317147/diff/1/tests/standalone/io/http_shut...
tests/standalone/io/http_shutdown_test.dart:56: request.done.catchError((_) {});
I think we should add default handling of this so that if there is no error
handler on the done future something default will happen. Developers will forget
this and there HTTP servers will crash. Not for this CL, but lets think about
it.

[Anders Johnsen, 2013-02-28 11:39:01]

PTAL

https://codereview.chromium.org/12317147/diff/1/tests/standalone/io/http_serv...
File tests/standalone/io/http_server_response_test.dart (right):

https://codereview.chromium.org/12317147/diff/1/tests/standalone/io/http_serv...
tests/standalone/io/http_server_response_test.dart:48: void
testResponseAddStream() {
On 2013/02/27 15:07:43, Søren Gjesse wrote:
> Please add a test where you add more than one stream.
> 
> Please add a test where you add an empty stream.
> 
> Maybe we should check that the client actually received the concattenated
> streams.

Done.

https://codereview.chromium.org/12317147/diff/1/tests/standalone/io/http_shut...
File tests/standalone/io/http_shutdown_test.dart (right):

https://codereview.chromium.org/12317147/diff/1/tests/standalone/io/http_shut...
tests/standalone/io/http_shutdown_test.dart:56: request.done.catchError((_) {});
On 2013/02/27 15:07:43, Søren Gjesse wrote:
> I think we should add default handling of this so that if there is no error
> handler on the done future something default will happen. Developers will
forget
> this and there HTTP servers will crash. Not for this CL, but lets think about
> it.

Yes, we need to decide on what should happen on this one. Let's discuss offline
for another CL.

[Søren Gjesse, 2013-02-28 11:53:23]

LGTM - it is really nice to get the errors propagated correctly!

Please add a reference to issue 8821 to the decscrition.

[Anders Johnsen, 2013-02-28 11:55:13]

Committed patchset #2 manually as r19213 (presubmit successful).