Prevent Hub from having nil scope and client (#2402)

* Prevent Hub from having nil scope and client

`Hub#pop_scope` currently can cause a Hub to have a nil scope and client,
which would highly likely lead of errors. For example, `Hub#configuration`
would simply cause `NoMethodError`.

This commit prevents that from ever happening by making sure that `Hub#pop_scope`
would at least leave a brand new scope and the old client on the stack.

CHANGELOG.md

@@ -1,4 +1,4 @@
-### 5.20.0
+## 5.20.0
 
 - Add support for `$SENTRY_DEBUG` and `$SENTRY_SPOTLIGHT` ([#2374](https://github.com/getsentry/sentry-ruby/pull/2374))
 - Support human readable intervals in `sidekiq-cron` ([#2387](https://github.com/getsentry/sentry-ruby/pull/2387))
@@ -10,6 +10,7 @@
 - Fix error events missing a DSC when there's an active span ([#2408](https://github.com/getsentry/sentry-ruby/pull/2408))
 - Verifies presence of client before adding a breadcrumb ([#2394](https://github.com/getsentry/sentry-ruby/pull/2394))
 - Fix `Net:HTTP` integration for non-ASCII URI's ([#2417](https://github.com/getsentry/sentry-ruby/pull/2417))
+- Prevent Hub from having nil scope and client ([#2402](https://github.com/getsentry/sentry-ruby/pull/2402))
 
 ## 5.19.0
 

sentry-ruby/lib/sentry/hub.rb

@@ -73,7 +73,13 @@ def push_scope
     end
 
     def pop_scope
-      @stack.pop
+      if @stack.size > 1
+        @stack.pop
+      else
+        # We never want to enter a situation where we have no scope and no client
+        client = current_client
+        @stack = [Layer.new(client, Scope.new)]
+      end
     end
 
     def start_transaction(transaction: nil, custom_sampling_context: {}, instrumenter: :sentry, **options)

sentry-ruby/spec/sentry/hub_spec.rb

@@ -519,9 +519,25 @@
 
   describe "#pop_scope" do
     it "pops the current scope" do
+      prev_scope = subject.current_scope
+      subject.push_scope
+      scope = subject.current_scope
       expect(subject.current_scope).to eq(scope)
       subject.pop_scope
-      expect(subject.current_scope).to eq(nil)
+      expect(subject.current_scope).to eq(prev_scope)
+    end
+
+    it "doesn't pop the last layer" do
+      expect(subject.instance_variable_get(:@stack).count).to eq(1)
+
+      subject.pop_scope
+
+      expect(subject.instance_variable_get(:@stack).count).to eq(1)
+
+      # It should be a completely new scope
+      expect(subject.current_scope).not_to eq(scope)
+      # But it should be the same client
+      expect(subject.current_client).to eq(client)
     end
   end
 
@@ -543,21 +559,6 @@
       expect(subject.current_scope).not_to eq(scope)
       expect(subject.current_scope.tags).to eq({ foo: "bar" })
     end
-
-    context "when the current_scope is nil" do
-      before do
-        subject.pop_scope
-        expect(subject.current_scope).to eq(nil)
-      end
-      it "creates a new scope" do
-        scope.set_tags({ foo: "bar" })
-
-        subject.push_scope
-
-        expect(subject.current_scope).not_to eq(scope)
-        expect(subject.current_scope.tags).to eq({})
-      end
-    end
   end
 
   describe '#configure_scope' do