Skip to content

Conversation

@ts-3156
Copy link

@ts-3156 ts-3156 commented May 6, 2021

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
Copy link

2 Warnings
⚠️ There're library changes, but not tests. That's OK as long as you're refactoring existing code.
⚠️ 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 🚫 Danger

@dblock
Copy link
Collaborator

dblock commented May 6, 2021

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
Copy link
Author

ts-3156 commented May 7, 2021

@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
Copy link
Collaborator

dblock commented May 7, 2021

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
@dblock dblock force-pushed the master branch 4 times, most recently from 1963b52 to 097b3ca Compare April 30, 2023 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants