Error logging for analytics tasks

[annehaley]

Resolves #302

[cloudflare-workers-and-pages]

Deploying geodatalytics with    Cloudflare Pages

Latest commit:	db4962e
Status:	✅  Deploy successful!
Preview URL:	https://9caf08cc.geodatalytics.pages.dev
Branch Preview URL:	https://task-errors.geodatalytics.pages.dev

View logs

[brianhelba]

I'm struggling to review this in GitHub, because of all the indentation changes. I can load this in my editor and look more closely, but I have another idea for how to handle this. Let me know what you think.

---

Giant try... blocks (also explicit raise inside a try...) are typically a code smell (because they add nesting and make it unclear precisely which code we're expecting to fail). A common remedy is additional use of encapsulation via functions.

It seems what we really care about is being notified of the exception so:

1. The TaskResult.error gets updated (though perhaps the exact details don't even matter, which this change suggests)
2. Sentry gets notified with a full stack trace of the exception

Note, 2 will naturally happen if we just allow an exception to bubble up unhandled.

---

We could use Celery's Task.on_failure handler to de-indent and reduce boilerplate. I think it'll be something like:

class AnalysisTask(celery.Task):
    def on_failure(self, exc, task_id, args, kwargs, einfo):
        # All analysis tasks use this signature
        task_result_id = args[0]

        task_result = TaskResult.objects.get(pk=task_result_id)
        # Could be a generic string, since the user doesn't need details
        task_result.error = str(exc)
        task_result.save()

@shared_task(base=AnalysisTask)
def ...

This will allow tasks to formally "fail" (in the Celery sense) when they raise exceptions. This has a few implications:

1. Sentry should just work, since it knows about Celery and will report any task failures.
2. More advanced Celery features for failed tasks, like "Automatic retry for known exceptions" could be used in the future.
3. Failed tasks should be properly cleared from the task queue, so they're not rerun (outside of a deliberate decision to retry). This should already happen, looking at the default behavior of task_acks_on_failure.
4. To the extent we ever want to look at Celery tasks in the aggregate (which admittedly can be difficult) or just via Clery logs, we now have metadata on the Celery task itself of whether it failed, matching the actual semantics of our business logic's failures.