Have `HTTP::Client.get(path)` make the request object available?

I’m looking at 1.7.3’s HTTP::Client and I was wondering if this change could be made? Not to one function, probably to most of the functions in the file.

   def exec(method : String, path, headers : HTTP::Headers? = nil, body : BodyType = nil)
-    exec(new_request(method, path, headers, body)) do |response|
-       yield response, request
-     end
+    request = new_request(method, path, headers, body)
+    exec(request) do |response|
+       yield response, request
+     end

I have a scenario where I’d like to do some stuff where I’m able to use the request object in some of my error reporting

request = HTTP::Request.new("GET", "/api/example-endpoint")
response = client.exec request
ok_or_raise request, response
# do other stuff

and it would be nicer to write this as:

client.get "/api/example-endpoint" do |response, request|
    ok_or_raise response, request
    # do other stuff

both samples using this macro:

macro ok_or_raise(request, response)
  unless {{response.id}}.success?
    %msg = "#{{{response.id}}.status_code} #{{{response.id}}.status} #{{{request.id}}.path} "
    Log.error { "Unexpected status code: #{{{response.id}}.status_code}" }
    Log.error { %msg }
    Log.error { {{response.id}}.body }
    raise Exception.new(%msg)

the order of |response, request| is backwards from what most people are use to in this example, I only have it that way for backwards compatibility. Also, all my samples are a rough draft, I barely know this language and never used ruby, so maybe there is some obvious utility I’m not aware exists.

If all you’re wanting the request object for is the path, you could use either version just doing like:

response = client.get path = "/api/example-endpoint"
ok_or_raise response, path


client.get path = "/api/example-endpoint" do |response|
    ok_or_raise response, path
    # do other stuff

Not directly related, and I’m not sure if it makes a difference for you, but the yielding and non-yielding variants of #exec (and the other request methods) are not semantically identical. The yielding variant provides streaming access to response body such that you could use it w/o loading the entire body into memory while the non-yielding variant does just that.

EDIT: Maybe another option would be to have the option to raise on failed requests?
EDIT2: Related: Redesign HTTP::Client to implement missing features · Issue #6011 · crystal-lang/crystal · GitHub

FWIW I’m not currently trying to use the streaming versions of HTTP.

but something like this would be good enough for me:

class HTTP::Client::RaisedResponse < Exception
    getter request : HTTP::Request
    getter response : HTTP::Client::Response
    getter message : String?
    def initialize(@request, @response)
        @message = "Unexpected response status code: #{@response.status_code}"

client = HTTP::Client.new()
client.raise_unless_status_codes= 200..204
# might raise
response = client.get("/api/example-endpoint")


client = HTTP::Client.new()
# custom variant
# I 
client.add_response_hander { |request, response|
  unless response.success?
     raise HTTP::Client::RaisedResponse.new(request, response)
# or convenient built-in
client.add_response_hander HTTP::Client::Handler::SuccessOrRaise.new
# might raise
response = client.get("/api/example-endpoint")

This is possible if you wrap or subclass HTTP::Client. For example, subclassing it to add response handlers:

require "http"

class MyClient < HTTP::Client
  alias HandlerProc = HTTP::Request, HTTP::Client::Response -> Nil
  @response_handlers = [] of Handler | HandlerProc

  def add_response_handler(handler : Handler)
    @response_handlers << handler

  def add_response_handler(&handler : HandlerProc)
    @response_handlers << handler

  def exec_internal(request)
    response = super
    @response_handlers.each(&.call(request, response))

  module Handler
    abstract def call(request : HTTP::Request, response : HTTP::Client::Response)

struct SuccessOrRaise
  include MyClient::Handler

  def call(request, response)
    raise Failure.new("Response was unsuccessful", response) unless (200...400).includes? response.status_code

  class Failure < Exception
    getter response : HTTP::Client::Response
    def initialize(message, @response)
      super message

client = MyClient.new(URI.parse("https://zomglol.wtf"))
client.add_response_handler do |request, response|
  pp response.status
client.add_response_handler SuccessOrRaise.new

client.get "/@jamie"

I don’t think it makes sense for HTTP::Client itself to include configuration for raising exceptions or anything like that itself, though. HTTP is just a protocol and error responses are not exceptional (they happen all the time). How those responses are handled is a concern for what’s running on top of that protocol.

With that said, it might make sense to add callbacks similar to HTTP::Client#before_request that runs after you get the response back but before it returns it to the caller. The before_request callback is great for adding headers (it’s used by OAuth2 to add access tokens to requests made by that client, for example), and something like after_request that yields the request/response pair could be useful for instrumentation.