Skip to content

Commit bb5dccc

Browse files
mabdinurYun-Kim
andauthored
fix(wsgi): ensure traced iterable does not implement __len__ (#5858)
Streaming responses with [waitress](https://github.com/Pylons/waitress) is not supported by the wsgi middleware. This incompatibility was introduced by the following commit: 7ce49bd. ## Reproduction 1. `pip install "waitress>=2.1.2" "ddtrace==1.13.0" "Flask>=2.2.4,<2.3.0"` 2. Create a flask application and add an endpoint that streams a response: app.py: ``` from waitress import serve from flask import Flask app = Flask(__name__) @app.route('/large.csv') def generate_large_csv(): def generate(): for row in [1, 2, 3, 4]: yield f"{row}\n" return app.response_class(generate(), mimetype='text/csv') @app.route("/") def hello(): return "Hello, World!" serve(app, listen='*:8080') ``` 3. Run ` python path/to/flask_app/app.py` 3. Run `curl http://localhost:8080/large.csv` 4. Error: ``` ERROR:waitress:Exception while serving /large.csv Traceback (most recent call last): File "/s1/remote-venvs/tesla/lib/python3.10/site-packages/waitress/channel.py", line 428, in service task.service() File "/s1/remote-venvs/tesla/lib/python3.10/site-packages/waitress/task.py", line 168, in service self.execute() File "/s1/remote-venvs/tesla/lib/python3.10/site-packages/waitress/task.py", line 466, in execute app_iter_len = len(app_iter) TypeError: object of type 'ClosingIterator' has no len() ``` ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] OPTIONAL: PR description includes explicit acknowledgement of the performance implications of the change as reported in the benchmarks PR comment. ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. --------- Co-authored-by: Yun Kim <[email protected]>
1 parent 6b359a4 commit bb5dccc

File tree

3 files changed

+33
-0
lines changed

3 files changed

+33
-0
lines changed

ddtrace/contrib/wsgi/wsgi.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,14 @@ def _finish_spans(self):
8888
self._self_parent_span.finish()
8989
self._self_span_finished = True
9090

91+
def __getattribute__(self, name):
92+
if name == "__len__":
93+
# __len__ is defined by the parent class, wrapt.ObjectProxy.
94+
# However this attribute should not be defined for iterables.
95+
# By definition, iterables should not support len(...).
96+
raise AttributeError("__len__ is not supported")
97+
return super(_TracedIterable, self).__getattribute__(name)
98+
9199

92100
class _DDWSGIMiddlewareBase(object):
93101
"""Base WSGI middleware class.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
wsgi: Resolves an issue where accessing the ``__len__`` attribute on traced wsgi middlewares raised a TypeError

tests/contrib/wsgi/test_wsgi.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,3 +308,24 @@ def test_distributed_tracing_nested():
308308
assert config.wsgi.distributed_tracing is True
309309
assert resp.status == "200 OK"
310310
assert resp.status_int == 200
311+
312+
313+
def test_wsgi_traced_iterable(tracer, test_spans):
314+
# Regression test to ensure wsgi iterable does not define an __len__ attribute
315+
middleware = wsgi.DDWSGIMiddleware(application)
316+
environ = {
317+
"PATH_INFO": "/chunked",
318+
"wsgi.url_scheme": "http",
319+
"SERVER_NAME": "localhost",
320+
"SERVER_PORT": "80",
321+
"REQUEST_METHOD": "GET",
322+
}
323+
324+
def start_response(status, headers, exc_info=None):
325+
pass
326+
327+
resp = middleware(environ, start_response)
328+
assert hasattr(resp, "__iter__")
329+
assert hasattr(resp, "close")
330+
assert hasattr(resp, "next") or hasattr(resp, "__next__")
331+
assert not hasattr(resp, "__len__"), "Iterables should not define __len__ attribute"

0 commit comments

Comments
 (0)