playframework
[#2060] drop exceptions thrown by @Every jobs so that they get rescheduled
This pull request fixes a regression from 1.2.7 that was introduced by the fix for [#1518]. In short, the way that "every" jobs are scheduled will cause them to stop being re-scheduled if they throw an exception. The fix for [#1518] started to throw exceptions that had previously been swallowed by the invocation framework. The fix for [#2060] is to swallow any exceptions in the narrow context between an "every" job and the executor service. The fix for [#1518] is retained.
The ticket for [#2060] only mentions the @Every annotation, but Job.every() had the same regression. To ensure that the fix was in one place, some refactoring was done to make the @Every logic invoke Job.every(). A slight (and pleasant) side-effect of the refactoring is that /@status no longer reports @Every("never") jobs as being scheduled to "run every never".
The pull request includes unit tests, which pass on 1.2.7, fail on 1.4.3, and pass with the fix.
Thank you for the thoughtful review, @flybyray. I don't know if your "general summary" comment was directed at me, but you're correct that I never considered Job.onException() to be part of the public API. I don't know why, but I always considered it to be Play! internals.
I'm sorry to be a pest, but I didn't understand all of the recommendations, so I responded with questions instead of a code update. Once I get a clear understanding of what you have in mind, I'll be able to fix my broken pull request.
@davidcostanzo it was directed to all who cannot understand what // Customize Invocation. means. the javadoc comment is messy. Should be fixed too! And at all the documentation could be better with a topic on exception handling for jobs https://www.playframework.com/documentation/1.4.x/jobs
I have updated the pull request as @flybyray recommended, adding a flag to limit the exception throwing to the code paths of any job scheduled by in or now. The pull request includes setting this flag for afterRequest, so that afterRequest's return value has consistent behavior, regardless of whether the job was scheduled with now or as an after action.
Units tests were added to cover the exception handling of in, now, and @On.
The pull request update also reverts the change to JobsPlugin.java, since Job.every() remains simple enough not to warrant this refactoring.