Merge pull request #17 from square/lazarus/isolated-eval

isolate eval call so it can't muck with local variables

Author: nerdrew

.github/workflows/ruby.yml

@@ -18,7 +18,6 @@ permissions:
 
 jobs:
   test:
-
     runs-on: ubuntu-latest
     strategy:
       matrix:
@@ -27,10 +26,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Set up Ruby
-    # To automatically get bug fixes and new Ruby versions for ruby/setup-ruby,
-    # change this to (see https://github.com/ruby/setup-ruby#versioning):
-    # uses: ruby/setup-ruby@v1
-      uses: ruby/setup-ruby@55283cc23133118229fd3f97f9336ee23a179fcf # v1.146.0
+      uses: ruby/setup-ruby@v1
       with:
         ruby-version: ${{ matrix.ruby-version }}
         bundler-cache: true # runs 'bundle install' and caches installed gems automatically

.rubocop.yml

@@ -1,15 +1,25 @@
 AllCops:
-  TargetRubyVersion: 2.4
+  TargetRubyVersion: 3.1
   DisplayCopNames: true
+  NewCops: enable
 
-Style/FrozenStringLiteralComment:
-  Enabled: true
+plugins:
+  - rubocop-rake
+  - rubocop-rspec
 
+Layout/LineLength:
+  Enabled: false
 Metrics:
   Enabled: false
-
+RSpec/DescribedClass:
+  Enabled: false
+RSpec/ExampleLength:
+  Enabled: false
+RSpec/MultipleExpectations:
+  Enabled: false
 Style/Documentation:
   Enabled: false
-
+Style/FrozenStringLiteralComment:
+  Enabled: true
 Style/StringLiterals:
   EnforcedStyle: double_quotes

.travis.yml

@@ -1,8 +1,14 @@
 ### 0.1.9 2025-04-03
 
+- [#17](https://github.com/square/debug_socket/pull/17)
+  Refactor error handling. Previously all errors (including `eval` errors) were caught in the same `rescue Exception`.
+  Now, we only `rescue Exception` for `eval` errors. For `DebugSocket` errors, we only `rescue StandardError` and we
+  allow 20 consecutive errors before `DebugSocket` gives up and dies permanently.
+  (@nerdrew)
+
 - [#16](https://github.com/square/debug_socket/pull/16)
   Allow external auditing of debug sessions.
-  (dogor@)
+  (@doctown)
 
 ### 0.1.8 2022-10-10
 

CHANGES.md

@@ -5,7 +5,13 @@ source "https://rubygems.org"
 # Specify your gem's dependencies in debug_socket.gemspec
 gemspec
 
-gem "pry"
+gem "base64"
+gem "ostruct"
+gem "pry-byebug"
 gem "rake"
 gem "rspec", "~> 3.8"
-gem "rubocop", "0.59.1"
+gem "rubocop", "1.73.1"
+gem "rubocop-rake"
+gem "rubocop-rspec"
+gem "ruby-lsp", require: false
+gem "syntax_tree", require: false

Gemfile

@@ -17,4 +17,7 @@ Gem::Specification.new do |spec|
   spec.bindir        = "exe"
   spec.executables   = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
   spec.require_paths = ["lib"]
+  spec.metadata["rubygems_mfa_required"] = "true"
+
+  spec.required_ruby_version = ">= 3.1"
 end

debug_socket.gemspec

@@ -12,8 +12,10 @@ end
 socket_path = ARGV[0]
 command = ARGV[1] || "backtrace"
 
-warn "\nSending `#{command}` to the following socket: #{socket_path}"\
+warn(
+  "\nSending `#{command}` to the following socket: #{socket_path}" \
   "----------------------------------------------------------\n\n"
+)
 
 UNIXSocket.open(socket_path) do |socket|
   socket.write(command)

exe/debug-socket

@@ -5,6 +5,30 @@
 require "time"
 
 module DebugSocket
+  module Commands
+    # When running `eval`, we don't want the input to overwrite local variables etc. `eval` runs in the current scope,
+    # so we have an empty scope here that runs in a module that only has other shortcut commands the client might want
+    # to run.
+    def self.isolated_eval(input)
+      eval(input) # rubocop:disable Security/Eval
+    # We rescue Exception here because the input could have SyntaxErrors etc.
+    rescue Exception => e # rubocop:disable Lint/RescueException
+      DebugSocket.logger&.error { "debug-socket-error=#{e.inspect} input=#{input.inspect} path=#{@path} backtrace=#{e.backtrace.inspect}" }
+      "#{e.class.name}: #{e.message}\n#{e.backtrace.join("\n")}"
+    end
+
+    # Print the backtrace for every Thread
+    def self.backtrace
+      pid = Process.pid
+      "#{Time.now.utc.iso8601} #{$PROGRAM_NAME}\n" + Thread.list.map do |thread|
+        output = "#{Time.now.utc.iso8601} pid=#{pid} thread.object_id=#{thread.object_id} thread.status=#{thread.status}"
+        backtrace = thread.backtrace || []
+        output << "\n#{backtrace.join("\n")}" if backtrace
+        output
+      end.join("\n\n")
+    end
+  end
+
   @thread = nil
   @pid = Process.pid
 
@@ -16,7 +40,7 @@ def self.logger
     return @logger if defined?(@logger)
 
     require "logger"
-    @logger = Logger.new(STDERR)
+    @logger = Logger.new($stderr)
   end
 
   def self.start(path, &block)
@@ -32,22 +56,27 @@ def self.start(path, &block)
 
     server = UNIXServer.new(@path)
     @thread = Thread.new do
+      errors = 0
       loop do
-        begin
-          socket = server.accept
-          input = socket.read
-          logger&.warn("debug-socket-command=#{input.inspect}")
-
-          self.perform_audit(input, &block) if block
-
-          socket.puts(eval(input)) # rubocop:disable Security/Eval
-
-        rescue Exception => e # rubocop:disable Lint/RescueException
-          logger&.error { "debug-socket-error=#{e.inspect} backtrace=#{e.backtrace.inspect}" }
-        ensure
-          socket&.close
-        end
+        socket = server.accept
+        input = socket.read
+        logger&.warn("debug-socket-command=#{input.inspect}")
+
+        perform_audit(input, &block) if block
+        socket.puts(Commands.isolated_eval(input))
+
+        errors = 0
+      rescue StandardError => e
+        errors += 1
+        logger&.error { "debug-socket-error=#{e.inspect} errors=#{errors} path=#{@path} backtrace=#{e.backtrace.inspect}" }
+        raise e if errors > 20
+
+        sleep(1)
+      ensure
+        socket&.close
       end
+    rescue Exception => e # rubocop:disable Lint/RescueException
+      logger&.error { "debug-socket-error=#{e.inspect} DebugSocket is broken now path=#{@path} backtrace=#{e.backtrace.inspect}" }
     end
 
     logger&.unknown { "Debug socket listening on #{@path}" }
@@ -65,21 +94,10 @@ def self.stop
     @path = nil
   end
 
-  def self.backtrace
-    pid = Process.pid
-    "#{Time.now.utc.iso8601} #{$PROGRAM_NAME}\n" + Thread.list.map do |thread|
-      output =
-        +"#{Time.now.utc.iso8601} pid=#{pid} thread.object_id=#{thread.object_id} thread.status=#{thread.status}"
-      backtrace = thread.backtrace || []
-      output << "\n#{backtrace.join("\n")}" if backtrace
-      output
-    end.join("\n\n")
-  end
-
   # Allow debug socket input commands to be audited by an external callback
   private_class_method def self.perform_audit(input)
     yield input
-  rescue Exception => e
-    logger&.error "debug-socket-error=callback unsuccessful: #{e.inspect} for #{input.inspect} socket_path=#{@path}"
+  rescue Exception => e # rubocop:disable Lint/RescueException
+    logger&.error "debug-socket-error=callback unsuccessful: #{e.inspect} for #{input.inspect} path=#{@path} backtrace=#{e.backtrace.inspect}"
   end
 end