Fix worker status tracking: use fresh DB session for notifications, commit before notifying
Agent-Logs-Url: https://github.com/christianlouis/InboxConverge/sessions/70a69601-e0b9-4a00-bcf4-0a85d2bdf6cb Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
|
|
||||||
<!-- version list -->
|
<!-- version list -->
|
||||||
|
|
||||||
|
## [Unreleased]
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- Worker tasks: use a fresh DB session for `send_user_notification` calls and move notifications after `db.commit()` to prevent the post-rollback `greenlet_spawn` SQLAlchemy error.
|
||||||
|
- Worker tasks: ensure `last_check_at` and error status are always committed before notifications, fixing accounts being endlessly re-queued after IMAP auth failures.
|
||||||
|
|
||||||
## v0.3.2 (2026-03-28)
|
## v0.3.2 (2026-03-28)
|
||||||
|
|
||||||
### Bug Fixes
|
### Bug Fixes
|
||||||
|
|||||||
@@ -285,8 +285,9 @@ async def process_mail_account(account_id: int):
|
|||||||
"User must re-authorise."
|
"User must re-authorise."
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
|
async with async_session_maker() as notif_db:
|
||||||
await send_user_notification(
|
await send_user_notification(
|
||||||
db=db,
|
db=notif_db,
|
||||||
user_id=int(account.user_id),
|
user_id=int(account.user_id),
|
||||||
title="InboxRescue: Gmail Authorization Expired",
|
title="InboxRescue: Gmail Authorization Expired",
|
||||||
body=f"Your Gmail credentials for account '{account.name}' have been revoked. Please re-authorize Gmail access in Settings.",
|
body=f"Your Gmail credentials for account '{account.name}' have been revoked. Please re-authorize Gmail access in Settings.",
|
||||||
@@ -364,9 +365,17 @@ async def process_mail_account(account_id: int):
|
|||||||
account.status = AccountStatus.ERROR # type: ignore[assignment]
|
account.status = AccountStatus.ERROR # type: ignore[assignment]
|
||||||
account.last_error_at = datetime.now(timezone.utc) # type: ignore[assignment]
|
account.last_error_at = datetime.now(timezone.utc) # type: ignore[assignment]
|
||||||
account.last_error_message = f"{emails_failed} emails failed to forward" # type: ignore[assignment]
|
account.last_error_message = f"{emails_failed} emails failed to forward" # type: ignore[assignment]
|
||||||
|
|
||||||
|
await db.commit()
|
||||||
|
|
||||||
|
# Send failure notification after the commit so the status is
|
||||||
|
# persisted even if the notification fails. Use a fresh session
|
||||||
|
# to avoid interfering with the (now-committed) main transaction.
|
||||||
|
if emails_failed > 0:
|
||||||
try:
|
try:
|
||||||
|
async with async_session_maker() as notif_db:
|
||||||
await send_user_notification(
|
await send_user_notification(
|
||||||
db=db,
|
db=notif_db,
|
||||||
user_id=int(account.user_id),
|
user_id=int(account.user_id),
|
||||||
title="InboxRescue: Mail Forwarding Failures",
|
title="InboxRescue: Mail Forwarding Failures",
|
||||||
body=f"Mail account '{account.name}': {emails_failed} email(s) failed to forward.",
|
body=f"Mail account '{account.name}': {emails_failed} email(s) failed to forward.",
|
||||||
@@ -375,8 +384,6 @@ async def process_mail_account(account_id: int):
|
|||||||
except Exception as notify_exc:
|
except Exception as notify_exc:
|
||||||
logger.warning(f"Failed to send notification: {notify_exc}")
|
logger.warning(f"Failed to send notification: {notify_exc}")
|
||||||
|
|
||||||
await db.commit()
|
|
||||||
|
|
||||||
# Record Prometheus metrics for this completed run
|
# Record Prometheus metrics for this completed run
|
||||||
_run_status = "completed" if emails_failed == 0 else "partial_failure"
|
_run_status = "completed" if emails_failed == 0 else "partial_failure"
|
||||||
MAIL_PROCESSING_RUNS_TOTAL.labels(status=_run_status).inc()
|
MAIL_PROCESSING_RUNS_TOTAL.labels(status=_run_status).inc()
|
||||||
@@ -437,20 +444,6 @@ async def process_mail_account(account_id: int):
|
|||||||
# throttles re-dispatch instead of queuing a new task every cycle.
|
# throttles re-dispatch instead of queuing a new task every cycle.
|
||||||
account.last_check_at = datetime.now(timezone.utc) # type: ignore[assignment]
|
account.last_check_at = datetime.now(timezone.utc) # type: ignore[assignment]
|
||||||
|
|
||||||
# Notify user about the error
|
|
||||||
try:
|
|
||||||
await send_user_notification(
|
|
||||||
db=db,
|
|
||||||
user_id=int(account.user_id),
|
|
||||||
title="InboxRescue: Mail Processing Error",
|
|
||||||
body=f"Error processing mail account '{account.name}': {e}",
|
|
||||||
notify_on_error=True,
|
|
||||||
)
|
|
||||||
except Exception as notify_exc:
|
|
||||||
logger.warning(
|
|
||||||
f"Failed to send error notification: {notify_exc}"
|
|
||||||
)
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
await db.commit()
|
await db.commit()
|
||||||
except Exception as commit_exc:
|
except Exception as commit_exc:
|
||||||
@@ -459,6 +452,24 @@ async def process_mail_account(account_id: int):
|
|||||||
f"{account_id}: {commit_exc}"
|
f"{account_id}: {commit_exc}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Send error notification after the commit (and outside the run/account
|
||||||
|
# guards) so the status is always persisted first. Use a fresh session
|
||||||
|
# to avoid the post-rollback session's broken greenlet context causing
|
||||||
|
# the notification query itself to fail with "greenlet_spawn has not
|
||||||
|
# been called".
|
||||||
|
if "account" in locals() and account is not None:
|
||||||
|
try:
|
||||||
|
async with async_session_maker() as notif_db:
|
||||||
|
await send_user_notification(
|
||||||
|
db=notif_db,
|
||||||
|
user_id=int(account.user_id),
|
||||||
|
title="InboxRescue: Mail Processing Error",
|
||||||
|
body=f"Error processing mail account '{account.name}': {e}",
|
||||||
|
notify_on_error=True,
|
||||||
|
)
|
||||||
|
except Exception as notify_exc:
|
||||||
|
logger.warning(f"Failed to send error notification: {notify_exc}")
|
||||||
|
|
||||||
|
|
||||||
@celery_app.task(base=AsyncTask, name="app.workers.tasks.process_all_enabled_accounts")
|
@celery_app.task(base=AsyncTask, name="app.workers.tasks.process_all_enabled_accounts")
|
||||||
async def process_all_enabled_accounts():
|
async def process_all_enabled_accounts():
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ Comprehensive task breakdown for repository improvements and production readines
|
|||||||
|
|
||||||
## ✅ Recently Completed
|
## ✅ Recently Completed
|
||||||
|
|
||||||
|
- [x] Fixed worker `send_user_notification` using rolled-back DB session causing `greenlet_spawn has not been called` errors; status/`last_check_at` now always committed before sending notifications via a fresh session.
|
||||||
- [x] **Pull Now**: Added "Pull Now" button on Accounts page that immediately queues a `process_mail_account` Celery task via `POST /mail-accounts/{id}/pull-now`. Button shows spinner while in flight and is disabled for inactive accounts.
|
- [x] **Pull Now**: Added "Pull Now" button on Accounts page that immediately queues a `process_mail_account` Celery task via `POST /mail-accounts/{id}/pull-now`. Button shows spinner while in flight and is disabled for inactive accounts.
|
||||||
- [x] Fixed 21 mypy type errors: `Column[T]` vs native type mismatches in `notification_service.py`, `mail_processor.py`, `auth.py`, `tasks.py`, `providers.py`, `mail_accounts.py`, and `main.py` (`lifespan` parameter rename).
|
- [x] Fixed 21 mypy type errors: `Column[T]` vs native type mismatches in `notification_service.py`, `mail_processor.py`, `auth.py`, `tasks.py`, `providers.py`, `mail_accounts.py`, and `main.py` (`lifespan` parameter rename).
|
||||||
- [x] **Provider logos rework**: Logos now displayed as full-width banner strips at the top of each account card using `next/image fill + object-contain`. Handles all aspect ratios (1:1 square to 6:1 wordmark) without distortion. Proton Mail added.
|
- [x] **Provider logos rework**: Logos now displayed as full-width banner strips at the top of each account card using `next/image fill + object-contain`. Handles all aspect ratios (1:1 square to 6:1 wordmark) without distortion. Proton Mail added.
|
||||||
|
|||||||
Reference in New Issue
Block a user