diff --git a/CHANGELOG.md b/CHANGELOG.md index 26162892..19a4a2b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ### 3.3.0 (Next) +* [#605](https://github.com/slack-ruby/slack-ruby-client/pull/605): Add configured client copies with request and response callbacks, including pagination - [@dblock](https://github.com/dblock). * [#603](https://github.com/slack-ruby/slack-ruby-client/pull/603): Enforce consistent exception messages with rubocop-exception_messages and run lint separately from the test matrix - [@dblock](https://github.com/dblock). * [#602](https://github.com/slack-ruby/slack-ruby-client/pull/602): Make gli an optional dependency; only the `slack` command-line client needs it - [@corsonknowles](https://github.com/corsonknowles). * [#599](https://github.com/slack-ruby/slack-ruby-client/pull/599): Set a default filename in `files_upload` so Slack displays image previews correctly when none is specified - [@ts-3156](https://github.com/ts-3156), [@dblock](https://github.com/dblock). diff --git a/README.md b/README.md index a34a4b3c..e7bc01cd 100644 --- a/README.md +++ b/README.md @@ -301,6 +301,26 @@ You can also pass request options, including `timeout` and `open_timeout` into i client.conversations_list(request: { timeout: 180 }) ``` +Use `with_request` and `with_response` to create a configured client copy with HTTP callbacks, without changing the original client or parsed-body return values: + +```ruby +scopes = nil +observed_client = client.with_request do |request| + request.headers['X-Custom'] = 'value' +end.with_response do |response| + scopes = response.headers['x-oauth-scopes']&.split(',')&.map(&:strip) +end + +result = observed_client.auth_test +observed_client.users_list do |page| + puts page.members +end +``` + +Request callbacks receive a `Faraday::Request` after authentication and request options are configured, before dispatch. They can change headers, body, and options. Response callbacks receive a `Faraday::Response` with the parsed body, headers, and status after successful completion; they do not run when Slack or transport errors are raised. Callbacks apply to each HTTP call, including pagination pages and requests made by helpers. Endpoint blocks retain their existing meaning. + +Chaining registers callbacks in order. Copies preserve client configuration but use independent connections and callback lists; the original client is unchanged. Callback exceptions propagate and request callback failures prevent dispatch. Callbacks are synchronous; avoid storing sensitive authorization headers, and synchronize any shared state if using a client concurrently. + You can control what proxy options are used by modifying the `http_proxy` environment variable per [Net::HTTP's documentation](https://docs.ruby-lang.org/en/2.0.0/Net/HTTP.html#class-Net::HTTP-label-Proxies). Note that Docker on OSX seems to incorrectly set the proxy, causing `Faraday::ConnectionFailed, ERROR -- : Failed to open TCP connection to : (getaddrinfo: Name or service not known)`. You might need to manually unset `http_proxy` in that case, eg. `http_proxy="" bundle exec ruby ./my_bot.rb`. diff --git a/lib/slack/web/client.rb b/lib/slack/web/client.rb index 3cde1c1e..c512606a 100644 --- a/lib/slack/web/client.rb +++ b/lib/slack/web/client.rb @@ -14,10 +14,23 @@ def initialize(options = {}) Slack::Web::Config::ATTRIBUTES.each do |key| send("#{key}=", options.fetch(key, Slack::Web.config.send(key))) end + @logger ||= Slack::Config.logger || Slack::Logger.default @token ||= Slack.config.token end + def with_response(&block) + raise ArgumentError, 'response callback block is required' unless block + + with_callback(:@response_callbacks, block) + end + + def with_request(&block) + raise ArgumentError, 'request callback block is required' unless block + + with_callback(:@request_callbacks, block) + end + class << self def configure block_given? ? yield(Config) : Config @@ -27,6 +40,16 @@ def config Config end end + + private + + def with_callback(variable, block) + copy = dup + copy.instance_variable_set(:@connection, nil) + copy.instance_variable_set(:@options, nil) + copy.instance_variable_set(variable, Array(instance_variable_get(variable)) + [block]) + copy + end end end end diff --git a/lib/slack/web/faraday/request.rb b/lib/slack/web/faraday/request.rb index f3c70b7c..95b305a3 100644 --- a/lib/slack/web/faraday/request.rb +++ b/lib/slack/web/faraday/request.rb @@ -22,7 +22,14 @@ def delete(path, options = {}) private def request(method, path, options) - response = connection.send(method) do |request| + response = perform_request(method, path, options) + Array(@response_callbacks).each { |callback| callback.call(response) } + response.body + end + + def perform_request(method, path, options) + configuring_request = false + connection.send(method) do |request| case method when :get, :delete request.url(path, options) @@ -34,9 +41,13 @@ def request(method, path, options) request.headers['Authorization'] = "Bearer #{token}" if token request.options.merge!(options.delete(:request)) if options.key?(:request) + configuring_request = true + Array(@request_callbacks).each { |callback| callback.call(request) } + configuring_request = false end - response.body rescue ::Faraday::ParsingError => e + raise if configuring_request + raise Slack::Web::Api::Errors::ParsingError, e.response end end diff --git a/spec/slack/web/client_callbacks_spec.rb b/spec/slack/web/client_callbacks_spec.rb new file mode 100644 index 00000000..220f0947 --- /dev/null +++ b/spec/slack/web/client_callbacks_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Slack::Web::Client do + subject(:client) { described_class.new(token: 'example-token') } + + before do + stub_request(:post, 'https://slack.com/api/auth.test').to_return( + body: '{"ok":true}', + headers: { 'Content-Type' => 'application/json', 'X-OAuth-Scopes' => 'chat:write' } + ) + end + + it 'returns a configured copy without changing body returns' do + responses = [] + observed = client.with_response { |response| responses << response } + result = observed.auth_test + expect(result).to be_a(Slack::Messages::Message) + expect(responses.first.body).to equal(result) + expect(responses.first.headers['x-oauth-scopes']).to eq('chat:write') + expect(observed.token).to eq(client.token) + client.auth_test + expect(responses.size).to eq(1) + end + + it 'chains request and response callbacks in registration order' do + calls = [] + observed = client.with_request do |request| + calls << :request + request.headers['X-Custom'] = 'value' + end + observed = observed.with_response do |response| + calls << response.status + end + observed = observed.with_response do + calls << :second + end + observed.auth_test + expect(calls).to eq([:request, 200, :second]) + expect(a_request(:post, 'https://slack.com/api/auth.test') + .with(headers: { 'X-Custom' => 'value', 'Authorization' => 'Bearer example-token' })).to have_been_made.once + end + + it 'requires callback blocks' do + expect { client.with_response }.to raise_error(ArgumentError, 'response callback block is required') + expect { client.with_request }.to raise_error(ArgumentError, 'request callback block is required') + end + + it 'observes every pagination request while preserving page blocks' do + stub_request(:post, 'https://slack.com/api/users.list') + .to_return(body: '{"ok":true,"members":[{"id":"U1"}],"response_metadata":{"next_cursor":"next"}}') + .then.to_return(body: '{"ok":true,"members":[{"id":"U2"}],"response_metadata":{"next_cursor":""}}') + requests = [] + responses = [] + pages = [] + observed = client.with_request { |request| requests << request } + observed = observed.with_response { |response| responses << response } + observed.users_list { |page| pages << page } + expect(requests.size).to eq(2) + expect(responses.map(&:body)).to eq(pages) + expect(pages.map { |page| page.members.first.id }).to eq(%w[U1 U2]) + end + + it 'does not share cached connections' do + original_connection = client.send(:connection) + observed = client.with_response { |_response| nil } + expect(observed.send(:connection)).not_to equal(original_connection) + end + + it 'propagates response callback errors unchanged' do + error = Faraday::ParsingError.new('callback error') + observed = client.with_response { raise error } + expect { observed.auth_test }.to(raise_error { |raised| expect(raised).to equal(error) }) + end + + it 'does not dispatch an HTTP request when a request callback fails' do + observed = client.with_request { raise ArgumentError, 'callback error' } + expect { observed.auth_test }.to raise_error(ArgumentError, 'callback error') + expect(a_request(:post, 'https://slack.com/api/auth.test')).not_to have_been_made + end + + it 'does not translate parsing errors raised by request callbacks' do + error = Faraday::ParsingError.new('callback error') + observed = client.with_request { raise error } + expect { observed.auth_test }.to(raise_error { |raised| expect(raised).to equal(error) }) + end + + it 'does not call response callbacks on Slack errors' do + stub_request(:post, 'https://slack.com/api/auth.test').to_return(body: '{"ok":false,"error":"not_authed"}') + responses = [] + observed = client.with_response { |response| responses << response } + expect { observed.auth_test }.to raise_error(Slack::Web::Api::Errors::SlackError) + expect(responses).to be_empty + end + + it 'does not call response callbacks on transport errors' do + stub_request(:post, 'https://slack.com/api/auth.test').to_timeout + responses = [] + observed = client.with_response { |response| responses << response } + expect { observed.auth_test }.to raise_error(Slack::Web::Api::Errors::TimeoutError) + expect(responses).to be_empty + end +end