From 8a15de10b8c27e42e434f0e1f29c1821fc4da383 Mon Sep 17 00:00:00 2001 From: st0012 Date: Fri, 14 Jan 2022 12:09:23 +0000 Subject: [PATCH 1/3] Add specs for ActionCable's connection handling --- .../spec/sentry/rails/action_cable_spec.rb | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/sentry-rails/spec/sentry/rails/action_cable_spec.rb b/sentry-rails/spec/sentry/rails/action_cable_spec.rb index 2eba6db73..0c946f6af 100644 --- a/sentry-rails/spec/sentry/rails/action_cable_spec.rb +++ b/sentry-rails/spec/sentry/rails/action_cable_spec.rb @@ -29,6 +29,19 @@ def unsubscribed end end + class FailToOpenConnection < ActionCable::Connection::Base + def connect + raise "foo" + end + end + + class FailToCloseConnection < ActionCable::Connection::Base + def disconnect + raise "bar" + end + end + + RSpec.describe "Sentry::Rails::ActionCableExtensions", type: :channel do let(:transport) { Sentry.get_current_client.transport } @@ -36,6 +49,46 @@ def unsubscribed transport.events = [] end + describe "Connection" do + before do + make_basic_app + end + + let(:connection) do + env = Rack::MockRequest.env_for "/test", "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket", + "HTTP_HOST" => "localhost", "HTTP_ORIGIN" => "http://rubyonrails.com" + described_class.new(spy, env) + end + + before do + connection.process + end + + describe FailToOpenConnection do + it "captures errors happen when establishing connection" do + expect { connection.send(:handle_open) }.to raise_error(RuntimeError, "foo") + + expect(transport.events.count).to eq(1) + + event = transport.events.last.to_json_compatible + expect(event["transaction"]).to eq("FailToOpenConnection#connect") + end + end + + describe FailToCloseConnection do + it "captures errors happen when establishing connection" do + connection.send(:handle_open) + + expect { connection.send(:handle_close) }.to raise_error(RuntimeError, "bar") + + expect(transport.events.count).to eq(1) + + event = transport.events.last.to_json_compatible + expect(event["transaction"]).to eq("FailToCloseConnection#disconnect") + end + end + end + context "without tracing" do before do make_basic_app From fee75a4d3b80261b253707647b4241c4cff5eec4 Mon Sep 17 00:00:00 2001 From: st0012 Date: Fri, 14 Jan 2022 12:19:24 +0000 Subject: [PATCH 2/3] Add workaround for ActionCable's ConnectionStub ActionCable's ConnectionStub (for testing) doesn't implement the exact same interfaces as Connection::Base. One thing that's missing is `env`. So calling `connection.env` direclty will fail in test environments when `stub_connection` is used. See https://github.com/getsentry/sentry-ruby/pull/1684 for more information. --- sentry-rails/lib/sentry/rails/action_cable.rb | 15 ++++++++++----- .../spec/sentry/rails/action_cable_spec.rb | 9 --------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/sentry-rails/lib/sentry/rails/action_cable.rb b/sentry-rails/lib/sentry/rails/action_cable.rb index f588575d5..b253ac6f3 100644 --- a/sentry-rails/lib/sentry/rails/action_cable.rb +++ b/sentry-rails/lib/sentry/rails/action_cable.rb @@ -3,7 +3,12 @@ module Rails module ActionCableExtensions class ErrorHandler class << self - def capture(env, transaction_name:, extra_context: nil, &block) + def capture(connection, transaction_name:, extra_context: nil, &block) + # ActionCable's ConnectionStub (for testing) doesn't implement the exact same interfaces as Connection::Base. + # One thing that's missing is `env`. So calling `connection.env` direclty will fail in test environments when `stub_connection` is used. + # See https://github.com/getsentry/sentry-ruby/pull/1684 for more information. + env = connection.respond_to?(:env) ? connection.env : {} + Sentry.with_scope do |scope| scope.set_rack_env(env) scope.set_context("action_cable", extra_context) if extra_context @@ -43,13 +48,13 @@ module Connection private def handle_open - ErrorHandler.capture(env, transaction_name: "#{self.class.name}#connect") do + ErrorHandler.capture(self, transaction_name: "#{self.class.name}#connect") do super end end def handle_close - ErrorHandler.capture(env, transaction_name: "#{self.class.name}#disconnect") do + ErrorHandler.capture(self, transaction_name: "#{self.class.name}#disconnect") do super end end @@ -69,7 +74,7 @@ def self.included(base) def sentry_capture(hook, &block) extra_context = { params: params } - ErrorHandler.capture(connection.env, transaction_name: "#{self.class.name}##{hook}", extra_context: extra_context, &block) + ErrorHandler.capture(connection, transaction_name: "#{self.class.name}##{hook}", extra_context: extra_context, &block) end end @@ -79,7 +84,7 @@ module Actions def dispatch_action(action, data) extra_context = { params: params, data: data } - ErrorHandler.capture(connection.env, transaction_name: "#{self.class.name}##{action}", extra_context: extra_context) do + ErrorHandler.capture(connection, transaction_name: "#{self.class.name}##{action}", extra_context: extra_context) do super end end diff --git a/sentry-rails/spec/sentry/rails/action_cable_spec.rb b/sentry-rails/spec/sentry/rails/action_cable_spec.rb index 0c946f6af..0d82302c2 100644 --- a/sentry-rails/spec/sentry/rails/action_cable_spec.rb +++ b/sentry-rails/spec/sentry/rails/action_cable_spec.rb @@ -4,15 +4,6 @@ ::ActionCable.server.config.cable = { "adapter" => "test" } - # ensure we can access `connection.env` in tests like we can in production - ActiveSupport.on_load :action_cable_channel_test_case do - class ::ActionCable::Channel::ConnectionStub - def env - @_env ||= ::ActionCable::Connection::TestRequest.create.env - end - end - end - class ChatChannel < ::ActionCable::Channel::Base def subscribed raise "foo" From 849f7a67c62816f99e2b58d371488a63482b7da1 Mon Sep 17 00:00:00 2001 From: st0012 Date: Fri, 14 Jan 2022 12:28:50 +0000 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a000b7a4..d2167b342 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 4.9.1 + +### Bug Fixes + +- Add workaround for ConnectionStub's missing interface [#1686](https://github.com/getsentry/sentry-ruby/pull/1686) + - Fixes [#1685](https://github.com/getsentry/sentry-ruby/issues/1685) + ## 4.9.0 ### Features