Start better error reporting for sockets

runtime/bin/socket_impl.dart

Index: runtime/bin/socket_impl.dart
diff --git a/runtime/bin/socket_impl.dart b/runtime/bin/socket_impl.dart
index 364e5b6ac2e851bc85c1f9756fcbef827b3a36c2..70415f299e17d6aad27e4e7dd6b2078431c98f21 100644
--- a/runtime/bin/socket_impl.dart
+++ b/runtime/bin/socket_impl.dart
@@ -63,7 +63,12 @@ class _SocketBase {
           if ((i == _IN_EVENT) && (this is _Socket) && (available() == 0)) {
             continue;
           }
-          eventHandler();
+          if (i == _ERROR_EVENT) {
+            eventHandler(new SocketIOException(""));

[Mads Ager (google), 2012/03/14 12:51:51]

I suppose next step is to find a way to actually propagate some information
here?

[Søren Gjesse, 2012/03/19 10:05:43]

Yes.

+            close();
+          } else {
+            eventHandler();
+          }
         }
       }
     }
@@ -71,7 +76,7 @@ class _SocketBase {
     _activateHandlers();
   }
 
-  void _setHandler(int event, void callback()) {
+  void _setHandler(int event, Function callback) {
     if (callback == null) {
       _handlerMask &= ~(1 << event);
     } else {
@@ -93,7 +98,7 @@ class _SocketBase {
 
   void _getPort() native "Socket_GetPort";
 
-  void set onError(void callback()) {
+  void set onError(void callback(Exception e)) {
     _setHandler(_ERROR_EVENT, callback);
   }
 
@@ -255,6 +260,10 @@ class _ServerSocket extends _SocketBase implements ServerSocket {
 
 
 class _Socket extends _SocketBase implements Socket {
+  static final kSuccessResponse = 0;
+  static final kIllegalArgumentResponse = 1;
+  static final kOSErrorResponse = 2;
+
   // Constructor for socket. First a socket object is allocated
   // in which the native socket is stored. After that _createConnect is
   // called which creates a file discriptor and connects to the given
@@ -262,10 +271,22 @@ class _Socket extends _SocketBase implements Socket {
   // or connect failed.
   factory _Socket(String host, int port) {
     Socket socket = new _Socket._internal();
-    if (!socket._createConnect(host, port)) {
-      socket.close();
-      return null;
-    }
+    _ensureSocketService();
+    List request = new List(2);
+    request[0] = 0;

[Mads Ager (google), 2012/03/14 12:51:51]

Maybe create a kHostLookup constant?

[Søren Gjesse, 2012/03/19 10:05:43]

Done.

+    request[1] = host;
+    _socketService.call(request).then((response) {
+      if (socket._isErrorResponse(response)) {
+        socket._reportError(response, "Failed host name lookup");
+      } else {
+        if (!socket._createConnect(response, port)) {
+          socket.close();
+          return null;

[Mads Ager (google), 2012/03/14 12:51:51]

No need for the null return here.

We should probably report an error here or something? We used to return null
which the user could see. Now we don't.

[Søren Gjesse, 2012/03/19 10:05:43]

Good catch. Changed to do an onError callback when initiating connecting the
socket fails.

+        } else {
+          socket._activateHandlers();
+        }
+      }
+      });
     return socket;
   }
 
@@ -301,7 +322,7 @@ class _Socket extends _SocketBase implements Socket {
       }
       int result = _readList(buffer, offset, bytes);
       if (result < 0) {
-        _reportError();
+        _reportError(null, "Read failed");
       }
       return result;
     }
@@ -352,7 +373,7 @@ class _Socket extends _SocketBase implements Socket {
         // If writing fails we return 0 as the number of bytes and
         // report the error on the error handler.
         bytes_written = 0;
-        _reportError();
+        _reportError(null, "Write failed");
       }
       return bytes_written;
     }
@@ -363,14 +384,34 @@ class _Socket extends _SocketBase implements Socket {
   int _writeList(List<int> buffer, int offset, int bytes)
       native "Socket_WriteList";
 
-  void _reportError() {
+  bool _isErrorResponse(response) {
+    return response is List && response[0] != _FileUtils.kSuccessResponse;
+  }
+
+  bool _reportError(response, String message) {
+    if (response != null) {
+      assert(_isErrorResponse(response));
+    }
     // For all errors we close the socket, call the error handler and
     // disable further calls of the error handler.
     close();
     var onError = _handlerMap[_ERROR_EVENT];
     if (onError != null) {
-      onError();
-      _setHandler(_ERROR_EVENT, null);
+      if (response != null) {
+        switch (response[0]) {
+          case _FileUtils.kIllegalArgumentResponse:
+            onError(new IllegalArgumentException());
+            break;
+          case _FileUtils.kOSErrorResponse:
+            onError(new SocketIOException(
+                message, new OSError(response[2], response[1])));
+            break;
+          default:
+            onError(new Exception("Unknown error"));

[Mads Ager (google), 2012/03/14 12:51:51]

End the default case with a break as well.

[Søren Gjesse, 2012/03/19 10:05:43]

Done.

+        }
+      } else {
+        onError(new SocketIOException(message));
+      }
     }
   }
 
@@ -404,7 +445,7 @@ class _Socket extends _SocketBase implements Socket {
 
   void set onClosed(void callback()) {
     if (_inputStream != null) throw new StreamException(
-            "Cannot set close handler when input stream is used");
+           "Cannot set close handler when input stream is used");
     _onClosed = callback;
   }
 
@@ -477,6 +518,14 @@ class _Socket extends _SocketBase implements Socket {
     }
   }
 
+  static SendPort _newServicePort() native "Socket_NewServicePort";
+
+  static void _ensureSocketService() {
+    if (_socketService == null) {
+      _socketService = _Socket._newServicePort();
+    }
+  }
+
   bool _seenFirstOutEvent = false;
   bool _closedRead = false;
   bool _closedWrite = false;
@@ -485,5 +534,5 @@ class _Socket extends _SocketBase implements Socket {
   Function _clientWriteHandler;
   SocketInputStream _inputStream;
   SocketOutputStream _outputStream;
+  static SendPort _socketService;
 }
-