diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 251aead..a10d418 100644 --- a/.github/workflows/ruby.yml +++ b/.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 diff --git a/.rubocop.yml b/.rubocop.yml index 4c163fc..ed71815 100644 --- a/.rubocop.yml +++ b/.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 diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index ff37a0b..0000000 --- a/.travis.yml +++ /dev/null @@ -1,11 +0,0 @@ -language: ruby -rvm: - - 2.3.4 - - 2.4.1 - - jruby-9.1.13.0 - - ruby-head -matrix: - allow_failures: - - rvm: ruby-head - fast_finish: true -sudo: false diff --git a/CHANGES.md b/CHANGES.md index 3ef5aa7..4693a67 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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 diff --git a/Gemfile b/Gemfile index fcee18a..ef32ea9 100644 --- a/Gemfile +++ b/Gemfile @@ -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 diff --git a/debug_socket.gemspec b/debug_socket.gemspec index 1069519..aeb911b 100644 --- a/debug_socket.gemspec +++ b/debug_socket.gemspec @@ -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 diff --git a/exe/debug-socket b/exe/debug-socket index 5f7bc35..10bed4e 100755 --- a/exe/debug-socket +++ b/exe/debug-socket @@ -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) diff --git a/lib/debug_socket.rb b/lib/debug_socket.rb index 75a830f..21f2e1f 100644 --- a/lib/debug_socket.rb +++ b/lib/debug_socket.rb @@ -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 diff --git a/spec/debug_socket_spec.rb b/spec/debug_socket_spec.rb index 90d0365..26f4fc1 100644 --- a/spec/debug_socket_spec.rb +++ b/spec/debug_socket_spec.rb @@ -22,6 +22,20 @@ DebugSocket.start(path) end + def write(str, socket_path = path) + 20.times { sleep(0.1) unless File.exist?(socket_path) } + socket = UNIXSocket.new(socket_path) + + raise "Socket write timeout=#{socket_path}" unless socket.wait_writable(1) + + socket.write(str) + socket.close_write + + raise "Socket read timeout=#{socket_path}" unless socket.wait_readable(1) + + socket.read + end + after do DebugSocket.stop 10.times { sleep(1) if File.exist?(path) } @@ -29,9 +43,7 @@ end it "logs and evals input" do - socket.write("2 + 2") - socket.close_write - expect(socket.read).to eq("4\n") + expect(write("2 + 2")).to eq("4\n") expect(log_buffer.string).to include('debug-socket-command="2 + 2"') end @@ -55,38 +67,62 @@ expect { DebugSocket.start(another_path) } .to raise_exception("debug socket thread already running for this process") - 10.times { sleep(1) unless File.exist?(another_path) } - another_socket = UNIXSocket.new(another_path) - another_socket.write("Thread.list.each(&:wakeup)") - another_socket.close_write - expect(another_socket.read).to match(/Thread/) + expect(write("Thread.list.each(&:wakeup)", another_path)).to match(/Thread/) Process.wait(child, Process::WNOHANG) else - DebugSocket.start(another_path) - sleep - sleep(1) - DebugSocket.stop - exit!(1) + begin + DebugSocket.start(another_path) + sleep + sleep(1) + DebugSocket.stop + ensure + exit!(1) + end end end end it "catches errors in the debug socket thread" do - socket.write("asdf}(]") - socket.close_write - expect(socket.read).to eq("") + expect(write("asdf}(]")).to include("SyntaxError") + expect(write("2")).to eq("2\n") - another_socket = UNIXSocket.new(path) - another_socket.write("2") - another_socket.close_write - expect(another_socket.read).to eq("2\n") - - expect(log_buffer.string).to include("debug-socket-error=# for "3 + 3"') end end - end - describe ".backtrace" do - it "returns a stacktrace for all threads" do - time_pid = "\\d{4}-\\d\\d-\\d\\dT\\d\\d:\\d\\d:\\d\\dZ\\ pid=#{Process.pid}" - running_thread = %r{#{time_pid}\ thread\.object_id=#{Thread.current.object_id}\ thread\.status=run\n - .*lib/debug_socket\.rb:\d+:in\ `backtrace'\n - .*lib/debug_socket\.rb:\d+:in\ `block\ in\ backtrace'\n - .*lib/debug_socket\.rb:\d+:in\ `map'\n - .*lib/debug_socket\.rb:\d+:in\ `backtrace'\n - .*spec/debug_socket_spec\.rb:\d+:in\ `block.*'}x - thread = Thread.new { sleep 1 } - sleep 0.1 - sleeping_thread = %r{#{time_pid}\ thread\.object_id=#{thread.object_id}\ thread\.status=sleep\n - .*spec/debug_socket_spec\.rb:\d+:in\ `sleep'\n - .*spec/debug_socket_spec\.rb:\d+:in\ `block.*'}x - bt = DebugSocket.backtrace - expect(bt).to match(running_thread) - expect(bt).to match(sleeping_thread) - end - end - - describe "stress test", slow: true do - it "works with lots of threads, even in jruby" do - threads = Array.new(10) do - Thread.new { 100.times { Thread.new { sleep(0.001) }.join } } + describe "Commands.backtrace" do + it "returns a stacktrace for all threads" do + time_pid = "\\d{4}-\\d\\d-\\d\\dT\\d\\d:\\d\\d:\\d\\dZ\\ pid=#{Process.pid}" + running_thread = %r{#{time_pid}\ thread\.object_id=\d+\ thread\.status=run\n + .*lib/debug_socket\.rb:\d+:in\ .*backtrace'\n + .*lib/debug_socket\.rb:\d+:in\ .*block\ in.*backtrace'\n + .*lib/debug_socket\.rb:\d+:in\ .*map'\n + .*lib/debug_socket\.rb:\d+:in\ .*backtrace'}x + thread = Thread.new { sleep 1 } + sleep 0.1 + sleeping_thread = %r{#{time_pid}\ thread\.object_id=#{thread.object_id}\ thread\.status=sleep\n + .*spec/debug_socket_spec\.rb:\d+:in\ .*sleep'\n + .*spec/debug_socket_spec\.rb:\d+:in\ .*block.*'}x + bt = write("backtrace") + expect(bt).to match(running_thread) + expect(bt).to match(sleeping_thread) end - - expect do - DebugSocket.backtrace while threads.any?(&:alive?) - threads.join - threads.map(&:value) - end.not_to raise_exception end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c4963ee..4cc4dd8 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,4 +1,18 @@ # frozen_string_literal: true -$LOAD_PATH.unshift File.expand_path("../lib", __dir__) +require "bundler/setup" require "debug_socket" +require "io/wait" +require "pry-byebug" + +RSpec.configure do |_config| + def almost_there(retries = 100) + yield + rescue RSpec::Expectations::ExpectationNotMetError + raise if retries < 1 + + sleep 0.1 + retries -= 1 + retry + end +end