Closed brandonrdn closed 2 years ago
We sure can, please contribute @brandonrdn !
I tried to have a go at this myself but the chain of gems that actually needs updating is pretty troublesome...
So, am wondering is it more important to be able to use danger in this repo than users to be able to use Faraday 2.0, which as @pboling commented "is far more consequential for the users of oauth2, so it takes precedence", at least until danger does support Faraday 2.0?
The easiest workaround is to move Danger into a separate danger/Gemfile and separate the task in GHA.
Ignoring danger for the moment, this is as far as I've got locally trying to get Faraday updated.
Code changes:
I've commented out the code that needs replacing rather than removing for the moment. Also ignore the debugger
, was trying to figure out why the parse error wasn't being thrown.
diff --git i/Gemfile w/Gemfile
index 4faec06..adf42a9 100644
--- i/Gemfile
+++ w/Gemfile
@@ -14,14 +14,16 @@ end
group :test do
gem 'activesupport'
+ gem 'byebug'
gem 'erubis'
+ gem 'faraday-typhoeus'
gem 'json-schema'
gem 'rake', '~> 13'
gem 'rspec'
gem 'rubocop', '~> 1.26.0'
gem 'rubocop-performance'
gem 'rubocop-rspec'
- gem 'slack-ruby-danger', '~> 0.2.0', require: false
+ # gem 'slack-ruby-danger', github: "schinery/danger" # '~> 0.2.0', require: false
gem 'timecop'
if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.0.0')
# https://github.com/vcr/vcr/pull/907
diff --git i/lib/slack-ruby-client.rb w/lib/slack-ruby-client.rb
index 54c351d..ee96afd 100644
--- i/lib/slack-ruby-client.rb
+++ w/lib/slack-ruby-client.rb
@@ -10,7 +10,9 @@ require_relative 'slack/messages/formatting'
# Web API
require 'faraday'
-require 'faraday_middleware'
+# require 'faraday_middleware'
+require 'faraday/mashify'
+require 'faraday/multipart'
require 'json'
require 'logger'
begin
diff --git i/lib/slack/web/faraday/connection.rb w/lib/slack/web/faraday/connection.rb
index 5f461b5..d89bfac 100644
--- i/lib/slack/web/faraday/connection.rb
+++ w/lib/slack/web/faraday/connection.rb
@@ -22,11 +22,15 @@ module Slack
options[:request] = request_options if request_options.any?
::Faraday::Connection.new(endpoint, options) do |connection|
- connection.use ::Faraday::Request::Multipart
- connection.use ::Faraday::Request::UrlEncoded
+ # connection.use ::Faraday::Request::Multipart
+ connection.request :multipart
+ # connection.use ::Faraday::Request::UrlEncoded
+ connection.request :url_encoded
connection.use ::Slack::Web::Faraday::Response::RaiseError
- connection.use ::FaradayMiddleware::Mashify, mash_class: Slack::Messages::Message
- connection.use ::FaradayMiddleware::ParseJson
+ # connection.use ::FaradayMiddleware::Mashify, mash_class: Slack::Messages::Message
+ connection.response :mashify, mash_class: Slack::Messages::Message
+ # connection.use ::FaradayMiddleware::ParseJson
+ connection.response :json
connection.use ::Slack::Web::Faraday::Response::WrapError
connection.response :logger, logger if logger
connection.adapter adapter
diff --git i/lib/slack/web/faraday/response/raise_error.rb w/lib/slack/web/faraday/response/raise_error.rb
index ce37685..8465ea2 100644
--- i/lib/slack/web/faraday/response/raise_error.rb
+++ w/lib/slack/web/faraday/response/raise_error.rb
@@ -3,7 +3,7 @@ module Slack
module Web
module Faraday
module Response
- class RaiseError < ::Faraday::Response::Middleware
+ class RaiseError < ::Faraday::Middleware
def on_complete(env)
raise Slack::Web::Api::Errors::TooManyRequestsError, env.response if env.status == 429
@@ -13,6 +13,7 @@ module Slack
return unless body
return if body['ok']
+ # debugger
error_message =
body['error'] || body['errors'].map { |message| message['error'] }.join(',')
diff --git i/lib/slack/web/faraday/response/wrap_error.rb w/lib/slack/web/faraday/response/wrap_error.rb
index 4082c14..b593cd6 100644
--- i/lib/slack/web/faraday/response/wrap_error.rb
+++ w/lib/slack/web/faraday/response/wrap_error.rb
@@ -3,7 +3,7 @@ module Slack
module Web
module Faraday
module Response
- class WrapError < ::Faraday::Response::Middleware
+ class WrapError < ::Faraday::Middleware
UNAVAILABLE_ERROR_STATUSES = (500..599).freeze
def on_complete(env)
diff --git i/slack-ruby-client.gemspec w/slack-ruby-client.gemspec
index b775ef3..f695835 100644
--- i/slack-ruby-client.gemspec
+++ w/slack-ruby-client.gemspec
@@ -10,14 +10,17 @@ Gem::Specification.new do |s|
s.authors = ['Daniel Doubrovkine']
s.email = 'dblock@dblock.org'
s.platform = Gem::Platform::RUBY
+ s.required_ruby_version = '>= 2.5'
s.required_rubygems_version = '>= 1.3.6'
s.files = `git ls-files`.split("\n")
s.require_paths = ['lib']
s.homepage = 'http://github.com/slack-ruby/slack-ruby-client'
s.licenses = ['MIT']
s.summary = 'Slack Web and RealTime API client.'
- s.add_dependency 'faraday', '>= 1.0'
- s.add_dependency 'faraday_middleware'
+ s.add_dependency 'faraday', '>= 2.0'
+ # s.add_dependency 'faraday_middleware'
+ s.add_dependency 'faraday-mashify'
+ s.add_dependency 'faraday-multipart'
s.add_dependency 'gli'
s.add_dependency 'hashie'
s.add_dependency 'websocket-driver'
diff --git i/spec/slack/web/client_spec.rb w/spec/slack/web/client_spec.rb
index 3068e41..b2edec0 100644
--- i/spec/slack/web/client_spec.rb
+++ w/spec/slack/web/client_spec.rb
@@ -1,5 +1,6 @@
# frozen_string_literal: true
require 'spec_helper'
+require 'faraday/typhoeus'
RSpec.describe Slack::Web::Client do
before do
diff --git i/spec/spec_helper.rb w/spec/spec_helper.rb
index f57133b..c8c5db9 100644
--- i/spec/spec_helper.rb
+++ w/spec/spec_helper.rb
@@ -3,6 +3,7 @@ $LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..'))
require 'rubygems'
require 'rspec'
+require 'byebug'
require 'timecop'
require 'slack_ruby_client'
Specs:
With the above changes I currently have 1 failing spec and a number of pending ones.
Pending: (Failures listed here are expected and do not affect your suite's status)
1) integration test gets hello
# missing SLACK_API_TOKEN and/or CONCURRENCY
# ./spec/integration/integration_spec.rb:113
2) integration test gets close, followed by closed
# missing SLACK_API_TOKEN and/or CONCURRENCY
# ./spec/integration/integration_spec.rb:189
3) integration test client connected responds to message
# missing SLACK_API_TOKEN and/or CONCURRENCY
# ./spec/integration/integration_spec.rb:84
4) integration test client connected sends message
# missing SLACK_API_TOKEN and/or CONCURRENCY
# ./spec/integration/integration_spec.rb:103
5) integration test with websocket_ping set sends pings
# missing SLACK_API_TOKEN and/or CONCURRENCY
# ./spec/integration/integration_spec.rb:128
6) integration test with websocket_ping set rebuilds the websocket connection when dropped
# missing SLACK_API_TOKEN and/or CONCURRENCY
# ./spec/integration/integration_spec.rb:142
7) integration test with websocket_ping not set does not send pings
# missing SLACK_API_TOKEN and/or CONCURRENCY
# ./spec/integration/integration_spec.rb:171
8) with CONCURRENCY detects concurrency
# missing CONCURRENCY
# ./spec/slack/real_time/concurrency/with_concurrency_spec.rb:7
Failures:
1) Slack::Web::Client global config server failures parsing error raises ParsingError
Failure/Error: expect { request }.to raise_error(Slack::Web::Api::Errors::ParsingError).with_message('parsing_error')
expected Slack::Web::Api::Errors::ParsingError with "parsing_error", got #<NoMethodError: undefined method `map' for nil:NilClass
body['error'] || body['errors'].map { |message| message['error'] }.join(',')
^^^^> with backtrace:
# ./lib/slack/web/faraday/response/raise_error.rb:18:in `on_complete'
# ./lib/slack/web/faraday/response/raise_error.rb:26:in `call'
# ./lib/slack/web/faraday/request.rb:25:in `request'
# ./lib/slack/web/faraday/request.rb:11:in `post'
# ./lib/slack/web/api/endpoints/api.rb:17:in `api_test'
# ./spec/slack/web/client_spec.rb:269:in `block (4 levels) in <top (required)>'
# ./spec/slack/web/client_spec.rb:282:in `block (6 levels) in <top (required)>'
# ./spec/slack/web/client_spec.rb:282:in `block (5 levels) in <top (required)>'
# ./spec/slack/web/client_spec.rb:282:in `block (5 levels) in <top (required)>'
Finished in 10.54 seconds (files took 1.93 seconds to load)
461 examples, 1 failure, 8 pending
Failed examples:
rspec ./spec/slack/web/client_spec.rb:281 # Slack::Web::Client global config server failures parsing error raises ParsingError
Also, this error was thrown while running the specs. Don't know if it is because I have something missing or what...
Slack::RealTime::Client
E, [2019-01-19T21:25:48.000000 #33521] ERROR -- id=T04KB5WQH, name=dblock, domain=dblockdotorg#run_handlers: ArgumentError (ArgumentError)
/Users/stu/Workspace/github/slack-ruby-client/spec/slack/real_time/event_handlers/event_handlers_spec.rb:13:in `block (3 levels) in <top (required)>'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-mocks-3.11.1/lib/rspec/mocks/message_expectation.rb:763:in `block in call'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-mocks-3.11.1/lib/rspec/mocks/message_expectation.rb:762:in `map'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-mocks-3.11.1/lib/rspec/mocks/message_expectation.rb:762:in `call'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-mocks-3.11.1/lib/rspec/mocks/message_expectation.rb:622:in `invoke_incrementing_actual_calls_by'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-mocks-3.11.1/lib/rspec/mocks/message_expectation.rb:475:in `invoke'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-mocks-3.11.1/lib/rspec/mocks/proxy.rb:217:in `message_received'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-mocks-3.11.1/lib/rspec/mocks/proxy.rb:366:in `message_received'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-mocks-3.11.1/lib/rspec/mocks/method_double.rb:80:in `proxy_method_invoked'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-mocks-3.11.1/lib/rspec/mocks/method_double.rb:64:in `block (2 levels) in define_proxy_method'
/Users/stu/Workspace/github/slack-ruby-client/lib/slack/real_time/stores/store.rb:429:in `block in <class:Store>'
/Users/stu/Workspace/github/slack-ruby-client/lib/slack/real_time/client.rb:256:in `instance_exec'
/Users/stu/Workspace/github/slack-ruby-client/lib/slack/real_time/client.rb:256:in `block in run_handlers'
/Users/stu/Workspace/github/slack-ruby-client/lib/slack/real_time/client.rb:255:in `each'
/Users/stu/Workspace/github/slack-ruby-client/lib/slack/real_time/client.rb:255:in `run_handlers'
/Users/stu/Workspace/github/slack-ruby-client/lib/slack/real_time/client.rb:244:in `dispatch'
/Users/stu/Workspace/github/slack-ruby-client/spec/slack/real_time/event_handlers/event_handlers_spec.rb:14:in `block (3 levels) in <top (required)>'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-expectations-3.11.0/lib/rspec/matchers/built_in/raise_error.rb:59:in `matches?'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-expectations-3.11.0/lib/rspec/matchers/built_in/raise_error.rb:81:in `does_not_match?'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-expectations-3.11.0/lib/rspec/expectations/handler.rb:90:in `does_not_match?'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-expectations-3.11.0/lib/rspec/expectations/handler.rb:79:in `block in handle_matcher'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-expectations-3.11.0/lib/rspec/expectations/handler.rb:27:in `with_matcher'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-expectations-3.11.0/lib/rspec/expectations/handler.rb:76:in `handle_matcher'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-expectations-3.11.0/lib/rspec/expectations/expectation_target.rb:78:in `not_to'
/Users/stu/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/rspec-expectations-3.11.0/lib/rspec/expectations/expectation_target.rb:144:in `not_to'
/Users/stu/Workspace/github/slack-ruby-client/spec/slack/real_time/event_handlers/event_handlers_spec.rb:14:in `block (2 levels) in <top (required)>'
...
is not fatal
Does this look like I'm on the right track with this and is there something I should/could be doing to get fix the error and to get the pending tests to run?
It's on the right track! Set export CONCURRENCY=async-websocket
and some of these will run. The rest are integration tests, and you need a Slack app (an API token). If you can PR a green passing version I can confirm that those work before merging.
@dblock I've put this PR up for you to have a look at. Definitely would be good to get some thoughts on the lib/slack/web/faraday/response/raise_error.rb
changes.
Also as mentioned in the PR, I tried running the specs with the CONCURRENCY and SLACK_API_TOKEN set but was having issues with the Slack token. The rest of the specs ran successfully though so 🤞🏻
Thanks @schinery for doing the work of upgrading Faraday! Closing via https://github.com/slack-ruby/slack-ruby-client/pull/406. If ya'll could try HEAD before we release that'd be grand.
For what it's worth, I tested it in my own application with Faraday 2 and found that the slack client is working for us.
Sorry @dblock I've only got around to testing this myself as been busy on other stuff.
Can also confirm that the Faraday upgrade works for me in one of the apps I've tested it in, which I tested using the following:
Gemfile:
gem "slack-ruby-client", github: "slack-ruby/slack-ruby-client", branch: "master" # "~> 1.0"
Code:
client = Slack::Web::Client.new
client.chat_postMessage(channel: "#qa", text: "Testing Slack client with Faraday 2", as_user: true)
Slack message:
So like @armilam, for me it is working. There were also no problems installing it with other gems that use Faraday such as Octokit.
Does this help with getting this released soon?
I released 1.1.0.
If someone could take care of https://github.com/slack-ruby/slack-ruby-client/issues/409 that'd be very helpful.
faraday_middleware
has been deprecated, withjson
andinstrumentation
middleware being bundled infaraday 2.0
, and other middleware are being re-released as independent gems. Sincefaraday_middleware
is never being updated to supportfaraday >= 2.0
, the currentslack-ruby-client
can never use it either. Is there any chance we can remove thefaraday_middleware
dependency and bump the minimum version offaraday
to2.0
?