Skip to content

fix: Enhance error logging in BackgroundTaskManager to include exception details #4504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 26, 2025

Conversation

HyeockJinKim
Copy link
Collaborator

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • Mention to the original issue
  • Installer updates including:
    • Fixtures for db schema changes
    • New mandatory config options
  • Update of end-to-end CLI integration tests in ai.backend.test
  • API server-client counterparts (e.g., manager API -> client SDK)
  • Test case(s) to:
    • Demonstrate the difference of before/after
    • Demonstrate the flow of abstract/conceptual models with a concrete implementation
  • Documentation
    • Contents in the docs directory
    • docstrings in public interfaces and type annotations

@HyeockJinKim HyeockJinKim self-assigned this May 26, 2025
@Copilot Copilot AI review requested due to automatic review settings May 26, 2025 06:53
@HyeockJinKim HyeockJinKim added the skip:changelog Make the action workflow to skip towncrier check label May 26, 2025
@github-actions github-actions bot added size:XS ~10 LoC comp:manager Related to Manager component comp:common Related to Common component labels May 26, 2025
@HyeockJinKim HyeockJinKim enabled auto-merge May 26, 2025 06:53
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances error logging in the background task observer and corrects the placement of sched_dispatcher_ctx in the root app builder.

  • Moves sched_dispatcher_ctx earlier in the build_root_app call, removing a duplicate and grouping dispatcher contexts.
  • Updates _observe_bgtask to log exception details when a task fails.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/ai/backend/manager/server.py Reordered sched_dispatcher_ctx argument in build_root_app call.
src/ai/backend/common/bgtask/bgtask.py Included exception variable e in the error log message.
Comments suppressed due to low confidence (1)

src/ai/backend/common/bgtask/bgtask.py:334

  • Add a test case that triggers an exception in _observe_bgtask to verify the log output includes the exception message (and stack trace if implemented).
log.error("Task {} ({}): unhandled error: {}", task_id, task_name, e)

@@ -331,7 +331,7 @@ async def _observe_bgtask(
operation=ErrorOperation.EXECUTE,
error_detail=ErrorDetail.INTERNAL_ERROR,
)
log.error("Task {} ({}): unhandled error", task_id, task_name)
log.error("Task {} ({}): unhandled error: {}", task_id, task_name, e)
Copy link
Preview

Copilot AI May 26, 2025

Choose a reason for hiding this comment

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

Consider using log.exception or passing exc_info=True to capture the full stack trace, for example:

log.exception("Task %s (%s) failed", task_id, task_name)
Suggested change
log.error("Task {} ({}): unhandled error: {}", task_id, task_name, e)
log.exception("Task {} ({}): unhandled error: {}", task_id, task_name, e)

Copilot uses AI. Check for mistakes.

@HyeockJinKim HyeockJinKim added this pull request to the merge queue May 26, 2025
Merged via the queue into main with commit e92c51a May 26, 2025
31 checks passed
@HyeockJinKim HyeockJinKim deleted the feat/add-bgtask-error-message branch May 26, 2025 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:common Related to Common component comp:manager Related to Manager component size:XS ~10 LoC skip:changelog Make the action workflow to skip towncrier check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant