Skip to content

Commit ca7d386

Browse files
authored
Fix a crash when ActiveJob context include a range with TimeWithZone (#2548)
* Add spec:versioned rake task * Add a spec checking range serialization in ActiveJob * Fix argument serialization for ranges If a range consists for ActiveSupport::TimeWithZone, it will be serialized as a string. Previously it would raise an error ``` TypeError: can't iterate from ActiveSupport::TimeWithZone ``` Closes #2545 * Ensure begin-less and end-less ranges work too * Update CHANGELOG * [rubocop] ignore versioned specs Our target ruby version is 2.6 so...
1 parent cc1dc53 commit ca7d386

File tree

8 files changed

+191
-1
lines changed

8 files changed

+191
-1
lines changed

.rubocop.yml

+1
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,4 @@ AllCops:
1818
- "sentry-raven/**/*"
1919
- "sentry-*/tmp/**/*"
2020
- "sentry-*/examples/**/*"
21+
- "sentry-*/spec/versioned/2.7/**/*"

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
- Add correct breadcrumb levels for 4xx/5xx response codes ([#2549](https://github.com/getsentry/sentry-ruby/pull/2549))
66

7+
### Bug Fixes
8+
9+
- Fix argument serialization for ranges that consist of ActiveSupport::TimeWithZone ([#2548](https://github.com/getsentry/sentry-ruby/pull/2548))
10+
711
### Miscellaneous
812

913
- Deprecate `enable_tracing` in favor of `traces_sample_rate = 1.0` [#2535](https://github.com/getsentry/sentry-ruby/pull/2535)

sentry-rails/Rakefile

+19-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,24 @@ require "rspec/core/rake_task"
55

66
RSpec::Core::RakeTask.new(:spec).tap do |task|
77
task.rspec_opts = "--order rand"
8+
task.pattern = "spec/sentry/**/*_spec.rb"
9+
end
10+
11+
namespace :spec do
12+
RSpec::Core::RakeTask.new(:versioned).tap do |task|
13+
ruby_ver_dir = RUBY_VERSION.split(".")[0..1].join(".")
14+
matching_dir = Dir["spec/versioned/*"].detect { |dir| File.basename(dir) <= ruby_ver_dir }
15+
16+
unless matching_dir
17+
puts "No versioned specs found for ruby #{RUBY_VERSION}"
18+
exit 0
19+
end
20+
21+
puts "Running versioned specs from #{matching_dir} for ruby #{RUBY_VERSION}"
22+
23+
task.rspec_opts = "--order rand"
24+
task.pattern = "#{matching_dir}/**/*_spec.rb"
25+
end
826
end
927

1028
task :isolated_specs do
@@ -13,4 +31,4 @@ task :isolated_specs do
1331
end
1432
end
1533

16-
task default: [:spec, :isolated_specs]
34+
task default: [:spec, :"spec:versioned", :isolated_specs]

sentry-rails/lib/sentry/rails/active_job.rb

+6
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,12 @@ def sentry_context(job)
7979

8080
def sentry_serialize_arguments(argument)
8181
case argument
82+
when Range
83+
if (argument.begin || argument.end).is_a?(ActiveSupport::TimeWithZone)
84+
argument.to_s
85+
else
86+
argument.map { |v| sentry_serialize_arguments(v) }
87+
end
8288
when Hash
8389
argument.transform_values { |v| sentry_serialize_arguments(v) }
8490
when Array, Enumerable

sentry-rails/spec/sentry/rails/activejob_spec.rb

+43
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,49 @@ def post.to_global_id
139139
]
140140
)
141141
end
142+
143+
it "serializes range arguments gracefully when Range#map is implemented" do
144+
post = Post.create!
145+
146+
expect do
147+
JobWithArgument.perform_now("foo", { bar: Sentry }, integer: 1, post: post, range: 1..3)
148+
end.to raise_error(RuntimeError)
149+
150+
event = transport.events.last.to_json_compatible
151+
expect(event.dig("extra", "arguments")).to eq(
152+
[
153+
"foo",
154+
{ "bar" => "Sentry" },
155+
{
156+
"integer" => 1,
157+
"post" => "gid://rails-test-app/Post/#{post.id}",
158+
"range" => [1, 2, 3]
159+
}
160+
]
161+
)
162+
end
163+
164+
it "serializes range arguments gracefully when Range consists of ActiveSupport::TimeWithZone" do
165+
post = Post.create!
166+
range = 5.days.ago...1.day.ago
167+
168+
expect do
169+
JobWithArgument.perform_now("foo", { bar: Sentry }, integer: 1, post: post, range: range)
170+
end.to raise_error(RuntimeError)
171+
172+
event = transport.events.last.to_json_compatible
173+
expect(event.dig("extra", "arguments")).to eq(
174+
[
175+
"foo",
176+
{ "bar" => "Sentry" },
177+
{
178+
"integer" => 1,
179+
"post" => "gid://rails-test-app/Post/#{post.id}",
180+
"range" => "#{range.first}...#{range.last}"
181+
}
182+
]
183+
)
184+
end
142185
end
143186

144187
it "adds useful context to extra" do

sentry-rails/spec/spec_helper.rb

+2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030

3131
DUMMY_DSN = 'http://12345:[email protected]/sentry/42'
3232

33+
Dir["#{__dir__}/support/**/*.rb"].each { |file| require file }
34+
3335
RSpec.configure do |config|
3436
# Enable flags like --only-failures and --next-failure
3537
config.example_status_persistence_file_path = ".rspec_status"
+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# frozen_string_literal: true
2+
3+
require "active_job/railtie"
4+
5+
class NormalJob < ActiveJob::Base
6+
def perform
7+
"foo"
8+
end
9+
end
10+
11+
class FailedJob < ActiveJob::Base
12+
self.logger = nil
13+
14+
class TestError < RuntimeError
15+
end
16+
17+
def perform
18+
a = 1
19+
b = 0
20+
raise TestError, "Boom!"
21+
end
22+
end
23+
24+
class FailedWithExtraJob < FailedJob
25+
def perform
26+
Sentry.get_current_scope.set_extras(foo: :bar)
27+
super
28+
end
29+
end
30+
31+
class JobWithArgument < ActiveJob::Base
32+
def perform(*args, integer:, post:, **options)
33+
raise "foo"
34+
end
35+
end
36+
37+
class QueryPostJob < ActiveJob::Base
38+
self.logger = nil
39+
40+
def perform
41+
Post.all.to_a
42+
end
43+
end
44+
45+
class RescuedActiveJob < FailedWithExtraJob
46+
rescue_from TestError, with: :rescue_callback
47+
48+
def rescue_callback(error); end
49+
end
50+
51+
class ProblematicRescuedActiveJob < FailedWithExtraJob
52+
rescue_from TestError, with: :rescue_callback
53+
54+
def rescue_callback(error)
55+
raise "foo"
56+
end
57+
end
58+
59+
class NormalJobWithCron < NormalJob
60+
include Sentry::Cron::MonitorCheckIns
61+
sentry_monitor_check_ins
62+
end
63+
64+
class FailedJobWithCron < FailedJob
65+
include Sentry::Cron::MonitorCheckIns
66+
sentry_monitor_check_ins slug: "failed_job", monitor_config: Sentry::Cron::MonitorConfig.from_crontab("5 * * * *")
67+
end
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# frozen_string_literal: true
2+
3+
require "spec_helper"
4+
5+
RSpec.describe "ActiveJob integration", type: :job do
6+
before do
7+
make_basic_app
8+
end
9+
10+
let(:event) do
11+
transport.events.last.to_json_compatible
12+
end
13+
14+
let(:transport) do
15+
Sentry.get_current_client.transport
16+
end
17+
18+
it "returns #perform method's return value" do
19+
expect(NormalJob.perform_now).to eq("foo")
20+
end
21+
22+
describe "ActiveJob arguments serialization" do
23+
it "serializes range begin-less and end-less arguments gracefully when Range consists of ActiveSupport::TimeWithZone" do
24+
post = Post.create!
25+
26+
range_no_beginning = (..1.day.ago)
27+
range_no_end = (5.days.ago..)
28+
29+
expect do
30+
JobWithArgument.perform_now("foo", { bar: Sentry },
31+
integer: 1, post: post, range_no_beginning: range_no_beginning, range_no_end: range_no_end)
32+
end.to raise_error(RuntimeError)
33+
34+
event = transport.events.last.to_json_compatible
35+
expect(event.dig("extra", "arguments")).to eq(
36+
[
37+
"foo",
38+
{ "bar" => "Sentry" },
39+
{
40+
"integer" => 1,
41+
"post" => "gid://rails-test-app/Post/#{post.id}",
42+
"range_no_beginning" => "..#{range_no_beginning.last}",
43+
"range_no_end" => "#{range_no_end.first}.."
44+
}
45+
]
46+
)
47+
end
48+
end
49+
end

0 commit comments

Comments
 (0)