Skip to content

fix(worker): rollback session before marking commit error in process_upload - #2560

Open
sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/worker-process-upload-rollback-pending-rollback-error
Open

sentry[bot] wants to merge 1 commit into
mainfrom
seer/fix/worker-process-upload-rollback-pending-rollback-error

Conversation

@sentry

@sentry sentry Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Problem

When an error occurs during upload processing that invalidates the database connection (for example, a PostgreSQL OperationalError: server closed the connection unexpectedly), the except Exception: block in process_upload immediately attempted to set commit.state = "error" and call db_session.commit(). Because the invalidated transaction was not rolled back first, SQLAlchemy raised a PendingRollbackError: Can't reconnect until invalid transaction is rolled back., masking the root cause and preventing the commit state update from being recorded.

Solution

In apps/worker/services/processing/processing.py:

  1. Log the original exception immediately via log.exception("Could not properly process commit") so the primary error context is never obscured.
  2. Call db_session.rollback() prior to any subsequent database operations to clear invalid transaction state and allow reconnecting.
  3. Wrap the commit.state = "error" assignment and db_session.commit() in a guarded try/except SQLAlchemyError: block so secondary persistence failures are logged and rolled back without displacing the original exception.
  4. Re-raise the original exception so upstream retry and monitoring mechanisms function as intended.

Legal Boilerplate

Look, I get it. The entity doing business as "Codecov" is owned by Harness, Inc. In 2026 Harness acquired Codecov and as a result Harness is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Harness can use, modify, copy, and redistribute my contributions, under Harness's choice of terms.

Fixes WORKER-ZAP

@sentry <feedback>: Autofix iterates on these changes
@sentry stop iterating: Autofix stops iterating on this run

This PR was automatically generated by Sentry. You can adjust this setting at any time.


Note

Medium Risk
Changes error-path DB session handling in upload processing; behavior on success is unchanged, but failed uploads may now reliably record error state when the connection recovers after rollback.

Overview
Hardens upload processing failure handling so a dropped or invalidated DB connection no longer hides the real error or blocks marking the commit as failed.

On any non-Celery exception in process_upload, the worker now logs the original failure first, rollback()s the session before retrying DB work, then attempts commit.state = "error" inside a SQLAlchemyError guard that logs and rolls back if persistence fails. The original exception is still re-raised for retries and monitoring.

Reviewed by Cursor Bugbot for commit 5825f59. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.64%. Comparing base (c6a2a28) to head (5825f59).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
apps/worker/services/processing/processing.py 62.50% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2560      +/-   ##
==========================================
- Coverage   91.65%   91.64%   -0.01%     
==========================================
  Files        1341     1341              
  Lines       53342    53348       +6     
  Branches     1649     1649              
==========================================
+ Hits        48888    48891       +3     
- Misses       4133     4136       +3     
  Partials      321      321              
Flag Coverage Δ
workerintegration 58.47% <12.50%> (-0.02%) ⬇️
workerunit 90.53% <62.50%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@sentry

sentry Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

⚠️ Sentry needs additional GitHub App permissions

The Sentry GitHub App installation for this repository is missing permissions it needs to keep iterating on this pull request to get CI passing.

Review and accept the updated permissions to let Seer continue: https://github.com/organizations/codecov/settings/installations/86101127/permissions/update

This branch has not been deployed

No deployments
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.

0 participants