lostisland / faraday

Simple, but flexible HTTP client library, with support for multiple backends.
https://lostisland.github.io/faraday
MIT License
5.72k stars 972 forks source link

Feature Request: Allows logging of errors that raised in adapter or other middleware #1460

Closed epaew closed 1 year ago

epaew commented 1 year ago

Description

This patch introduces logging of errors that raised in adapter or other (deeper) middlewares.

I am very much looking forward to merged this patch. Thanks.

Todos

List any remaining work that needs to be done, i.e:

Additional Notes

Optional section

epaew commented 1 year ago

@iMacTia Thank you for your review and comment!

The only thing I'd ask is to add one more piece to the docs (in this section) to illustrate how the configuration block now accepts also a error option 👍

I'm sorry but I didn't couldn't get which one you wanted.

  1. You can give :headers and :bodies option with a Hash which includes request: bool, response: bool, error: bool.

    conn = Faraday.new(url: 'http://httpbingo.org') do |faraday|
       faraday.response :logger, nil, { headers: { error: true }, bodies: { error: false } }
    end
  2. You can give :error option to the middleware to control logger output.

    conn = Faraday.new(url: 'http://httpbingo.org') do |faraday|
       faraday.response :logger, nil, { error: false }
    end

With my patches, give error: false (i.e. 2) to the option does not change the error log output.
Ofcource it is possible to apply additional patches to faraday/logging/formatter.rb to be able to control it.

 def error(error)
+  return unless @options[:error]
+
   ...
 end

Which one did you want?

iMacTia commented 1 year ago

Ah sorry, the link is not working as expected (probably because the line is outside of the changed code). I'm referring to this section:

### Include and exclude headers/bodies
By default, the `logger` middleware logs only headers for security reasons, however, you can configure it
to log bodies as well, or disable headers logging if you need to. To do so, simply provide a configuration hash
when you add the middleware to the stack:

conn = Faraday.new(url: 'http://httpbingo.org') do |faraday|
  faraday.response :logger, nil, { headers: true, bodies: true }
end

Please note this only works with the default formatter.

which should be updated to mention that errors will also be logged if you pass true. You also make a good point that we should probably illustrate the following syntax as well:

faraday.response :logger, nil, { headers: { error: true }, bodies: { error: false } }

but to be fair, we're not doing that for the current available options, so I won't consider this a blocker for this PR

epaew commented 1 year ago

I added commits. 👍