Reset scheduler state after batch errors#17
Conversation
|
Thank you for your contribution! The scheduler is definitely in need of improved error handling so this is definitely a welcome change. Also, since this is our first public contribution, we're still ironing out the documentation/workflows that allow us to ingest new changes internally. This PR made me realize we had a gap in our contributing guide for flagging changes, which I have addressed in a recent commit: #19 the tldr; is basically that logic changes should be put behind a feature flag, which should be centralized in this package: https://github.com/Roblox/signals/blob/main/modules/signals-flags/src/init.lua you can basically just pattern match from the existing flag, and/or read over the full instructions here: https://github.com/Roblox/signals?tab=contributing-ov-file#code-guidelines |
jkelaty-rbx
left a comment
There was a problem hiding this comment.
A couple more thoughts on the implementation but otherwise this looks good
Co-authored-by: jkelaty-rbx <78873527+jkelaty-rbx@users.noreply.github.com>
jkelaty-rbx
left a comment
There was a problem hiding this comment.
Looks good! I need to get one more reviewer from Roblox to approve this before it can be merged, which I will do early next week (hopefully Monday!)
Fixes signals-scheduler so failed batch or scheduled work no longer leaves the scheduler stuck in a continuing state with stale queued continuations. The scheduler now clears pending continuations and restores its state before rethrowing the original error
I've also added test coverage for errors thrown by both batched work and scheduled work