Make header_callback behavior consistent across both HTTP clients.

SimpleAsyncHTTPClient now sends the first and last lines to the callback,
matching CurlAsyncHTTPClient's behavior.

All secondary callbacks are now wrapped for stack_context.

Closes tornadoweb#637.

tornado/httpclient.py

@@ -38,9 +38,9 @@
 import weakref
 
 from tornado.escape import utf8
-from tornado import httputil
+from tornado import httputil, stack_context
 from tornado.ioloop import IOLoop
-from tornado.util import import_object, bytes_type, Configurable
+from tornado.util import import_object, Configurable
 
 
 class HTTPClient(object):
@@ -232,8 +232,13 @@ def __init__(self, url, method="GET", headers=None, body=None,
            `~HTTPResponse.body` and `~HTTPResponse.buffer` will be empty in
            the final response.
         :arg callable header_callback: If set, `header_callback` will
-           be run with each header line as it is received, and
-           `~HTTPResponse.headers` will be empty in the final response.
+           be run with each header line as it is received (including the
+           first line, e.g. ``HTTP/1.0 200 OK\r\n``, and a final line
+           containing only ``\r\n``.  All lines include the trailing newline
+           characters).  `~HTTPResponse.headers` will be empty in the final
+           response.  This is most useful in conjunction with
+           `streaming_callback`, because it's the only way to get access to
+           header data while the request is in progress.
         :arg callable prepare_curl_callback: If set, will be called with
            a `pycurl.Curl` object to allow the application to make additional
            `setopt` calls.
@@ -281,9 +286,9 @@ def __init__(self, url, method="GET", headers=None, body=None,
         self.user_agent = user_agent
         self.use_gzip = use_gzip
         self.network_interface = network_interface
-        self.streaming_callback = streaming_callback
-        self.header_callback = header_callback
-        self.prepare_curl_callback = prepare_curl_callback
+        self.streaming_callback = stack_context.wrap(streaming_callback)
+        self.header_callback = stack_context.wrap(header_callback)
+        self.prepare_curl_callback = stack_context.wrap(prepare_curl_callback)
         self.allow_nonstandard_methods = allow_nonstandard_methods
         self.validate_cert = validate_cert
         self.ca_certs = ca_certs

tornado/simple_httpclient.py

@@ -365,8 +365,11 @@ def _on_headers(self, data):
             content_length = None
 
         if self.request.header_callback is not None:
+            # re-attach the newline we split on earlier
+            self.request.header_callback(first_line + _)
             for k, v in self.headers.get_all():
                 self.request.header_callback("%s: %s\r\n" % (k, v))
+            self.request.header_callback('\r\n')
 
         if self.request.method == "HEAD":
             # HEAD requests never have content, even though they may have

tornado/test/curl_httpclient_test.py

@@ -1,6 +1,10 @@
 from __future__ import absolute_import, division, with_statement
+from tornado.httpclient import HTTPRequest
+from tornado.stack_context import ExceptionStackContext
+from tornado.testing import AsyncHTTPTestCase
 from tornado.test import httpclient_test
 from tornado.test.util import unittest
+from tornado.web import Application
 
 try:
     import pycurl
@@ -20,3 +24,30 @@ def get_http_client(self):
 CurlHTTPClientCommonTestCase = unittest.skipIf(pycurl is None,
                                                "pycurl module not present")(
     CurlHTTPClientCommonTestCase)
+
+
+class CurlHTTPClientTestCase(AsyncHTTPTestCase):
+    def setUp(self):
+        super(CurlHTTPClientTestCase, self).setUp()
+        self.http_client = CurlAsyncHTTPClient(self.io_loop)
+
+    def get_app(self):
+        return Application([])
+
+    def test_prepare_curl_callback_stack_context(self):
+        exc_info = []
+        def error_handler(typ, value, tb):
+            exc_info.append((typ, value, tb))
+            self.stop()
+            return True
+
+        with ExceptionStackContext(error_handler):
+            request = HTTPRequest(self.get_url('/'),
+                                  prepare_curl_callback=lambda curl: 1 / 0)
+        self.http_client.fetch(request, callback=self.stop)
+        self.wait()
+        self.assertEqual(1, len(exc_info))
+        self.assertIs(exc_info[0][0], ZeroDivisionError)
+CurlHTTPClientTestCase = unittest.skipIf(pycurl is None,
+                                         "pycurl module not present")(
+    CurlHTTPClientTestCase)

tornado/test/httpclient_test.py

@@ -6,10 +6,12 @@
 import binascii
 from contextlib import closing
 import functools
+import re
 
 from tornado.escape import utf8
 from tornado.iostream import IOStream
 from tornado import netutil
+from tornado.stack_context import ExceptionStackContext
 from tornado.testing import AsyncHTTPTestCase, bind_unused_port
 from tornado.util import b, bytes_type
 from tornado.web import Application, RequestHandler, url
@@ -135,6 +137,25 @@ def accept_callback(conn, address):
             self.assertEqual(resp.body, b("12"))
             self.io_loop.remove_handler(sock.fileno())
 
+    def test_streaming_stack_context(self):
+        chunks = []
+        exc_info = []
+        def error_handler(typ, value, tb):
+            exc_info.append((typ, value, tb))
+            return True
+
+        def streaming_cb(chunk):
+            chunks.append(chunk)
+            if chunk == b('qwer'):
+                1 / 0
+
+        with ExceptionStackContext(error_handler):
+            self.fetch('/chunk', streaming_callback=streaming_cb)
+
+        self.assertEqual(chunks, [b('asdf'), b('qwer')])
+        self.assertEqual(1, len(exc_info))
+        self.assertIs(exc_info[0][0], ZeroDivisionError)
+
     def test_basic_auth(self):
         self.assertEqual(self.fetch("/auth", auth_username="Aladdin",
                                     auth_password="open sesame").body,
@@ -188,3 +209,43 @@ def test_types(self):
         self.assertEqual(type(response.headers["Content-Type"]), str)
         self.assertEqual(type(response.code), int)
         self.assertEqual(type(response.effective_url), str)
+
+    def test_header_callback(self):
+        first_line = []
+        headers = {}
+        chunks = []
+
+        def header_callback(header_line):
+            if header_line.startswith('HTTP/'):
+                first_line.append(header_line)
+            elif header_line != '\r\n':
+                k, v = header_line.split(':', 1)
+                headers[k] = v.strip()
+
+        def streaming_callback(chunk):
+            # All header callbacks are run before any streaming callbacks,
+            # so the header data is available to process the data as it
+            # comes in.
+            self.assertEqual(headers['Content-Type'], 'text/html; charset=UTF-8')
+            chunks.append(chunk)
+
+        self.fetch('/chunk', header_callback=header_callback,
+                   streaming_callback=streaming_callback)
+        self.assertEqual(len(first_line), 1)
+        self.assertRegexpMatches(first_line[0], 'HTTP/1.[01] 200 OK\r\n')
+        self.assertEqual(chunks, [b('asdf'), b('qwer')])
+
+    def test_header_callback_stack_context(self):
+        exc_info = []
+        def error_handler(typ, value, tb):
+            exc_info.append((typ, value, tb))
+            return True
+
+        def header_callback(header_line):
+            if header_line.startswith('Content-Type:'):
+                1 / 0
+
+        with ExceptionStackContext(error_handler):
+            self.fetch('/chunk', header_callback=header_callback)
+        self.assertEqual(len(exc_info), 1)
+        self.assertIs(exc_info[0][0], ZeroDivisionError)

website/sphinx/releases/next.rst

@@ -165,3 +165,10 @@ In progress
 * `tornado.auth.TwitterMixin` now works on Python 3.
 * ``Etag``/``If-None-Match`` requests now work with `StaticFileHandler`.
 * `StaticFileHandler` no longer sets ``Cache-Control: public`` unnecessarily.
+* The behavior of ``header_callback`` with `SimpleAsyncHTTPClient` has
+  changed and is now the same as that of `CurlAsyncHTTPClient`.  The
+  header callback now receives the first line of the response (e.g.
+  ``HTTP/1.0 200 OK``) and the final empty line.
+* Secondary `AsyncHTTPClient` callbacks (``streaming_callback``,
+  ``header_callback``, and ``prepare_curl_callback``) now respect
+  `StackContext`.