Merge pull request grpc#23632 from lidizheng/raise-when-bind-failed

Raises an exception when port binding failed

Author: lidizheng

src/python/grpcio/grpc/_common.py

@@ -61,6 +61,9 @@
 
 MAXIMUM_WAIT_TIMEOUT = 0.1
 
+_ERROR_MESSAGE_PORT_BINDING_FAILED = 'Failed to bind to address %s; set ' \
+    'GRPC_VERBOSITY=debug environment variable to see detailed error message.'
+
 
 def encode(s):
     if isinstance(s, bytes):
@@ -144,3 +147,22 @@ def wait(wait_fn, wait_complete_fn, timeout=None, spin_cb=None):
                 return True
             _wait_once(wait_fn, remaining, spin_cb)
     return False
+
+
+def validate_port_binding_result(address, port):
+    """Validates if the port binding succeed.
+
+    If the port returned by Core is 0, the binding is failed. However, in that
+    case, the Core API doesn't return a detailed failing reason. The best we
+    can do is raising an exception to prevent further confusion.
+
+    Args:
+        address: The address string to be bound.
+        port: An int returned by core
+    """
+    if port == 0:
+        # The Core API doesn't return a failure message. The best we can do
+        # is raising an exception to prevent further confusion.
+        raise RuntimeError(_ERROR_MESSAGE_PORT_BINDING_FAILED % address)
+    else:
+        return port

src/python/grpcio/grpc/_server.py

@@ -958,11 +958,14 @@ def add_generic_rpc_handlers(self, generic_rpc_handlers):
         _add_generic_handlers(self._state, generic_rpc_handlers)
 
     def add_insecure_port(self, address):
-        return _add_insecure_port(self._state, _common.encode(address))
+        return _common.validate_port_binding_result(
+            address, _add_insecure_port(self._state, _common.encode(address)))
 
     def add_secure_port(self, address, server_credentials):
-        return _add_secure_port(self._state, _common.encode(address),
-                                server_credentials)
+        return _common.validate_port_binding_result(
+            address,
+            _add_secure_port(self._state, _common.encode(address),
+                             server_credentials))
 
     def start(self):
         _start(self._state)

src/python/grpcio/grpc/experimental/aio/_server.py

@@ -80,7 +80,8 @@ def add_insecure_port(self, address: str) -> int:
         Returns:
           An integer port on which the server will accept RPC requests.
         """
-        return self._server.add_insecure_port(_common.encode(address))
+        return _common.validate_port_binding_result(
+            address, self._server.add_insecure_port(_common.encode(address)))
 
     def add_secure_port(self, address: str,
                         server_credentials: grpc.ServerCredentials) -> int:
@@ -97,8 +98,10 @@ def add_secure_port(self, address: str,
         Returns:
           An integer port on which the server will accept RPC requests.
         """
-        return self._server.add_secure_port(_common.encode(address),
-                                            server_credentials)
+        return _common.validate_port_binding_result(
+            address,
+            self._server.add_secure_port(_common.encode(address),
+                                         server_credentials))
 
     async def start(self) -> None:
         """Starts this Server.

src/python/grpcio_tests/commands.py

@@ -227,7 +227,10 @@ class TestGevent(setuptools.Command):
     )
     BANNED_WINDOWS_TESTS = (
         # TODO(https://github.com/grpc/grpc/pull/15411) enable this test
-        'unit._dns_resolver_test.DNSResolverTest.test_connect_loopback',)
+        'unit._dns_resolver_test.DNSResolverTest.test_connect_loopback',
+        # TODO(https://github.com/grpc/grpc/pull/15411) enable this test
+        'unit._server_test.ServerTest.test_failed_port_binding_exception',
+    )
     description = 'run tests with gevent.  Assumes grpc/gevent are installed'
     user_options = []
 

src/python/grpcio_tests/tests/unit/_server_test.py

@@ -18,6 +18,8 @@
 
 import grpc
 
+from tests.unit import resources
+
 
 class _ActualGenericRpcHandler(grpc.GenericRpcHandler):
 
@@ -47,6 +49,20 @@ def test_not_a_generic_rpc_handler_after_construction(self):
         self.assertIn('grpc.GenericRpcHandler',
                       str(exception_context.exception))
 
+    def test_failed_port_binding_exception(self):
+        server = grpc.server(None, options=(('grpc.so_reuseport', 0),))
+        port = server.add_insecure_port('localhost:0')
+        bind_address = "localhost:%d" % port
+
+        with self.assertRaises(RuntimeError):
+            server.add_insecure_port(bind_address)
+
+        server_credentials = grpc.ssl_server_credentials([
+            (resources.private_key(), resources.certificate_chain())
+        ])
+        with self.assertRaises(RuntimeError):
+            server.add_secure_port(bind_address, server_credentials)
+
 
 if __name__ == '__main__':
     logging.basicConfig()

src/python/grpcio_tests/tests_aio/unit/server_test.py

@@ -15,12 +15,14 @@
 import asyncio
 import gc
 import logging
+import socket
 import time
 import unittest
 
 import grpc
 from grpc.experimental import aio
 
+from tests.unit import resources
 from tests.unit.framework.common import test_constants
 from tests_aio.unit._test_base import AioTestBase
 
@@ -464,6 +466,20 @@ async def test_error_without_raise_in_stream_stream(self):
 
         self.assertEqual(grpc.StatusCode.INTERNAL, await call.code())
 
+    async def test_port_binding_exception(self):
+        server = aio.server(options=(('grpc.so_reuseport', 0),))
+        port = server.add_insecure_port('localhost:0')
+        bind_address = "localhost:%d" % port
+
+        with self.assertRaises(RuntimeError):
+            server.add_insecure_port(bind_address)
+
+        server_credentials = grpc.ssl_server_credentials([
+            (resources.private_key(), resources.certificate_chain())
+        ])
+        with self.assertRaises(RuntimeError):
+            server.add_secure_port(bind_address, server_credentials)
+
 
 if __name__ == '__main__':
     logging.basicConfig(level=logging.DEBUG)