Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RF-22752] Fix argument serializing when enqueuing failed job #43

Merged
merged 6 commits into from
Oct 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,7 @@ require "queue_classic_plus/datadog"
```
createdb queue_classic_plus_test
```

## Releasing

Releasing is done in CircleCI via the `push_to_rubygems`, triggered by pushing a tagged commit. To do so, simply [create a new GitHub release](https://github.com/rainforestapp/queue_classic_plus/releases/new).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Releasing is done in CircleCI via the `push_to_rubygems`, triggered by pushing a tagged commit. To do so, simply [create a new GitHub release](https://github.com/rainforestapp/queue_classic_plus/releases/new).
Releasing is done in CircleCI via the `push_to_rubygems`, triggered by pushing a tagged commit. To do so, simply [create a new GitHub release](https://github.com/rainforestapp/queue_classic_plus/releases/new) or run `rake release:source_control_push`

(untested)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then you lose getting the changelog for free! (and you also need to have the Rubygems credentials locally)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and you also need to have the Rubygems credentials locally

this is incorrect for release:source_control_push, as it just manipulates the SCM:
https://github.com/ruby/ruby/blob/5af3f7f3574c16ec76fb44b21beec17a74f4417a/lib/bundler/gem_helper.rb#L75-L77

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😮 TIL

2 changes: 1 addition & 1 deletion lib/queue_classic_plus/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module QueueClassicPlus
VERSION = '4.0.0.alpha8'.freeze
VERSION = '4.0.0.alpha9'.freeze
end
16 changes: 8 additions & 8 deletions lib/queue_classic_plus/worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ def handle_failure(job, e)
end

@failed_job = job
@failed_job_args = failed_job_class ? failed_job_class.deserialized(job[:args]) : job[:args]
@raw_args = job[:args]
@failed_job_args = failed_job_class ? failed_job_class.deserialized(@raw_args) : @raw_args

if force_retry && !(failed_job_class.respond_to?(:disable_retries) && failed_job_class.disable_retries)
Metrics.increment("qc.force_retry", source: @q_name)
retry_with_remaining(e)
# The mailers doesn't have a retries_on?
elsif failed_job_class && failed_job_class.respond_to?(:retries_on?) && failed_job_class.retries_on?(e)
elsif failed_job_class.respond_to?(:retries_on?) && failed_job_class.retries_on?(e)
Metrics.increment("qc.retry", source: @q_name)
retry_with_remaining(e)
else
Expand Down Expand Up @@ -60,11 +61,9 @@ def remaining_retries
end

def failed_job_class
begin
Object.const_get(@failed_job[:method].split('.')[0])
rescue NameError
nil
end
Object.const_get(@failed_job[:method].split('.')[0])
rescue NameError
nil
end

def backoff
Expand All @@ -80,7 +79,8 @@ def connection_error?(e)
def enqueue_failed(e)
sql = "INSERT INTO #{QC.table_name} (q_name, method, args, last_error) VALUES ('failed_jobs', $1, $2, $3)"
last_error = e.backtrace ? ([e.message] + e.backtrace ).join("\n") : e.message
QC.default_conn_adapter.execute sql, @failed_job[:method], JSON.dump(@failed_job_args), last_error

QC.default_conn_adapter.execute sql, @failed_job[:method], JSON.dump(@raw_args), last_error

QueueClassicPlus.exception_handler.call(e, @failed_job)
Metrics.increment("qc.errors", source: @q_name)
Expand Down
6 changes: 1 addition & 5 deletions spec/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,7 @@ def self.perform
end
end

context "with Rails defined" do
require 'active_job/arguments'

before { stub_const('Rails', true) }

context "with Rails defined", rails: true do
subject do
Class.new(QueueClassicPlus::Base) do
@queue = :test
Expand Down
7 changes: 7 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,11 @@
# Reset the default (memoized) queue instance between specs
QC.default_queue = nil
end

config.before(:each, rails: true) do
require 'active_job'
require 'active_job/arguments'

stub_const('Rails', Struct.new(:logger).new(Logger.new(STDOUT)))
end
end
27 changes: 21 additions & 6 deletions spec/worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,26 @@
worker.work
expect(failed_queue.count).to eq(1)
end

context 'when Rails is defined', rails: true do
let(:job_type) { Jobs::Tests::TestJobNoRetry }
let(:queue) { job_type.queue }

it 'properly serializes arguments for jobs in the failed queue' do
job_type.enqueue_perform(:raise)
expect(failed_queue.count).to eq(0)
worker.work

expect(failed_queue.count).to eq(1)
job = QueueClassicMatchers::QueueClassicRspec.find_by_args(
'failed_jobs',
'Jobs::Tests::TestJobNoRetry._perform',
[:raise]).first

expect(job).to_not be_nil
expect(job['last_error']).to_not be_nil
end
end
end

context "retry" do
Expand Down Expand Up @@ -67,12 +87,7 @@
end
end

context 'when Rails is defined' do
require 'active_job'
require 'active_job/arguments'

before { stub_const('Rails', Struct.new(:logger).new(Logger.new(STDOUT))) }

context 'when Rails is defined', rails: true do
it 'retries' do
expect do
job_type.enqueue_perform(:foo)
Expand Down