Repository: tornadoweb/tornado

Issue:
RequestHandler.finish should return a Future
`RequestHandler.finish` may call `flush()`, which returns a Future, but this Future is simply discarded. The main reason for that Future is flow control in streaming responses, which is no longer relevant by the time we are closing the connection, but it also contains errors if the stream is closed while the response is streamed. This error will be logged as a stack trace if left uncaught, so some applications may wish to await their calls to `finish()` to be able to catch it. 

This logic also extends to `render()`, which calls `finish()`. 

From https://github.com/tornadoweb/tornado/issues/2055#issuecomment-304456147

Discussion:

Patch:
diff --git a/tornado/iostream.py b/tornado/iostream.py
index 89e1e234..63110a1a 100644
--- a/tornado/iostream.py
+++ b/tornado/iostream.py
@@ -1410,13 +1410,7 @@ class IOStream(BaseIOStream):
         return future
 
     def _handle_connect(self):
-        try:
-            err = self.socket.getsockopt(socket.SOL_SOCKET, socket.SO_ERROR)
-        except socket.error as e:
-            # Hurd doesn't allow SO_ERROR for loopback sockets because all
-            # errors for such sockets are reported synchronously.
-            if errno_from_exception(e) == errno.ENOPROTOOPT:
-                err = 0
+        err = self.socket.getsockopt(socket.SOL_SOCKET, socket.SO_ERROR)
         if err != 0:
             self.error = socket.error(err, os.strerror(err))
             # IOLoop implementations may vary: some of them return
diff --git a/tornado/netutil.py b/tornado/netutil.py
index e63683ad..08c9d886 100644
--- a/tornado/netutil.py
+++ b/tornado/netutil.py
@@ -138,12 +138,7 @@ def bind_sockets(port, address=None, family=socket.AF_UNSPEC,
             raise
         set_close_exec(sock.fileno())
         if os.name != 'nt':
-            try:
-                sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-            except socket.error as e:
-                if errno_from_exception(e) != errno.ENOPROTOOPT:
-                    # Hurd doesn't support SO_REUSEADDR.
-                    raise
+            sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
         if reuse_port:
             sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
         if af == socket.AF_INET6:
@@ -185,12 +180,7 @@ if hasattr(socket, 'AF_UNIX'):
         """
         sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
         set_close_exec(sock.fileno())
-        try:
-            sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-        except socket.error as e:
-            if errno_from_exception(e) != errno.ENOPROTOOPT:
-                # Hurd doesn't support SO_REUSEADDR
-                raise
+        sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
         sock.setblocking(0)
         try:
             st = os.stat(file)
diff --git a/tornado/web.py b/tornado/web.py
index f970bd13..6760b0b9 100644
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -749,7 +749,18 @@ class RequestHandler(object):
         self._write_buffer.append(chunk)
 
     def render(self, template_name, **kwargs):
-        """Renders the template with the given arguments as the response."""
+        """Renders the template with the given arguments as the response.
+
+        ``render()`` calls ``finish()``, so no other output methods can be called
+        after it.
+
+        Returns a `.Future` with the same semantics as the one returned by `finish`.
+        Awaiting this `.Future` is optional.
+
+        .. versionchanged:: 5.1
+
+           Now returns a `.Future` instead of ``None``.
+        """
         if self._finished:
             raise RuntimeError("Cannot render() after finish()")
         html = self.render_string(template_name, **kwargs)
@@ -810,7 +821,7 @@ class RequestHandler(object):
         if html_bodies:
             hloc = html.index(b'</body>')
             html = html[:hloc] + b''.join(html_bodies) + b'\n' + html[hloc:]
-        self.finish(html)
+        return self.finish(html)
 
     def render_linked_js(self, js_files):
         """Default method used to render the final js links for the
@@ -993,7 +1004,20 @@ class RequestHandler(object):
                 return future
 
     def finish(self, chunk=None):
-        """Finishes this response, ending the HTTP request."""
+        """Finishes this response, ending the HTTP request.
+
+        Passing a ``chunk`` to ``finish()`` is equivalent to passing that
+        chunk to ``write()`` and then calling ``finish()`` with no arguments.
+
+        Returns a `.Future` which may optionally be awaited to track the sending
+        of the response to the client. This `.Future` resolves when all the response
+        data has been sent, and raises an error if the connection is closed before all
+        data can be sent.
+
+        .. versionchanged:: 5.1
+
+           Now returns a `.Future` instead of ``None``.
+        """
         if self._finished:
             raise RuntimeError("finish() called twice")
 
@@ -1025,12 +1049,13 @@ class RequestHandler(object):
             # are keepalive connections)
             self.request.connection.set_close_callback(None)
 
-        self.flush(include_footers=True)
+        future = self.flush(include_footers=True)
         self.request.connection.finish()
         self._log()
         self._finished = True
         self.on_finish()
         self._break_cycles()
+        return future
 
     def detach(self):
         """Take control of the underlying stream.


Test patch:
diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index 45072aac..b77311df 100644
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -191,6 +191,40 @@ class SecureCookieV2Test(unittest.TestCase):
         self.assertEqual(new_handler.get_secure_cookie('foo'), None)
 
 
+class FinalReturnTest(WebTestCase):
+    def get_handlers(self):
+        test = self
+
+        class FinishHandler(RequestHandler):
+            @gen.coroutine
+            def get(self):
+                test.final_return = self.finish()
+
+        class RenderHandler(RequestHandler):
+            def create_template_loader(self, path):
+                return DictLoader({'foo.html': 'hi'})
+
+            @gen.coroutine
+            def get(self):
+                test.final_return = self.render('foo.html')
+
+        return [("/finish", FinishHandler),
+                ("/render", RenderHandler)]
+
+    def get_app_kwargs(self):
+        return dict(template_path='FinalReturnTest')
+
+    def test_finish_method_return_future(self):
+        response = self.fetch(self.get_url('/finish'))
+        self.assertEqual(response.code, 200)
+        self.assertIsInstance(self.final_return, Future)
+
+    def test_render_method_return_future(self):
+        response = self.fetch(self.get_url('/render'))
+        self.assertEqual(response.code, 200)
+        self.assertIsInstance(self.final_return, Future)
+
+
 class CookieTest(WebTestCase):
     def get_handlers(self):
         class SetCookieHandler(RequestHandler):