From 7b7cfef70893002a74ad7e5692998572742bbdbb Mon Sep 17 00:00:00 2001 From: Tom Most Date: Tue, 29 Sep 2020 22:52:25 -0700 Subject: [PATCH 1/5] Improve error message for short/long file tuples If passed a tuple with a length other than 2 or 3 IBodyProducer would end up trying to adapt None, producing this confusing message: Traceback (most recent call last): File ".../treq/test/test_client.py", line 513, in test_request_files_bad_tuples self.client.request( File ".../treq/client.py", line 190, in request bodyProducer, contentType = self._request_body( File ".../treq/client.py", line 297, in _request_body files = list(_convert_files(files)) File ".../treq/client.py", line 388, in _convert_files yield (param, (file_name, content_type, IBodyProducer(fobj))) builtins.TypeError: ('Could not adapt', None, ) This changeset produces a TypeError that mentions tuple length. --- src/treq/client.py | 12 ++++++++++++ src/treq/test/test_client.py | 26 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/treq/client.py b/src/treq/client.py index 4c13b3a9..c061f81a 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -298,6 +298,17 @@ def _convert_files(files): file_name, fobj = val elif len(val) == 3: file_name, content_type, fobj = val + else: + # NB: This is TypeError for backward compatibility. This case + # used to fall through to `IBodyProducer`, below, which raised + # TypeError about being unable to coerce None. + raise TypeError( + ( + "Files must given as tuples of the form (file_name, file_obj)" + " or (file_name, content_type, file_obj), but the {!r} tuple" + " has length {}: {!r}" + ).format(param, len(val), val), + ) else: fobj = val if hasattr(fobj, "name"): @@ -306,6 +317,7 @@ def _convert_files(files): if not content_type: content_type = _guess_content_type(file_name) + # XXX: Shouldn't this call self._data_to_body_producer? yield (param, (file_name, content_type, IBodyProducer(fobj))) diff --git a/src/treq/test/test_client.py b/src/treq/test/test_client.py index 1334fc7a..f7f42102 100644 --- a/src/treq/test/test_client.py +++ b/src/treq/test/test_client.py @@ -397,6 +397,32 @@ def test_request_named_attachment_and_ctype(self): boundary=b'heyDavid'), self.MultiPartProducer.call_args) + def test_request_files_tuple_too_short(self): + """ + The `HTTPClient.request()` *files* argument requires tuples of length + 2 or 3. It raises `TypeError` when the tuple is too short. + """ + with self.assertRaises(TypeError) as c: + self.client.request("POST", b"http://example.com/", files=[("t1", ("foo.txt",))]) + + self.assertIn("'t1' tuple has length 1", str(c.exception)) + + def test_request_files_tuple_too_long(self): + """ + The `HTTPClient.request()` *files* argument requires tuples of length + 2 or 3. It raises `TypeError` when the tuple is too long. + """ + with self.assertRaises(TypeError) as c: + self.client.request( + "POST", + b"http://example.com/", + files=[ + ("t4", ("foo.txt", "text/plain", BytesIO(b"...\n"), "extra!")), + ], + ) + + self.assertIn("'t4' tuple has length 4", str(c.exception)) + @mock.patch('treq.client.uuid.uuid4', mock.Mock(return_value="heyDavid")) def test_request_mixed_params(self): From 5127428ea998ca418674e313285591744a0a813d Mon Sep 17 00:00:00 2001 From: Tom Most Date: Tue, 29 Sep 2020 22:58:43 -0700 Subject: [PATCH 2/5] Update changelog --- changelog.d/299.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/299.feature.rst diff --git a/changelog.d/299.feature.rst b/changelog.d/299.feature.rst new file mode 100644 index 00000000..c80d711c --- /dev/null +++ b/changelog.d/299.feature.rst @@ -0,0 +1 @@ +treq produces a more helpful exception when passed a tuple of the wrong size in the *files* parameter. From a613b0cad328e0c50837d821115391601da76a57 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Sat, 26 Dec 2020 23:37:32 -0800 Subject: [PATCH 3/5] Wrap long line --- src/treq/test/test_client.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/treq/test/test_client.py b/src/treq/test/test_client.py index ac7576e4..571353b5 100644 --- a/src/treq/test/test_client.py +++ b/src/treq/test/test_client.py @@ -404,7 +404,11 @@ def test_request_files_tuple_too_short(self): 2 or 3. It raises `TypeError` when the tuple is too short. """ with self.assertRaises(TypeError) as c: - self.client.request("POST", b"http://example.com/", files=[("t1", ("foo.txt",))]) + self.client.request( + "POST", + b"http://example.com/", + files=[("t1", ("foo.txt",))], + ) self.assertIn("'t1' tuple has length 1", str(c.exception)) From 13861690b12e287c72c50174f942844cbb0debe6 Mon Sep 17 00:00:00 2001 From: Tom Most Date: Mon, 28 Dec 2020 00:04:03 -0800 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Adi Roiban --- src/treq/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/treq/client.py b/src/treq/client.py index 4b79a64b..9095680b 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -400,7 +400,7 @@ def _convert_files(files): # TypeError about being unable to coerce None. raise TypeError( ( - "Files must given as tuples of the form (file_name, file_obj)" + "`files` argument must be a sequence of tuples of (file_name, file_obj)" " or (file_name, content_type, file_obj), but the {!r} tuple" " has length {}: {!r}" ).format(param, len(val), val), From ebe28ff075a5217f44b28b9cc01adaada0f473ca Mon Sep 17 00:00:00 2001 From: Tom Most Date: Mon, 28 Dec 2020 14:15:33 -0800 Subject: [PATCH 5/5] Wrap long line --- src/treq/client.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/treq/client.py b/src/treq/client.py index 9095680b..509160b0 100644 --- a/src/treq/client.py +++ b/src/treq/client.py @@ -400,9 +400,10 @@ def _convert_files(files): # TypeError about being unable to coerce None. raise TypeError( ( - "`files` argument must be a sequence of tuples of (file_name, file_obj)" - " or (file_name, content_type, file_obj), but the {!r} tuple" - " has length {}: {!r}" + "`files` argument must be a sequence of tuples of" + " (file_name, file_obj) or" + " (file_name, content_type, file_obj)," + " but the {!r} tuple has length {}: {!r}" ).format(param, len(val), val), ) else: