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

Streaming response middleware executes twice #2197

Closed
ahopkins opened this issue Jul 14, 2021 · 3 comments
Closed

Streaming response middleware executes twice #2197

ahopkins opened this issue Jul 14, 2021 · 3 comments

Comments

@ahopkins
Copy link
Member

ahopkins commented Jul 14, 2021

Describe the bug
When responding with the response object from a streaming handler, the response middleware is executed twice.

Code snippet
Compare:

@app.get("/")
async def handler(request: Request):
    logger.info("handler")
    resp = await request.respond()
    logger.info("sending")
    await resp.send(b"Now I'm free, free-falling")
    logger.info("sent")
    await resp.send(b"Now I'm free, free-falling")
    logger.info("sent")
    await resp.eof()
    logger.info("done")
    return resp


@app.on_request
async def req(_):
    logger.info("request")


@app.on_response
async def res(*_):
    logger.info("response")

With:

@app.get("/")
async def handler(request: Request):
    logger.info("handler")
    resp = await request.respond()
    logger.info("sending")
    await resp.send(b"Now I'm free, free-falling")
    logger.info("sent")
    await resp.send(b"Now I'm free, free-falling")
    logger.info("sent")
    await resp.eof()
    logger.info("done")
    # return resp


@app.on_request
async def req(_):
    logger.info("request")


@app.on_response
async def res(*_):
    logger.info("response")
[2021-07-14 09:23:36 +0300] [1772072] [INFO] request
[2021-07-14 09:23:36 +0300] [1772072] [INFO] handler
[2021-07-14 09:23:36 +0300] [1772072] [INFO] response
[2021-07-14 09:23:36 +0300] [1772072] [INFO] sending
[2021-07-14 09:23:36 +0300] - (sanic.access)[INFO][127.0.0.1:48476]: GET http://localhost:9999/  200 26
[2021-07-14 09:23:36 +0300] [1772072] [INFO] sent
[2021-07-14 09:23:36 +0300] [1772072] [INFO] sent
[2021-07-14 09:23:36 +0300] [1772072] [INFO] done
[2021-07-14 09:23:36 +0300] [1772072] [INFO] response
---
[2021-07-14 09:24:04 +0300] [1772197] [INFO] request
[2021-07-14 09:24:04 +0300] [1772197] [INFO] handler
[2021-07-14 09:24:04 +0300] [1772197] [INFO] response
[2021-07-14 09:24:04 +0300] [1772197] [INFO] sending
[2021-07-14 09:24:04 +0300] - (sanic.access)[INFO][127.0.0.1:48484]: GET http://localhost:9999/  200 26
[2021-07-14 09:24:04 +0300] [1772197] [INFO] sent
[2021-07-14 09:24:04 +0300] [1772197] [INFO] sent
[2021-07-14 09:24:04 +0300] [1772197] [INFO] done

Expected behavior
Response middleware does not execute twice per streaming request.

There probably is still a use case for wanting some middleware type event at the conclusion of the handler, but this is can be covered by #1630 when #2160 is finally implemented.

Environment (please complete the following information):

  • Version main
@ahopkins ahopkins added the bug label Jul 14, 2021
@ahopkins
Copy link
Member Author

There are probably two ways to handle this:

  1. prevent handle_request from calling respond() a second time
  2. prevent respond from executing the middleware a second time

@ChihweiLHBird
Copy link
Member

Should has been fixed in #2327

@ahopkins
Copy link
Member Author

ahopkins commented Dec 9, 2021

Yes this is resolved with the new change. I will update the documentation about streaming to also let users know that they should NOT be returning an object from the handler when streaming.

@ahopkins ahopkins closed this as completed Dec 9, 2021
@ahopkins ahopkins added bug-fixed and removed bug labels Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants