Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable_compression doesn't work with FileResponse in sendfile mode #2942

Closed
savv opened this issue Apr 17, 2018 · 6 comments · Fixed by #2944
Closed

enable_compression doesn't work with FileResponse in sendfile mode #2942

savv opened this issue Apr 17, 2018 · 6 comments · Fixed by #2944

Comments

@savv
Copy link

savv commented Apr 17, 2018

Long story short

response.enable_compression doesn't work on Heroku.

Expected behaviour

enable_compression on a response should work on Heroku.

Actual behaviour

While this worked locally just fine, Heroku didn't like it when I enabled compression on a FileResponse:
Apr 17 11:58:19 my-app-staging heroku/router: http_error="Bad chunk" at=error code=H17 desc="Poorly formatted HTTP response" method=GET path="/" host=my-app-staging.herokuapp.com request_id=0bc47133-af19-4999-bc0f-179054b7388a fwd="xx.xx.xx.xx" dyno=web.1 connect=0ms service=19ms status=503 bytes= protocol=https
Apr 17 11:58:19 my-app-staging heroku/router: http_error="Bad chunk" at=error code=H17 desc="Poorly formatted HTTP response" method=GET path="/" host=my-app-staging.herokuapp.com request_id=9c1b4583-61d5-4867-bf1a-75de194cf71f fwd="xx.xx.xx.xx" dyno=web.1 connect=0ms service=5ms status=503 bytes= protocol=https
Apr 17 11:58:23 my-app-staging heroku/router: http_error="Bad chunk" at=error code=H17 desc="Poorly formatted HTTP response" method=GET path="/" host=my-app-staging.herokuapp.com request_id=a2f80aee-0ba0-4117-9139-123a79fd2f1c fwd="xx.xx.xx.xx" dyno=web.1 connect=0ms service=3ms status=503 bytes= protocol=https

Steps to reproduce

        resp = web.FileResponse('./dist/index.html', headers={'Cache-Control': 'no-cache'})
        resp.enable_compression()
        return resp

Your environment

aiohttp==2.3.6
Using aiohttp directly on Heroku (no gunicorn/etc).

@asvetlov
Copy link
Member

Try to set AIOHTTP_NOSENDFILE=1 environment variable.

@savv
Copy link
Author

savv commented Apr 17, 2018

Works now, thanks. Is there a way to set this programmatically?

@asvetlov
Copy link
Member

asvetlov commented Apr 17, 2018

Not yet.
The problem is: aiohttp uses sendfile syscall even for compressed files.
The fast path should be disabled for non-identity content encoding.

The path for FileResponse is welcome (the required change is relatively simple).

@asvetlov asvetlov changed the title enable_compression doesn't work with heroku enable_compression doesn't work with FileResponse in sendfile mode Apr 17, 2018
@asvetlov
Copy link
Member

The issue is waiting for a champion.

@roganov
Copy link
Contributor

roganov commented Apr 17, 2018

I'll take this one.

@lock
Copy link

lock bot commented Oct 28, 2019

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

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants