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

UnboundLocalError - chunk_size is not always set in middleware method #1335

Closed
CasperWA opened this issue Sep 19, 2022 · 7 comments · Fixed by #1336
Closed

UnboundLocalError - chunk_size is not always set in middleware method #1335

CasperWA opened this issue Sep 19, 2022 · 7 comments · Fixed by #1336
Assignees
Labels
bug Something isn't working priority/high Issue or PR with a consensus of high priority python Pull requests that update Python code server Issues pertaining to the example server implementation

Comments

@CasperWA
Copy link
Member

In the AddWarnings middleware, the chunk_size variable may not always be set, resulting in an UnboundLocalError exception being thrown.

The relevant lines of code can be found here.

The issue is (my guess), if the body is empty the for-loop is never entered, hence chunk_size is not set.
Solution: Any variables always reached should be preset at the beginning of functions/methods.

@CasperWA CasperWA added bug Something isn't working priority/high Issue or PR with a consensus of high priority server Issues pertaining to the example server implementation python Pull requests that update Python code labels Sep 19, 2022
@ml-evs ml-evs self-assigned this Sep 19, 2022
@ml-evs
Copy link
Member

ml-evs commented Sep 19, 2022

Written a few tests to try to trigger this, but even with an empty b"" response the first loop of response.body_iterator sets the chunk size appropriately. Do you know what triggered this for you? Obviously can just fix the code to set a default chunk_size to avoid this but just trying to understand

@CasperWA
Copy link
Member Author

Huh - you can see the error from a test in the gateway repository here.
I'm testing that the response from the OPTIMADE server will time out, essentially.

If it's not the content of the response.body_iterator, then I don't know?

@ml-evs
Copy link
Member

ml-evs commented Sep 19, 2022

Huh - you can see the error from a test in the gateway repository here. I'm testing that the response from the OPTIMADE server will time out, essentially.

If it's not the content of the response.body_iterator, then I don't know?

Right, hadn't appreciated the timeout aspect. Well #1336 should fix it either way, but probably not something we can test here

@ml-evs
Copy link
Member

ml-evs commented Sep 19, 2022

Just noticed that we are running into the exact same bug with FastAPI 0.85 here: #1334

@ml-evs
Copy link
Member

ml-evs commented Sep 19, 2022

Going to reopen this until I can verify that most responses still have an appropriate chunk size, not the default of 1 byte

@ml-evs ml-evs reopened this Sep 19, 2022
@ml-evs
Copy link
Member

ml-evs commented Sep 19, 2022

Going to reopen this until I can verify that most responses still have an appropriate chunk size, not the default of 1 byte

Seems fine! Tested by adding a breakpoint in the case of chunk_size = 0, launching the server then running the validator, and it wasn't triggered. Will close this and make a patch release. If I get time I'll play around with whichever test (looks like API hint tests) was actually failing in the FastAPI update PR but maybe not today!

@CasperWA
Copy link
Member Author

So very much appreciated! Thank you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority/high Issue or PR with a consensus of high priority python Pull requests that update Python code server Issues pertaining to the example server implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants