Skip to content

Fix worker lifecycle after internal error notifications - #1384

Open
LeulTew wants to merge 1 commit into
pytest-dev:masterfrom
LeulTew:leultew-worker-error-notification-lifecycle
Open

LeulTew wants to merge 1 commit into
pytest-dev:masterfrom
LeulTew:leultew-worker-error-notification-lifecycle

Conversation

@LeulTew

@LeulTew LeulTew commented Sep 16, 2026 •

Copy link
Copy Markdown

Keep workers active after internal-error notifications until they actually finish or disconnect, preventing duplicate-removal and no-active-worker errors. Report-only notifications remain nonfatal; confirmed fatal worker exits are propagated after worker shutdown.

Fixes #986.

Validation limits: Local testing used Windows/Python 3.14 and excluded five cases requiring more than two workers. Two pytestmain CI jobs fail grouping checks that also fail on the base commit in a controlled local comparison.

Implementation and validation

config.notify_exception() reports an exception but does not itself terminate a worker. Active-worker removal now remains with workerfinished and errordown; a confirmed INTERNAL_ERROR completion triggers shutdown and exit code 3 after the remaining worker events are processed.

Real pytester/execnet subprocess cases cover the original collection notifications, report-only finish notifications, fatal collection/runtest hooks, and notification followed by a crash/restart. Native pytest comparisons establish exit 0 for report-only finish notifications and exit 3 for the collection reproducer's subsequent pytest capture failure; the fix preserves these outcomes without xdist bookkeeping errors.

  • Final regression selection: 9 failures on unchanged code, all 10 cases passing with the fix.
  • Broader lifecycle suites: 175 passed, 3 skipped, 5 deselected, 8 expected failures, 1 non-strict unexpected pass.
  • Changed-file Ruff, formatting, RST, and mypy checks passed; towncrier draft rendering passed.

The five excluded cases use three or four workers; no tests, restart limits, or checks were modified to avoid running them.

The CI comparison pinned pytest to 99ab2accc, the exact revision from both failed jobs. All five affected grouping tests fail on both the base and this branch, without INTERNALERROR: that pytest revision reads _id for nodeid, while unchanged xdist worker code writes _nodeid. The comparison used Windows/Python 3.14, not CI's Python 3.11 environments. This separate compatibility problem was not changed, and the CI gate remains unresolved.

Repository checklist

Thanks for submitting a PR, your contribution is really appreciated!

Here's a quick checklist that should be present in PRs:

  • Make sure to include reasonable tests for your change if necessary

  • We use towncrier for changelog management, so please add a news file into the changelog folder following these guidelines:

    • Name it $issue_id.$type for example 588.bugfix;

    • If you don't have an issue_id change it to the PR id after creating it

    • Ensure type is one of removal, feature, bugfix, vendor, doc or trivial

    • Make sure to use full sentences with correct case and punctuation, for example:

      Fix issue with non-ascii contents in doctest text files.
      

Keep reported internal errors separate from worker termination. Wait for terminal events and drain workers before propagating a confirmed internal-error exit status.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KeyError in dsession.worker_internal_error() when a hook invokes config.notify_exception()

1 participant