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

ActiveJob - log enqueued message only after the job was successfully enqueued #20116

Merged
merged 1 commit into from Oct 5, 2015

Conversation

cristianbica
Copy link
Member

Currently if you try to enqueue the job with unserializable parameters you will see in the logs that the job was enqueued but you got an exception:

[72553d29-bd35-4e22-b0f7-b019df240aed] [ActiveJob] Enqueued Activejob::PerformLater::Job (Job ID: 6880ff6a-4558-4b1e-bef1-920b4229e280) to Sidekiq(default) with arguments: #<PaymentsService:0x007fc5cb4dc4c8>, "process", []
[72553d29-bd35-4e22-b0f7-b019df240aed] Completed 500 Internal Server Error in 54ms (ActiveRecord: 1.8ms)

  app/models/payment.rb:10:in `block in <class:Payment>'
  app/controllers/admin/base_controller.rb:16:in `create'

This PR moved the logging on the after_enqueue calback

@arthurnn
Copy link
Member

This make sense..
I guess we need a changelog for this.
Also I would guess we will need to backport this to 4-2-stable right?

@cristianbica
Copy link
Member Author

@arthurnn I don't see a need for a changelog entry.
About backporting I'm still not able to judge that so I'll just ask @rafaelfranca :)

arthurnn pushed a commit that referenced this pull request Oct 5, 2015
ActiveJob - log enqueued message only after the job was successfully enqueued
@arthurnn arthurnn merged commit d2ef471 into rails:master Oct 5, 2015
@arthurnn
Copy link
Member

arthurnn commented Oct 5, 2015

thanks

arthurnn pushed a commit that referenced this pull request Oct 5, 2015
ActiveJob - log enqueued message only after the job was successfully enqueued
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants