Improve error handling in the cleanup artifact worker concern
🔥 Problem
In the package registry and the dependency proxy, we have this requirement of go through a set of records marked as destroyed and actually destroy them. Workers there are quite simple: select one marked record and destroy it.
This action is so simple and similar in the package registry and the dependency proxy that we created a concern to share common logic.
Related to !84700 (merged), we saw several flaws in that concern:
- There is
status
updates going on.- this already brought issues (#357373 (closed))
- we don't see the value of such
status
updates. Observability is already good enough without it. - we currently don't have any handling of objects that stays in the
processing
orerror
status.
- Errors are swallowed.
- This makes hard to debug issues as the original error is hidden and will not appear in the logs.
🚒 Solution
- Remove all
status
updates.- In the end, what matters is: either the worker destroyed the record or not. If not, then we need to log it (already done by the
fail
message of the worker and the considered record needs to stay as it is so that it is retried.
- In the end, what matters is: either the worker destroyed the record or not. If not, then we need to log it (already done by the
- Remove the
rescue
block.- Don't try to handle errors and simply let them bubble up.
- This will create a
fail
message that will log the error and its backtrace = much smoother debugging experience.
Edited by David Fernandez