Skip to content

Conversation

@roback
Copy link
Member

@roback roback commented Nov 6, 2024

Added a Faraday middleware which streams the body, counting the number of bytes it receives. If the body exceeds the configured maximum size (using the max_response_body_size_bytes: argument), it raises an error.

close #21

Added a Faraday middleware which streams the body, counting the number
of bytes it receives. If the body exceeds the configured maximum size
(using the `max_response_body_size_bytes:` argument), it raises an
error.

close #21
@roback roback self-assigned this Nov 6, 2024
@roback
Copy link
Member Author

roback commented Nov 7, 2024

An example with the URL that caused me to take a look at this issue in the first place (https://groovefm.stream.laut.fm/groovefm?t302=2024-11-06_11-42-26&uuid=64608ac6-2e1d-4300-ab7f-024022b06701).

The URL above returns an audio stream that never ends, but if you configure the client with max_response_body_size_bytes, it will abort the stream after the configured number of bytes:

$ bundle exec ruby -rlogger -rtwingly/http -e 'logger = Logger.new($stdout); logger.level = :INFO; puts Twingly::HTTP::Client.new(logger: logger, base_user_agent: "").tap { |client| client.max_response_body_size_bytes = 1000 * 1000 }.get("https://groovefm.stream.laut.fm/groovefm?t302=2024-11-06_11-42-26&uuid=64608ac6-2e1d-4300-ab7f-024022b06701")'
I, [2024-11-07T08:27:54.053982 #62937]  INFO -- request: source=upstream-request method=GET url=https://groovefm.stream.laut.fm/groovefm?t302=2024-11-06_11-42-26&uuid=64608ac6-2e1d-4300-ab7f-024022b06701 request_id= release=unknown_heroku_release_version
/Users/me/repos/twingly-http/lib/twingly/http.rb:115:in `rescue in http_response_for': Response body too large, exceeced the configured max size of 1000000 bytes. (Twingly::HTTP::ResponseBodySizeLimitExceededError)
	from /Users/me/repos/twingly-http/lib/twingly/http.rb:101:in `http_response_for'
	from /Users/me/repos/twingly-http/lib/twingly/http.rb:62:in `get'
	from -e:1:in `<main>'
/Users/me/repos/twingly-http/lib/faraday/response_body_size_limit/middleware.rb:23:in `block in call': Response body too large, exceeced the configured max size of 1000000 bytes. (Faraday::ResponseBodySizeLimit::LimitExceededError)
	from /Users/me/.gem/ruby/3.2.2/gems/faraday-net_http-1.0.1/lib/faraday/adapter/net_http.rb:113:in `block in perform_request'
...

Copy link
Contributor

@Pontus4 Pontus4 left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@roback roback merged commit 42513d6 into master Nov 7, 2024
@roback roback deleted the issue/21/abort-request-if-body-is-too-large branch November 7, 2024 08:08
roback added a commit that referenced this pull request Nov 7, 2024
Because of the new feature in #32
roback added a commit that referenced this pull request Jan 2, 2025
When I added the middleware that checks the response body size in #32,
I introduced a bug where the response bodies for all requests in a
redirect "chain" gets included in the body returned in the `Twingly::HTTP::Response`
object.

What I did here was to move the middleware after the FollowRedirects middleware,
to ensure we run the middleware on each request, instead of handling all response
body chunks at once.

The test I added here (along with some other existing tests), would fail with the
following error before this fix:

    1) Twingly::HTTP::Client#post when given a host that redirects when following redirects only returns the body of the final request
      Failure/Error: expect(response.fetch(:body)).to eq("finished!")

        expected: "finished!"
              got: "redirect...finished!"

        (compared using ==)
      Shared Example Group: "common HTTP behaviour for" called from ./spec/lib/twingly/http_spec.rb:554
      # ./spec/lib/twingly/http_spec.rb:159:in `block (5 levels) in <top (required)>'
roback added a commit that referenced this pull request Jan 2, 2025
Before this it was registered as a request middleware. I didn't think
about that when I added it in #32.

This doesn't seem to make any difference, I just thought it makes sense
to change this anyway, as this middleware works with the response.
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.

Add capability to abort request when content-length is too large

2 participants