Skip to content

Commit

Permalink
Support capturing Puma's error
Browse files Browse the repository at this point in the history
This PR implements Puma's error reporting by patching `Puma::Server#lowlevel_error`.

I thought about using the `lowlevel_error_handler` API to add a Sentry error
handler, but it's not possible to do that after `puma.rb` is loaded (which
is extremely early in a normal app setup, way before the SDK is loaded).
There is also no good ways to fetch and update the server's `lowlevel_error_handler` option.

So the only option left is to patch the method.
  • Loading branch information
st0012 committed Apr 18, 2023
1 parent 7fd714e commit ef4207a
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 1 deletion.
2 changes: 2 additions & 0 deletions sentry-ruby/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ gem "rack", "~> #{Gem::Version.new(rack_version)}" unless rack_version == "0"
redis_rb_version = ENV.fetch("REDIS_RB_VERSION", "5.0")
gem "redis", "~> #{redis_rb_version}"

gem "puma"

gem "rake", "~> 12.0"
gem "rspec", "~> 3.0"
gem "rspec-retry"
Expand Down
9 changes: 8 additions & 1 deletion sentry-ruby/Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ require "rspec/core/rake_task"

RSpec::Core::RakeTask.new(:spec).tap do |task|
task.rspec_opts = "--order rand"
task.exclude_pattern = "spec/isolated/**/*_spec.rb"
end

task :default => :spec
task :isolated_specs do
Dir["spec/isolated/*"].each do |file|
sh "bundle exec rspec #{file}"
end
end

task :default => [:spec, :isolated_specs]
1 change: 1 addition & 0 deletions sentry-ruby/lib/sentry-ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -519,3 +519,4 @@ def utc_now
# patches
require "sentry/net/http"
require "sentry/redis"
require "sentry/puma"
25 changes: 25 additions & 0 deletions sentry-ruby/lib/sentry/puma.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

module Sentry
module Puma
module Server
def lowlevel_error(e, env, status=500)
result = super

begin
Sentry.capture_exception(e) do |scope|
scope.set_rack_env(env)
end
rescue
# if anything happens, we don't want to break the app
end

result
end
end
end
end

if defined?(Puma::Server)
Sentry.register_patch(Sentry::Puma::Server, Puma::Server)
end
91 changes: 91 additions & 0 deletions sentry-ruby/spec/isolated/puma_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
require "puma"
require_relative "../spec_helper"

# Because puma doesn't have any dependency, if Rack is not installed the entire test won't work
return if ENV["RACK_VERSION"] == "0"

RSpec.describe Puma::Server do
class TestServer
def initialize(app, options)
@host = "127.0.0.1"
@ios = []
@server = Puma::Server.new(app, nil, options)
@port = (@server.add_tcp_listener @host, 0).addr[1]
@server.run
end

def send_http_and_read(req)
(new_connection << req).read
end

def new_connection
TCPSocket.new(@host, @port).tap {|sock| @ios << sock}
end

def close
@ios.each { |io| io.close }
end
end

let(:app) do
proc { raise "foo" }
end

def server_run(app, lowlevel_error_handler: nil, &block)
server = TestServer.new(app, lowlevel_error_handler: lowlevel_error_handler)
yield server
ensure
server.close
end

before do
perform_basic_setup
end

it "captures low-level errors" do
res = server_run(app) do |server|
server.send_http_and_read("GET / HTTP/1.0\r\n\r\n")
end
expect(res).to match(/500 Internal Server Error/)
events = sentry_events
expect(events.count).to eq(1)
event = events.first
expect(event.exception.values.first.value).to match("foo")
end

context "when user defines lowlevel_error_handler" do
it "captures low-level errors" do
handler_executed = false

lowlevel_error_handler = ->(e, env) do
handler_executed = true
# Due to the way we test Puma::Server, we won't be verify this response
[500, {}, ["Error is handled"]]
end

res = server_run(app, lowlevel_error_handler: lowlevel_error_handler) do |server|
server.send_http_and_read("GET / HTTP/1.0\r\n\r\n")
end

expect(res).to match(/500 Internal Server Error/)
expect(handler_executed).to eq(true)
events = sentry_events
expect(events.count).to eq(1)
event = events.first
expect(event.exception.values.first.value).to match("foo")
end
end

context "when Sentry.capture_exception causes error" do
it "doesn't affect the response" do
expect(Sentry).to receive(:capture_exception).and_raise("bar")

res = server_run(app) do |server|
server.send_http_and_read("GET / HTTP/1.0\r\n\r\n")
end

expect(res).to match(/500 Internal Server Error/)
expect(sentry_events).to be_empty
end
end
end

0 comments on commit ef4207a

Please sign in to comment.