Skip to content

Commit b771787

Browse files
dblockCopilot
andcommitted
Expose response callbacks through request configuration blocks
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b4ac50a commit b771787

5 files changed

Lines changed: 161 additions & 2 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
### 3.3.0 (Next)
22

3+
* [#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).
34
* [#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).
45
* [#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).
56
* [#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).

‎README.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,26 @@ You can also pass request options, including `timeout` and `open_timeout` into i
301301
client.conversations_list(request: { timeout: 180 })
302302
```
303303

304+
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:
305+
306+
```ruby
307+
scopes = nil
308+
observed_client = client.with_request do |request|
309+
request.headers['X-Custom'] = 'value'
310+
end.with_response do |response|
311+
scopes = response.headers['x-oauth-scopes']&.split(',')&.map(&:strip)
312+
end
313+
314+
result = observed_client.auth_test
315+
observed_client.users_list do |page|
316+
puts page.members
317+
end
318+
```
319+
320+
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.
321+
322+
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.
323+
304324
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).
305325

306326
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`.

‎lib/slack/web/client.rb‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,23 @@ def initialize(options = {})
1414
Slack::Web::Config::ATTRIBUTES.each do |key|
1515
send("#{key}=", options.fetch(key, Slack::Web.config.send(key)))
1616
end
17+
1718
@logger ||= Slack::Config.logger || Slack::Logger.default
1819
@token ||= Slack.config.token
1920
end
2021

22+
def with_response(&block)
23+
raise ArgumentError, 'response callback block is required' unless block
24+
25+
with_callback(:@response_callbacks, block)
26+
end
27+
28+
def with_request(&block)
29+
raise ArgumentError, 'request callback block is required' unless block
30+
31+
with_callback(:@request_callbacks, block)
32+
end
33+
2134
class << self
2235
def configure
2336
block_given? ? yield(Config) : Config
@@ -27,6 +40,16 @@ def config
2740
Config
2841
end
2942
end
43+
44+
private
45+
46+
def with_callback(variable, block)
47+
copy = dup
48+
copy.instance_variable_set(:@connection, nil)
49+
copy.instance_variable_set(:@options, nil)
50+
copy.instance_variable_set(variable, Array(instance_variable_get(variable)) + [block])
51+
copy
52+
end
3053
end
3154
end
3255
end

‎lib/slack/web/faraday/request.rb‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,14 @@ def delete(path, options = {})
2222
private
2323

2424
def request(method, path, options)
25-
response = connection.send(method) do |request|
25+
response = perform_request(method, path, options)
26+
Array(@response_callbacks).each { |callback| callback.call(response) }
27+
response.body
28+
end
29+
30+
def perform_request(method, path, options)
31+
configuring_request = false
32+
connection.send(method) do |request|
2633
case method
2734
when :get, :delete
2835
request.url(path, options)
@@ -34,9 +41,13 @@ def request(method, path, options)
3441
request.headers['Authorization'] = "Bearer #{token}" if token
3542

3643
request.options.merge!(options.delete(:request)) if options.key?(:request)
44+
configuring_request = true
45+
Array(@request_callbacks).each { |callback| callback.call(request) }
46+
configuring_request = false
3747
end
38-
response.body
3948
rescue ::Faraday::ParsingError => e
49+
raise if configuring_request
50+
4051
raise Slack::Web::Api::Errors::ParsingError, e.response
4152
end
4253
end
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
# frozen_string_literal: true
2+
3+
require 'spec_helper'
4+
5+
RSpec.describe Slack::Web::Client do
6+
subject(:client) { described_class.new(token: 'example-token') }
7+
8+
before do
9+
stub_request(:post, 'https://slack.com/api/auth.test').to_return(
10+
body: '{"ok":true}',
11+
headers: { 'Content-Type' => 'application/json', 'X-OAuth-Scopes' => 'chat:write' }
12+
)
13+
end
14+
15+
it 'returns a configured copy without changing body returns' do
16+
responses = []
17+
observed = client.with_response { |response| responses << response }
18+
result = observed.auth_test
19+
expect(result).to be_a(Slack::Messages::Message)
20+
expect(responses.first.body).to equal(result)
21+
expect(responses.first.headers['x-oauth-scopes']).to eq('chat:write')
22+
expect(observed.token).to eq(client.token)
23+
client.auth_test
24+
expect(responses.size).to eq(1)
25+
end
26+
27+
it 'chains request and response callbacks in registration order' do
28+
calls = []
29+
observed = client.with_request do |request|
30+
calls << :request
31+
request.headers['X-Custom'] = 'value'
32+
end
33+
observed = observed.with_response do |response|
34+
calls << response.status
35+
end
36+
observed = observed.with_response do
37+
calls << :second
38+
end
39+
observed.auth_test
40+
expect(calls).to eq([:request, 200, :second])
41+
expect(a_request(:post, 'https://slack.com/api/auth.test')
42+
.with(headers: { 'X-Custom' => 'value', 'Authorization' => 'Bearer example-token' })).to have_been_made.once
43+
end
44+
45+
it 'requires callback blocks' do
46+
expect { client.with_response }.to raise_error(ArgumentError, 'response callback block is required')
47+
expect { client.with_request }.to raise_error(ArgumentError, 'request callback block is required')
48+
end
49+
50+
it 'observes every pagination request while preserving page blocks' do
51+
stub_request(:post, 'https://slack.com/api/users.list')
52+
.to_return(body: '{"ok":true,"members":[{"id":"U1"}],"response_metadata":{"next_cursor":"next"}}')
53+
.then.to_return(body: '{"ok":true,"members":[{"id":"U2"}],"response_metadata":{"next_cursor":""}}')
54+
requests = []
55+
responses = []
56+
pages = []
57+
observed = client.with_request { |request| requests << request }
58+
observed = observed.with_response { |response| responses << response }
59+
observed.users_list { |page| pages << page }
60+
expect(requests.size).to eq(2)
61+
expect(responses.map(&:body)).to eq(pages)
62+
expect(pages.map { |page| page.members.first.id }).to eq(%w[U1 U2])
63+
end
64+
65+
it 'does not share cached connections' do
66+
original_connection = client.send(:connection)
67+
observed = client.with_response { |_response| nil }
68+
expect(observed.send(:connection)).not_to equal(original_connection)
69+
end
70+
71+
it 'propagates response callback errors unchanged' do
72+
error = Faraday::ParsingError.new('callback error')
73+
observed = client.with_response { raise error }
74+
expect { observed.auth_test }.to(raise_error { |raised| expect(raised).to equal(error) })
75+
end
76+
77+
it 'does not dispatch an HTTP request when a request callback fails' do
78+
observed = client.with_request { raise ArgumentError, 'callback error' }
79+
expect { observed.auth_test }.to raise_error(ArgumentError, 'callback error')
80+
expect(a_request(:post, 'https://slack.com/api/auth.test')).not_to have_been_made
81+
end
82+
83+
it 'does not translate parsing errors raised by request callbacks' do
84+
error = Faraday::ParsingError.new('callback error')
85+
observed = client.with_request { raise error }
86+
expect { observed.auth_test }.to(raise_error { |raised| expect(raised).to equal(error) })
87+
end
88+
89+
it 'does not call response callbacks on Slack errors' do
90+
stub_request(:post, 'https://slack.com/api/auth.test').to_return(body: '{"ok":false,"error":"not_authed"}')
91+
responses = []
92+
observed = client.with_response { |response| responses << response }
93+
expect { observed.auth_test }.to raise_error(Slack::Web::Api::Errors::SlackError)
94+
expect(responses).to be_empty
95+
end
96+
97+
it 'does not call response callbacks on transport errors' do
98+
stub_request(:post, 'https://slack.com/api/auth.test').to_timeout
99+
responses = []
100+
observed = client.with_response { |response| responses << response }
101+
expect { observed.auth_test }.to raise_error(Slack::Web::Api::Errors::TimeoutError)
102+
expect(responses).to be_empty
103+
end
104+
end

0 commit comments

Comments
 (0)