StreamReaderPayload is broken

[asvetlov]

It works for sending data from request.content to response like return web.Response(body=request.content) but pretty useless for sending data from client HTTP request like

async def test_stream_reader(test_client):
    DATA = b'1234567890' * (2**16)
    async def h1(request):
        return web.Response(body=DATA)

    async def h2(request):
        async with aiohttp.ClientSession() as sess:
            async with sess.get(client.make_url('/h1')) as resp:
                return web.Response(body=resp.content)

    app = web.Application()
    app.router.add_get('/h1', h1)
    app.router.add_get('/h2', h2)

    client = await test_client(app)
    resp = await client.get('/h2')
    assert await resp.read() == DATA

Obviously resp.content from h2 web handler is closed before sending response body.

web.Response(body=request.content) is very rare usage -- usually web site does some processing of request BODY instead of just returning it back to client untouched.

I doubt if we need this payload writer at all.

@fafhrd91 what is the use case?

[fafhrd91]

Problem is not in StreamReaderPayload but in ClientSession. I think we discussed this in some issue.

Main reason was for proxy style endpoints. Actually because of client session you can not do proper streaming processing otherwise StreamReaderPayload should work

[asvetlov]

Do you have an evidence of real use case?

The problem not in client session but in ClientResponse -- the response is closed in web handler before starting StreamReadingPayload machinery. But the response should be closed on exit from web handler -- otherwise lifetime of such objects becomes too complicated. Transferring ownership from web-handler to StreamReaderPayload looks too error-prone.

For proxying the payload is useless: it could be safely applied to request.content only, all other streams are closed on exit from web-handler, isn't it?

[fafhrd91]

You can remove it

[asvetlov]

Fixed by #2558

[arthurdarcet]

@asvetlov I don't see how the "proxy" usecase works now, could you write an exemple?
Something to handle this kind of requests:

async def handle(request):
    async with aiohttp.ClientSession() as sess:
        async with sess.put('some external endpoint', data=request.content) as resp:
            pass
        return web.Response(body=b'ok')

request.content could be a very large body, and loading it in RAM first is not an option (and would be very inefficient anyway)

--

The mirrored usage is useful too, for instance an API service that is doing authentication but then forwarding data sent by an external service:

async def handle(request):
    resp = web.StreamResponse()
    await resp.prepare(request)
    async with aiohttp.ClientSession() as sess:
        async with sess.get('some external endpoint') as external:
            await resp.write(external.content)
    return resp

Here again, the external service could return very large amount of data, or could be streaming data slowly, and exposing properly that to the client is very useful

[asvetlov]

@arthurdarcet you are just in time :)
I've initiated a new aiohttp 3.1.1 release.
Please wait for release bot finish and run pip install -U aiohttp :)

[arthurdarcet]

Perfect, thanks.

[asvetlov]

Sure

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new