Removing ActiveJob::DeserializationError as a default ignored exception

[calvinhughes]

Summary

Recently in an ActiveJob worker we faced an issue where an exception was raised during deserialization of the arguments due to some database connection issues. This raised an ActiveJob::DeserializationError exception, which was caused by the following:

ActiveJob::DeserializationError Error while trying to deserialize arguments: PG::ConnectionBad: PQsocket() can't get socket descriptor

Because the ActiveJob::DeserializationError is ignored by default then this was not sent to Sentry.

The exception was added due to #701 and #642 where it was assumed that this is only raised when a record cannot be found but like the case above this is not the case. It's easy enough to fix in a particular app as it can just be removed from the default ignored exceptions but I feel like it could trip up new users of the gem using ActiveJob.

It would be great if we could somehow report these legitimate errors, and still ignore the NotFound type errors due to the intentioned lined out in the issues above.

---

We have the inspect_exception_causes_for_exclusion config option for the SDK. I understand that was added to not change the behaviour for existing users, but are there any plans for making that the default in the future?

If there are, I suppose we could just remove ActiveJob::DeserializationError when that happens as the NotFound style deserialization errors would be caught via ActiveRecord::RecordNotFound through the ExceptionCauseChain middleware.

Or if it remains solely to avoid the SentryJob example breaking when it isn't able to find a record, an alternative could be to remove it and update the async example to discard the job upon encountering a deserialization error via discard_on.

Wondered what your thoughts were, especially as the gem is currently undergoing a revamp. Happy to help with a PR too!

[agrobbin]

@calvinhughes you took the words right out of my mouth! We actually just ran into a very similar issue with Sentry ignoring ActiveJob::DeserializationError. In our case it was wrapping ActiveRecord::ConnectionTimeoutError:

ActiveJob::DeserializationError: Error while trying to deserialize arguments: could not obtain a connection from the pool within 5.000 seconds (waited 5.295 seconds); all pooled connections were in use

We don't discard jobs from Active Job when DeserializationError is raised, instead we discard on the more specific ActiveRecord::RecordNotFound exception, which is the only case we could think of where we don't want to retry the job or track it in Sentry:

# frozen_string_literal: true

class ApplicationJob < ActiveJob::Base
  # Automatically retry jobs that encountered a deadlock
  # retry_on ActiveRecord::Deadlocked

  # Most jobs are safe to ignore if the underlying records are no longer available
  discard_on ActiveRecord::RecordNotFound
end

[st0012]

@calvinhughes @agrobbin thanks for both of your opinions! since ActiveJob::DeserializationError is essentially a wrapper exception, excluding it to avoid infinite loop seems a bit too rough to me. however, because it's been in the codebase for 3 years and the 3.x version has entered maintenance mode, I want to avoid removing it directly from the old sdk, which feels risky to me.

so here's what I'm going to do:

• I will remove it from the new SDK and address the issue mentioned in Raven Ruby + ActiveJob can kill BG processing #642 in another way
• If the fix works, I may backport it to 3.x versions depends on the changes needed

wdyt?

[agrobbin]

That seems like a great plan to me @st0012!

[calvinhughes]

@st0012 Sounds great!

[st0012]

sorry for the long waiting. I've added #1180 to address this issue (in the new SDK) 🙂