Start better error reporting for sockets

runtime/bin/socket_impl.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 
 class _SocketBase {
   // Bit flags used when communicating between the eventhandler and
   // dart code. The EVENT flags are used to indicate events of
   // interest when sending a message from dart code to the
   // eventhandler. When receiving a message from the eventhandler the
@@ skipping 45 matching lines @@
         var eventHandler = _handlerMap[i];
         if (eventHandler != null) {
           // Unregister the out handler before executing it.
           if (i == _OUT_EVENT) _setHandler(i, null);
 
           // Don't call the in handler if there is no data available
           // after all.
           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();
+          }
         }
       }
     }
     _canActivateHandlers = true;
     _activateHandlers();
   }
 
-  void _setHandler(int event, void callback()) {
+  void _setHandler(int event, Function callback) {
     if (callback == null) {
       _handlerMask &= ~(1 << event);
     } else {
       _handlerMask |= (1 << event);
     }
     _handlerMap[event] = callback;
     // If the socket is only for writing then close the receive port
     // when not waiting for any events.
     if (this is _Socket &&
         _closedRead &&
         _handlerMask == 0 &&
         _handler != null) {
       _handler.close();
       _handler = null;
     } else {
       _activateHandlers();
     }
   }
 
   void _getPort() native "Socket_GetPort";
 
-  void set onError(void callback()) {
+  void set onError(void callback(Exception e)) {
     _setHandler(_ERROR_EVENT, callback);
   }
 
   void _activateHandlers() {
     if (_canActivateHandlers && (_id >= 0)) {
       if (_handlerMask == 0) {
         if (_handler != null) {
           _handler.close();
           _handler = null;
         }
@@ skipping 141 matching lines @@
   }
 
   bool _isListenSocket() => true;
   bool _isPipe() => false;
 
   var _clientConnectionHandler;
 }
 
 
 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
   // host on the given port. Null is returned if file descriptor creation

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

Update description. Maybe in the public API as well?

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

Done.

   // 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;
   }
 
   _Socket._internal();
   _Socket._internalReadOnly() : _closedWrite = true, _pipe = true;
   _Socket._internalWriteOnly() : _closedRead = true, _pipe = true;
 
   int available() {
     if (_id >= 0) {
       return _available();
@@ skipping 15 matching lines @@
         throw new IndexOutOfRangeException(offset);
       }
       if (bytes < 0) {
         throw new IndexOutOfRangeException(bytes);
       }
       if ((offset + bytes) > buffer.length) {
         throw new IndexOutOfRangeException(offset + bytes);
       }
       int result = _readList(buffer, offset, bytes);
       if (result < 0) {
-        _reportError();
+        _reportError(null, "Read failed");
       }
       return result;
     }
     throw new
         SocketIOException("Error: readList failed - invalid socket handle");
   }
 
   int _readList(List<int> buffer, int offset, int bytes)
       native "Socket_ReadList";
 
@@ skipping 30 matching lines @@
           }
           outBuffer[i] = value;
           j++;
         }
       }
       var bytes_written = _writeList(outBuffer, outOffset, bytes);
       if (bytes_written < 0) {
         // 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;
     }
     throw new
         SocketIOException("Error: writeList failed - invalid socket handle");
   }
 
   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));
+      }
     }
   }
 
   bool _createConnect(String host, int port) native "Socket_CreateConnect";
 
   void set onWrite(void callback()) {
     if (_outputStream != null) throw new StreamException(
             "Cannot set write handler when output stream is used");
     _clientWriteHandler = callback;
     _updateOutHandler();
@@ skipping 13 matching lines @@
   }
 
   void set onData(void callback()) {
     if (_inputStream != null) throw new StreamException(
             "Cannot set data handler when input stream is used");
     _onData = callback;
   }
 
   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;
   }
 
   bool _isListenSocket() => false;
 
   bool _isPipe() => _pipe;
 
   InputStream get inputStream() {
     if (_inputStream === null) {
       if (_handlerMap[_IN_EVENT] !== null ||
@@ skipping 52 matching lines @@
       _onWrite = null;
     } else {
       if (_seenFirstOutEvent) {
         _onWrite = _clientWriteHandler;
       } else {
         _onWrite = firstWriteHandler;
       }
     }
   }
 
+  static SendPort _newServicePort() native "Socket_NewServicePort";
+
+  static void _ensureSocketService() {
+    if (_socketService == null) {
+      _socketService = _Socket._newServicePort();
+    }
+  }
+
   bool _seenFirstOutEvent = false;
   bool _closedRead = false;
   bool _closedWrite = false;
   bool _pipe = false;
   Function _clientConnectHandler;
   Function _clientWriteHandler;
   SocketInputStream _inputStream;
   SocketOutputStream _outputStream;
+  static SendPort _socketService;
 }
-