Show nicer errors on some server failures.

Show nicer errors on some server failures.

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

[Bob Nystrom, 2012-08-31 23:50:13]

Fixes: http://code.google.com/p/dart/issues/detail?id=4854

This patch also includes changes in corelib from
https://chromiumcodereview.appspot.com/10908040/. I'll rebase those out once
that's in. You can ignore them here.

[nweiz, 2012-09-04 19:29:25]

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart
File utils/pub/io.dart (right):

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:315: Future<String> httpGetString(uri) {
Why is this method different than httpGet? Is it just because ListInputStream
doesn't support error propagation? If so, you should add a TODO here to merge
these two when issue 3657 is fixed.

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:324: if ((e is SocketIOException) && (e.osError.errorCode ==
8)) {
I don't like these unnecessary parens. I don't think it's unclear how tightly
"&&" binds.

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:329: };
Shouldn't you call client.shutdown here, too?

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:344: // we will have already completed with that exception.
onClosed and onError should be mutually exclusive.

[Bob Nystrom, 2012-09-04 22:14:46]

Thanks!

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart
File utils/pub/io.dart (right):

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:315: Future<String> httpGetString(uri) {
On 2012/09/04 19:29:25, nweiz wrote:
> Why is this method different than httpGet? Is it just because ListInputStream
> doesn't support error propagation?

Yup.

> If so, you should add a TODO here to merge
> these two when issue 3657 is fixed.

Done.

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:324: if ((e is SocketIOException) && (e.osError.errorCode ==
8)) {
On 2012/09/04 19:29:25, nweiz wrote:
> I don't like these unnecessary parens. I don't think it's unclear how tightly
> "&&" binds.

Done.

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:329: };
On 2012/09/04 19:29:25, nweiz wrote:
> Shouldn't you call client.shutdown here, too?

Done.

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:344: // we will have already completed with that exception.
On 2012/09/04 19:29:25, nweiz wrote:
> onClosed and onError should be mutually exclusive.

They didn't seem to be. Without this check, I get a FutureAlreadyCompleted
error.

[nweiz, 2012-09-04 22:38:18]

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart
File utils/pub/io.dart (right):

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:344: // we will have already completed with that exception.
On 2012/09/04 22:14:46, Bob Nystrom wrote:
> On 2012/09/04 19:29:25, nweiz wrote:
> > onClosed and onError should be mutually exclusive.
> 
> They didn't seem to be. Without this check, I get a FutureAlreadyCompleted
> error.

I believe that's a bug according to
https://docs.google.com/a/google.com/document/d/1ybQCrO6fbiTdj2h37G4Sgd-rMPzp....
I think https://chromiumcodereview.appspot.com/10836177/ will fix it, though.

[Bob Nystrom, 2012-09-04 23:01:20]

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart
File utils/pub/io.dart (right):

https://chromiumcodereview.appspot.com/10917053/diff/1/utils/pub/io.dart#newc...
utils/pub/io.dart:344: // we will have already completed with that exception.
On 2012/09/04 22:38:18, nweiz wrote:
> On 2012/09/04 22:14:46, Bob Nystrom wrote:
> > On 2012/09/04 19:29:25, nweiz wrote:
> > > onClosed and onError should be mutually exclusive.
> > 
> > They didn't seem to be. Without this check, I get a FutureAlreadyCompleted
> > error.
> 
> I believe that's a bug according to
>
https://docs.google.com/a/google.com/document/d/1ybQCrO6fbiTdj2h37G4Sgd-rMPzp....
> I think https://chromiumcodereview.appspot.com/10836177/ will fix it, though.

The pub tests seem to pass without the check, so I removed it. I recall seeing
an error here, but maybe that was just with my own local testing.

[nweiz, 2012-09-05 00:01:41]

lgtm