dwolla-v2-ruby icon indicating copy to clipboard operation
dwolla-v2-ruby copied to clipboard

Conflict with specifying custom middleware

Open javierjulio opened this issue 7 years ago • 2 comments

If I want to create custom middleware say for parsing Dwolla responses, I can but the way the following is structured I have to specify not just the default adapter line but also the response type, otherwise my middleware receives a string, not a hash. https://github.com/Dwolla/dwolla-v2-ruby/blob/4a01a1765fa3f39331fcdda58d7609e5f64a7337/lib/dwolla_v2/token.rb#L64-L65

I have a middleware to snake_case the keys in the response body and to get it to work I have to specify:

dwolla = DwollaV2::Client.new(...) do |config|
  config.faraday do |f|
    f.use UnderscoreResponseBodyKeys
    f.response :json, :content_type => /\bjson$/
    f.adapter Faraday.default_adapter
  end
end

I don't just have to specify the default adapter as mentioned but also the response type, otherwise my middleware will receive a string, not a hash. By having the response type after it works as expected and my middleware will receive a hash that DwollaV2 had parsed earlier in the stack.

I shouldn't have to specify the response type but it has to come before specifying the default adapter. At first I tried updating the Token class to have:

    def conn
      @conn ||= Faraday.new do |f|
        # ...
        client.faraday.call(f) if client.faraday
        f.response :json, :content_type => /\bjson$/
        f.adapter Faraday.default_adapter unless client.faraday
      end
    end

But I receive a warning here, since by just specifying the client.faraday block, the default adapter isn't set but it has to be the last item otherwise you receive a warning that this won't be supported:

WARNING: Unexpected middleware set after the adapter. This won't be supported from Faraday 1.0.

While I can temporarily specify the response type on my end I don't think the library should require that and I don't think that was the intention. But it seems you can get both specifying middleware or an adapter with the current configuration setup.

I would suggest breaking off the faraday adapter into its own block or config option that way devs can specify custom middleware while still retaining the defaults. I can submit a PR for that or another solution if you prefer something else?

javierjulio avatar Feb 19 '18 21:02 javierjulio

I had to change both the Token.conn and Client.conn methods with the following update:

    def conn
      @conn ||= Faraday.new do |f|
        # ...
        client.faraday.call(f) if client.faraday
        f.response :json, :content_type => /\bjson$/
        f.adapter Faraday.default_adapter
      end
    end

The response json middleware needs to come after any custom middleware but before the adapter, that hast to be last. If I remove the unless client.faraday (I realize why it was there hence the suggested solution in the description) then it works as expected, both with custom middleware such as:

dwolla = DwollaV2::Client.new(...) do |config|
  config.faraday do |f|
    f.use UnderscoreResponseBodyKeys
  end
end

Or if just using the client without any customizations the library parses responses successfully.

@sausman would you be open to possibly splitting out the option for setting faraday middleware and adapter? Or some other solution? I suggest the faraday adapter as separate config option as some of the libraries we use have it that way: twilio-ruby and https://github.com/amro/gibbon#other.

javierjulio avatar Feb 19 '18 21:02 javierjulio

I just ran into this issue FWIW. The workaround mentioned above got me going again.

    f.response :json, :content_type => /\bjson$/
    f.adapter Faraday.default_adapter

mculpsf avatar Feb 25 '20 00:02 mculpsf