resque-retry
resque-retry copied to clipboard
Callbacks don't run if they don't have the right arity
If the arity of the callback is not right, then an ArgumentError: wrong number of arguments gets thrown, but instance_exec
swallows all exceptions so basically the effect is that the callback doesn't get run.
Are you able to look at this issue before we look at #129
Sure thing. So what do you think would be a good solution? Here are some thoughts:
- We could do an arity check when the callback is registered, and throw an exception if the arity is 0, since that would probably catch most cases. I think it gets a bit more complicated if we want to have a tighter guarantee on the correct arity (by matching the arity of
self.perform
with the callback) - We can print a warning for all the exceptions we get, instead of just silently suppressing them
I think that the second approach might actually be favorable. Unless of course there is a reliable/easy way to do the arity check up front, during the registration process. If we dump something into the Resque
log it will likely give enough context to the person{,s} trouble-shooting so that they can remedy the issue on their side, we can, and should, only guard against so much. What do you think @lantins?
We need to let things blow up, as early as possible by letting the exceptions bubble through.
I guess the question is, are callbacks 'best effort' or 'job fails if callback fails'?
My concern here was allowing callbacks to blow up within the internals of
resque-retry
. I am all for blowin up early if we can do it with 100%
confidence. Perhaps we can enforce the call signature for these rather than
letting them be varargs? If neither seems feasible I think the best
solution here is to make it a best attempt and log when there is a failure.
On Nov 3, 2015 6:24 PM, "Luke Antins" [email protected] wrote:
We need to let things blow up, as early as possible by letting the exceptions bubble through.
I guess the question is, are callbacks 'best effort' or 'job fails if callback fails'?
— Reply to this email directly or view it on GitHub https://github.com/lantins/resque-retry/issues/127#issuecomment-153521621 .
Best attempts to log the failure is a good place to start.
Excellent. Glad we're in agreement.
:+1: