Skip to content

Conversation

@jhthompson
Copy link
Contributor

Fixes #50

@jhthompson jhthompson changed the title Allow ImmediateBackend to throw its errors Log errors in ImmediateBackend, and throw KeyboardInterrupt Jul 18, 2024
@jhthompson jhthompson changed the title Log errors in ImmediateBackend, and throw KeyboardInterrupt Swallow and log any BaseException in ImmediateBackend, but continue to throw KeyboardInterrupt Jul 18, 2024
Comment on lines 47 to 48
# Use `.exception` to integrate with error monitoring tools (eg Sentry)
logger.exception("Task execution failed: %s", e)
Copy link
Contributor Author

@jhthompson jhthompson Jul 18, 2024

Choose a reason for hiding this comment

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

Should probably improve this message? I was looking at these lines from db_worker as an idea, but not sure if task_path is available here though:

# Use `.exception` to integrate with error monitoring tools (eg Sentry)
logger.exception(
    "Task id=%s path=%s state=%s",
    db_task_result.id,
    db_task_result.task_path,
    ResultStatus.FAILED,
)

Copy link
Owner

Choose a reason for hiding this comment

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

There's a module path on the Task you can use.

logfmt would also be great - there should be everything available here to replicate the message, you just might need to move the TaskResult creation above the execution for convenience.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed an improvement, I just used the result_id for the ID, is that fine?

@jhthompson jhthompson marked this pull request as ready for review July 18, 2024 13:49
@jhthompson jhthompson changed the title Swallow and log any BaseException in ImmediateBackend, but continue to throw KeyboardInterrupt Swallow and log any BaseException in ImmediateBackend other than KeyboardInterrupt Jul 18, 2024
@RealOrangeOne RealOrangeOne merged commit 2a9558c into RealOrangeOne:master Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow ImmediateBackend to throw its errors

2 participants