Skip to content

fix(scrapy): Close AsyncThread on scheduler open() failure#820

Merged
vdusek merged 1 commit intomasterfrom
fix/close-async-thread-on-scheduler-open-failure
Mar 2, 2026
Merged

fix(scrapy): Close AsyncThread on scheduler open() failure#820
vdusek merged 1 commit intomasterfrom
fix/close-async-thread-on-scheduler-open-failure

Conversation

@vdusek
Copy link
Contributor

@vdusek vdusek commented Mar 2, 2026

Summary

  • Close AsyncThread in the except block of ApifyScheduler.open() before re-raising, preventing a thread leak when open_rq() fails (Scrapy only calls close() after a successful open()).

Test plan

  • CI passes

🤖 Generated with Claude Code

If `open_rq()` raises, Scrapy will not call `close()`, so the
`AsyncThread` (and its event loop thread) would leak. Close it
explicitly before re-raising.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vdusek vdusek added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Mar 2, 2026
@vdusek vdusek self-assigned this Mar 2, 2026
@github-actions github-actions bot added this to the 135th sprint - Tooling team milestone Mar 2, 2026
@codecov
Copy link

codecov bot commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.85%. Comparing base (69b8af9) to head (7113598).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/apify/scrapy/scheduler.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #820      +/-   ##
==========================================
- Coverage   85.92%   85.85%   -0.07%     
==========================================
  Files          46       46              
  Lines        2778     2779       +1     
==========================================
- Hits         2387     2386       -1     
- Misses        391      393       +2     
Flag Coverage Δ
e2e 37.06% <0.00%> (-0.02%) ⬇️
integration 58.58% <0.00%> (-0.06%) ⬇️
unit 73.40% <0.00%> (-0.03%) ⬇️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vdusek vdusek requested a review from Pijukatel March 2, 2026 12:16
@vdusek vdusek merged commit 7dfaf1a into master Mar 2, 2026
30 of 31 checks passed
@vdusek vdusek deleted the fix/close-async-thread-on-scheduler-open-failure branch March 2, 2026 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants