slack-ruby / slack-ruby-client

A Ruby and command-line client for the Slack Web, Real Time Messaging and Event APIs.
MIT License
1.21k stars 214 forks source link

Enable the logging of the request body #374

Open ts-3156 opened 3 years ago

ts-3156 commented 3 years ago

When debugging, I sometimes want to see the request body. I don't know if other people feel the same way. If you don't need it, please feel free to close this pull request.

dangerpr-bot commented 3 years ago
2 Warnings
:warning: There're library changes, but not tests. That's OK as long as you're refactoring existing code.
:warning: Unless you're refactoring existing code or improving documentation, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#374](https://github.com/slack-ruby/slack-ruby-client/pull/374): Enable the logging of the request body - [@ts-3156](https://github.com/ts-3156).

Generated by :no_entry_sign: Danger

dblock commented 3 years ago

I think we don't want this by default. Did you figure out a way to do this in your own code? I would like us to document how to do this without changing the default configuration, in README.

ts-3156 commented 3 years ago

@dblock Thank you for taking the time to reply.

I think we don't want this by default.

I have the same opinion. However, I have not come up with a suitable code to pass a value for bodies: true via Slack.configure.

Slack.configure do |config|
  config.log_bodies = true # <- This name feels strange.
end

Did you figure out a way to do this in your own code?

Yes. I found this option when I investigated strange behavior that Slack doesn't display the image preview correctly.

I would like us to document how to do this without changing the default configuration, in README.

If you need this option for logging, I will write how to do it in README. However, in order to use this option, I need to modify not just the documentation, but also the code. Is this really what you want?

You don't need to hurry. It is OK if this idea is discarded.

dblock commented 3 years ago

How about introducing logger_options?

If you feel more ambitious, take a look at https://lostisland.github.io/faraday/middleware/logger. It also supports blocks and filtering which we could expose. The https://github.com/ashkan18/graphlient gem does it like so:

 Graphlient::Client.new('http://test-graphql.biz/graphql') do |client|
      client.http do |h|
        h.connection do |c|
          c.adapter Faraday::Adapter::Rack, app
        end
      end
    end

So http and connection yield from within the configuration and you would want to be able to write the following, overriding the :logger part of the configuration.

 Slack.configure do |config|
   config.connection do |conn|
     conn.response :logger, logger, { bodies: true }
   end
 end
end