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

Introduce PayloadNotFoundError (subclass of DeserializationError) #1004

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

johnnyshields
Copy link

@johnnyshields johnnyshields commented Oct 23, 2017

Currently, there are two types of DeserializationErrors:

  1. The handler code itself is malformed, e.g. triggered by a Psych::SyntaxError

  2. The payload object is not found in the database (refer to Delayed::PsychExt class, e.g. the below)

        when %r{^!ruby/ActiveRecord:(.+)$}
          klass = resolve_class(Regexp.last_match[1])
          payload = Hash[*object.children.map { |c| accept c }]
          id = payload['attributes'][klass.primary_key]
          id = id.value if defined?(ActiveRecord::Attribute) && id.is_a?(ActiveRecord::Attribute)
          begin
            klass.unscoped.find(id)
          rescue ActiveRecord::RecordNotFound => error
            raise Delayed::PayloadNotFoundError, "ActiveRecord::RecordNotFound, class: #{klass}, primary key: #{id} (#{error.message})"
          end

Case 1 makes sense to handle by immediately permanently failing the job, if the yaml is malformed the job will never run.

Case 2 however, there is a possibility that the job will run at a later time, as the object could appear later in the database. This is especially true for Mongoid, which is non-transactional and uses replication with some minor timelag. When triggering a delayed job (e.g. a mailer) on new object creation, the job can possibly be invoked and query a replica set member BEFORE the data has replicated, raising a Mongoid::Errors::DocumentNotFound which is re-raised as a Delayed::DeserializationError.

To give power users like myself more flexibility, I've introduced a new config Delayed::Worker.fail_if_payload_not_found. The default is the current behavior (immediate failure) however it can be tweaked to retry.

@johnnyshields
Copy link
Author

Spec failure here is not due to this PR, please check Travis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant