Inaccurate comment in ApplicationJob template

[ce07c3]

https://github.com/rails/rails/blob/6-0-stable/activejob/lib/rails/generators/job/templates/application_job.rb.tt:

# Most jobs are safe to ignore if the underlying records are no longer available
# discard_on ActiveJob::DeserializationError

This error is raised for any errors during deserialization, not exclusively for AR records that are no longer available.

https://github.com/rails/rails/blob/6-0-stable/activejob/lib/active_job/arguments.rb:

   # Deserializes a set of arguments. Intrinsic types that can safely be
    # deserialized without mutation are returned as-is. Arrays/Hashes are
    # deserialized element by element. All other types are deserialized using
    # GlobalID.
    def deserialize(arguments)
      arguments.map { |argument| deserialize_argument(argument) }
    rescue
      raise DeserializationError
    end

Say the database is slow to respond and the query times out, then this would raise a DeserializationError error as well.

[p8]

If the database is slow to respond an AdapterTimeout is raised.

rails/activerecord/lib/active_record/errors.rb

Line 398 in 397bfb0

class AdapterTimeout < QueryAborted

AdapterTimeout eventually inherits from StandardError which is rescued by the default rescue. I think this is a bug and not by design.

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 6-1-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[ce07c3]

Still an issue.

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 6-1-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[ce07c3]

Still an issue.

[ghiculescu]

Say the database is slow to respond and the query times out, then this would raise a DeserializationError error as well.

While true, this is unlikely given the database query would be a Model.find query via GlobalID. I think the comment is correct 99+% of the time and that feels fine, particularly as the discard_on statement is not enabled by default.

rails/activejob/lib/rails/generators/job/templates/application_job.rb.tt

Line 7 in 21243aa

# discard_on ActiveJob::DeserializationError

@ce07c3 how do you propose this be improved? Is it an issue with the code comments or is there a change to functionality you think should be made?

[ce07c3]

Say the database is slow to respond and the query times out, then this would raise a DeserializationError error as well.

While true, this is unlikely given the database query would be a Model.find query via GlobalID. I think the comment is correct 99+% of the time and that feels fine, particularly as the discard_on statement is not enabled by default.

It's just an example. Even if that were the only problem, if it's 99%, it's still not generally true, unfortunately, which the documentation should strive for.

There are other deserialization errors that can occur. Since rescue here catches StandardError, this assumption is simply not valid I am afraid:

# Most jobs are safe to ignore if the underlying records are no longer available
# discard_on ActiveJob::DeserializationError

rails/activejob/lib/rails/generators/job/templates/application_job.rb.tt

Line 7 in 21243aa

# discard_on ActiveJob::DeserializationError

@ce07c3 how do you propose this be improved? Is it an issue with the code comments or is there a change to functionality you think should be made?

I believe all that needs to be changed is the comment.

[ghiculescu]

I believe all that needs to be changed is the comment.

Okay, I see your point - do you want to propose new wording? https://guides.rubyonrails.org/contributing_to_ruby_on_rails.html runs you through how to make a PR with your proposed changes 👍

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 7-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.