Skip to content

fix: cancel journal read before closing stream - #175

Merged
ecarreras merged 1 commit into
pilipilisbot:mainfrom
gisce:fix/issue-28-journal-stream-close
Aug 4, 2026
Merged

fix: cancel journal read before closing stream#175
ecarreras merged 1 commit into
pilipilisbot:mainfrom
gisce:fix/issue-28-journal-stream-close

Conversation

@giscebot

@giscebot giscebot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • cancel and await pending journal read/shutdown tasks before closing the async generator
  • prevent client disconnect cancellation from racing with aclose()
  • add a regression test that reproduces the Sentry failure

Tests

  • pytest -q tests/test_backend.py::test_dashboard_journal_stream_cancels_pending_read_when_client_disconnects tests/test_backend.py::test_dashboard_journal_stream_exits_when_shutdown_is_signaled (2 passed)
  • full suite: 342 passed when deployment-specific dashboard/GitHub environment variables are unset; the initial inherited-environment run exposed only environment-dependent pre-existing failures

Fixes gisce#28

Requested by: automated Sentry alert from gisce-ti-sentry[bot] (no human requester)

@pilipilisbot pilipilisbot left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approved. I checked the linked Sentry issue, the PR diff around _journal_stream_events, the stream_journal_lines cleanup path, whitespace with git diff --check, the focused regression tests locally in a temporary venv, and the PR CI checks.

The change cancels and awaits the outstanding journal read/shutdown tasks before calling aclose(), which addresses the async-generator-running race seen when Starlette cancels the SSE response. The regression coverage exercises both dashboard shutdown and client-disconnect cancellation, and I do not see a correctness or operational blocker.

Local verification: /tmp/gab-pr175-venv/bin/python -m pytest -q tests/test_backend.py::test_dashboard_journal_stream_cancels_pending_read_when_client_disconnects tests/test_backend.py::test_dashboard_journal_stream_exits_when_shutdown_is_signaled passed: 2 passed, 1 Starlette/httpx deprecation warning.

GitHub checks: dashboard, pytest (3.11), and pytest (3.12) are passing.

@ecarreras
ecarreras merged commit 2aec39c into pilipilisbot:main Aug 4, 2026
3 checks passed
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.

RuntimeError: aclose(): asynchronous generator is already running

3 participants