Skip to content

fix!: Keep the crawler usable after a failed run and retry the browser temp directory removal - #2239

Draft
vdusek wants to merge 5 commits into
masterfrom
fix/browser-temp-dir-removal
Draft

vdusek wants to merge 5 commits into
masterfrom
fix/browser-temp-dir-removal

Conversation

@vdusek

@vdusek vdusek commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Browser temp directory

PlaywrightPersistentBrowser.close() removed its temporary profile directory with a single shutil.rmtree(ignore_errors=True) call, 100 ms after closing the context. On Windows the browser process can still hold files inside that directory open at that point, so the removal failed, ignore_errors=True swallowed the error, and the directory stayed behind. Persistent context is the default for PlaywrightCrawler, so a browser close on Windows could leak a profile directory without reporting anything.

The removal now retries until the directory is gone, up to 50 attempts 100 ms apart, and logs a warning if it never succeeds. The fixed sleep in close() is gone, so the usual case finishes as soon as the first attempt succeeds.

Both close() and the context's close listener ask for the removal. They take a lock, and whichever one acquires it first clears _temp_dir while holding it, so a single close spends the retry budget once. close() still blocks until the removal finishes, whichever coroutine ran it.

This surfaced as an intermittent failure of tests/unit/browsers/test_playwright_browser.py::test_delete_temp_folder_with_close_browser on windows-latest (run 35085779018), where every other job of the same commit passed.

Crawler state after a failed run

These follow up on #2229, released in 1.10.1.

BasicCrawler.run() reset _running only around the await run_task phase, so an exception raised during setup (add_requests failing, for example) left _running set, and every later run() raised RuntimeError('This crawler instance is already running...'). The whole run body is now inside the try, with the reset in its finally.

#2229 also documented that "a run that ended with an exception does not count as a previous run, so a retry keeps the requests that were still pending". That holds only while no earlier run has succeeded: once _has_finished_before is set, the next run purges the queue and drops whatever the failed run left pending. A _last_run_failed flag now gates the implicit purge, which makes the documented behavior true in both cases.

The post-crawl steps, statistics logging and _save_crawler_state(), sit inside the same try, so a failure there marks the run as failed too. A _save_crawler_state() error otherwise left _has_finished_before set and _last_run_failed clear, and the next run purged the queue and dropped its pending requests.

The purge exemption also wins over an explicit purge_request_queue=True, which the docstring now states.

AutoscaledPool

run.result.exception() is guarded against a cancelled future. BasicCrawler cancels the run task on SIGINT, which cancels run.result; the unguarded call then raised CancelledError inside the orchestrator's finally and skipped the rest of it, including the error log below. The worker drain it also skipped is covered by run()'s own finally, so the visible effect is the lost traceback.

The orchestrator's fallback error message is now distinct from the identical one run() already emits, so the two paths can be told apart in logs.

Breaking change

BREAKING CHANGE: the AutoscaledPool orchestrator's fallback error log message changed from Exception in worker task orchestrator to Unpropagated exception in worker task orchestrator. The old wording shipped in 1.10.1, so log-based tooling that matches on it needs updating. The identical message emitted by run() keeps the old wording.

✍️ Drafted by Claude Code

@vdusek vdusek added t-tooling Issues with this label are in the ownership of the tooling team. adhoc Ad-hoc unplanned task added during the sprint. labels Sep 16, 2026
@vdusek vdusek self-assigned this Sep 16, 2026
@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.30435% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.73%. Comparing base (cb7822e) to head (ef02218).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
src/crawlee/crawlers/_basic/_basic_crawler.py 87.50% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2239      +/-   ##
==========================================
- Coverage   93.75%   93.73%   -0.03%     
==========================================
  Files         181      181              
  Lines       12877    12897      +20     
==========================================
+ Hits        12073    12089      +16     
- Misses        804      808       +4     
Flag Coverage Δ
unit 93.73% <91.30%> (-0.03%) ⬇️

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.

🚀 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 changed the title fix(browsers): Retry the temp directory removal until the browser releases its files fix: Keep the crawler usable after a failed run and retry the browser temp directory removal Sep 16, 2026
@vdusek vdusek changed the title fix: Keep the crawler usable after a failed run and retry the browser temp directory removal fix!: Keep the crawler usable after a failed run and retry the browser temp directory removal Sep 16, 2026
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