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

fix: Too much data for declared Content-Length error when endpoint caches enabled #92

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

matthewelwell
Copy link
Contributor

When using endpoint caches, for large payload sizes, the Edge Proxy would throw an exception but return a 200 OK response with an empty body. Looking in the logs, I determined the cause was

h11._util.LocalProtocolError: Too much data for declared Content-Length

When I compared a cached response with a non-cached response, I could see that the cached response was setting a Content-Length header about 1/4 of the size of the non-cached response.

I haven't investigated the cause of this too deeply as it seems to be buried deep in the internals of lru_cache and/or ORJSONResponse code. Instead, I have abstracted the serialization back to the main module and out of the environment service which, to me, feels cleaner anyway. It adds a small amount of processing back to the response generation, even when a cache hit is made, but in testing this seems negligible.

To test this fix, I attempted to write a test using the Fast API client, but I wasn't able to reproduce the error. Instead, I ran the edge proxy locally with the endpoint caches enabled and verified that multiple requests to the same endpoint, with the same data, work as expected.

@matthewelwell matthewelwell changed the title fix: fix Too much data for declared Content-Length error when endpoint caches enabled fix: Too much data for declared Content-Length error when endpoint caches enabled Feb 28, 2024
Copy link
Member

@gagantrivedi gagantrivedi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting

@matthewelwell matthewelwell merged commit 9422ef6 into main Feb 29, 2024
1 check passed
@matthewelwell matthewelwell deleted the fix/content-length-too-long-error branch February 29, 2024 09:29
matthewelwell added a commit that referenced this pull request Feb 29, 2024
…caches enabled (#92)

* Fix exception with cached responses

* Lint / format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants