Skip to content

feat(tasks): add centralized error handling for task failures#560

Closed
kelvinkipruto wants to merge 1 commit intomainfrom
ft/task-level-failure-notification
Closed

feat(tasks): add centralized error handling for task failures#560
kelvinkipruto wants to merge 1 commit intomainfrom
ft/task-level-failure-notification

Conversation

@kelvinkipruto
Copy link
Contributor

Description

Implement createOnFail utility to capture task failures in Sentry with consistent error reporting across all task configurations. This provides better observability by including task slug, job ID, and error details in Sentry captures, making debugging easier when tasks encounter errors.

Fixes #556

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

Implement createOnFail utility to capture task failures in Sentry with
consistent error reporting across all task configurations. This provides
better observability by including task slug, job ID, and error details
in Sentry captures, making debugging easier when tasks encounter errors.
@vercel
Copy link

vercel bot commented Mar 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
PromiseTracker Ready Ready Preview, Comment Mar 10, 2026 0:57am

Request Review

@kelvinkipruto
Copy link
Contributor Author

@claude Review

@claude
Copy link

claude bot commented Mar 10, 2026

Claude finished @kelvinkipruto's task in 1m 49s —— View job


Code Review

  • Read CLAUDE.md for repo guidelines
  • Read changed files (utils.ts, createPoliticalEntity.ts, sample task files)
  • Analyze changes and provide review

Overall, this is a clean, well-structured approach to centralizing task failure reporting. The factory pattern for createOnFail is easy to apply consistently and the code is readable. A few issues worth addressing:


Issues

1. onFail will never fire for createPoliticalEntity and createTenant

src/tasks/createPoliticalEntity.ts, line 45 / src/tasks/createTenant.ts, line 37

Both tasks wrap their main logic in a try-catch that swallows errors and returns { output: {} } instead of rethrowing:

// createPoliticalEntity.ts line 732
} catch (error) {
  logger.error({ message: "createPoliticalEntity:: Task aborted due to unhandled error", ... });
}
return { output: {} };  // ← always succeeds from Payload's perspective

Since the task never throws, Payload never marks it as failed, so onFail is never triggered. In contrast, downloadDocuments.ts correctly rethrows at line 539 (throw error), making onFail useful there.

For createPoliticalEntity and createTenant, the onFail: createOnFail(...) lines added in this PR will only fire for errors thrown before the outer try-catch (e.g., the missing API key check). Fix this →


2. captureMessage loses stack trace — prefer captureException when error is available

src/tasks/utils.ts, lines 133–144

Sentry.captureMessage(`${taskSlug}: task failed`, {
  level: "error",
  ...
  extra: { ..., error },
});

When error in taskStatus is an Error instance (which it likely is in most failure scenarios), using captureMessage discards the stack trace and prevents Sentry from grouping issues by exception type. captureException would give better deduplication and visibility:

if (error instanceof Error) {
  Sentry.captureException(error, { tags: { task: taskSlug }, extra: { jobId: job.id, taskStatus, input: ... } });
} else {
  Sentry.captureMessage(`${taskSlug}: task failed`, { level: "error", ... });
}

Fix this →


3. Double Sentry reporting for uncaught exceptions

src/tasks/utils.ts, lines 188–205 (withTaskTracing) and lines 123–145 (createOnFail)

When a task throws (e.g., in downloadDocuments):

  1. withTaskTracing's catch block calls Sentry.captureException and rethrows
  2. Payload marks the job as failed and calls onFail, which calls Sentry.captureMessage

This produces two Sentry events for the same failure. Since withTaskTracing already captures the exception with full context, the onFail handler adds noise. Consider either:

  • Skipping captureException in withTaskTracing and relying solely on onFail, or
  • Documenting that the duplication is intentional (e.g., onFail fires for retries exhausted, not just individual throws)

Minor Notes

  • Redundant error field in extra: In createOnFail, error is extracted from taskStatus and included separately in extra, while taskStatus is also logged in full. This means error appears twice in Sentry's extra data. Not harmful, but slightly noisy.

  • Type annotation style (utils.ts, line 123): The function is typed with the return type on the const declaration rather than inline. The more idiomatic style would be export const createOnFail = (taskSlug: string): NonNullable<TaskConfig["onFail"]> => ... — minor readability preference.


Summary

The core idea is solid and the consistent application across all task files is well executed. The main concern is that createPoliticalEntity and createTenant swallow their errors, making the newly added onFail handler a no-op in the common error cases for those tasks. Fixing the error rethrowing in those two files (matching downloadDocuments.ts) would make this PR deliver its intended value.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f20825068d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kelvinkipruto
Copy link
Contributor Author

withTaskTracing already handles errors on task failure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant