Improve error message when body size of a request is unknown

[savonarola]

Hello!
Recently I tried to send an Enumerable object as a body for a request:

http.put(url, body: enumerable_instance)

This is a valid body according to the code: https://github.com/httprb/http/blob/master/lib/http/request/body.rb#L76

def validate_source_type!
  return if @source.is_a?(String)
  return if @source.respond_to?(:read)
  return if @source.is_a?(Enumerable)
  return if @source.nil?

  raise RequestError, "body of wrong type: #{@source.class}"
end

But the request failed with the exception cannot determine size of body: ...

Indeed, although Enumerable is a valid option for body, there is no corresponding case in #size function: https://github.com/httprb/http/blob/master/lib/http/request/body.rb#L17

def size
  if @source.is_a?(String)
    @source.bytesize
  elsif @source.respond_to?(:read)
    raise RequestError, "IO object must respond to #size" unless @source.respond_to?(:size)

    @source.size
  elsif @source.nil?
    0 
  else
    raise RequestError, "cannot determine size of body: #{@source.inspect}"
  end
end

It seemed a bit vague for me and took some time to figure out that in this case one should set Content-Length header explicitly so that Body#size is not called: https://github.com/httprb/http/blob/master/lib/http/request/writer.rb#L50

def add_body_type_headers
  return if @headers[Headers::CONTENT_LENGTH] || chunked?

  @request_header << "#{Headers::CONTENT_LENGTH}: #{@body.size}"
end

It seemed for me that it would be useful to have some hint on it in the error message.

[ixti]

IMO this details should not be in exception details. It belongs to documentation/wiki.

[ixti]

@httprb/core WDYT?

[ixti]

I think for bodies that do not support #size we should use (force?) chunked encoding instead. Otherwise we might end up with user setting Content-Length that is invalid (e.g. smaller or bigger than the actual size is).

[janko]

I think for bodies that do not support #size we should use (force?) chunked encoding instead.

👍The user should still be able to specify Content-Length, but if they haven't we should default to Transfer-Encoding: chunked.

On a related note, I also think that for IO-like objects we don't need to require #size.

http/lib/http/request/body.rb

Line 21 in 6240672

raise RequestError, "IO object must respond to #size" unless @source.respond_to?(:size)

[ixti]

@janko I agree.

[savonarola]

Hello!

Just wanted to clarify, is this issue still worth being opened? As far as I can see, it was automatically closed because of the main branch rename.

[ixti]

I think so.

…

On Tue, 14 Sep 2021, 13:14 Ilya Averyanov, ***@***.***> wrote: Hello! Just wanted to clarify, is this issue still worth being opened? As far as I can see, it was automatically closed because of the main branch rename. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#560 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAXEPRELL23SGYVVK5KRP3UB4U75ANCNFSM4IKKVJPQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[ixti]

Reopened as #689

[tarcieri]

Yep, sorry, pardon my dust