diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md deleted file mode 100644 index 98a74f8ce..000000000 --- a/.github/pull_request_template.md +++ /dev/null @@ -1,31 +0,0 @@ - diff --git a/.github/workflows/auto-close-llm-pr.yml b/.github/workflows/auto-close-llm-pr.yml deleted file mode 100644 index 15120b0d9..000000000 --- a/.github/workflows/auto-close-llm-pr.yml +++ /dev/null @@ -1,50 +0,0 @@ -name: Auto-close LLM PRs -# The workflow only reads the pull request body through the API, it never -# checks out or runs pull request code, so pull_request_target is safe here. -on: # zizmor: ignore[dangerous-triggers] - pull_request_target: - types: [opened] -permissions: - contents: read - pull-requests: write -jobs: - close-llm-pr: - name: Close PR if marked as LLM-written - runs-on: ubuntu-latest - steps: - - name: Check PR body and close if LLM-written - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - const marker = "This PR was written entirely using an LLM"; - const { owner, repo } = context.repo; - const prNumber = context.payload.pull_request && context.payload.pull_request.number; - if (!prNumber) { - console.log('No pull request number found in context; exiting.'); - return; - } - const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber }); - const body = pr.body || ""; - if (body.includes(marker)) { - if (pr.state === 'closed') { - console.log(`PR #${prNumber} already closed.`); - return; - } - await github.rest.issues.addLabels({ - owner, - repo, - issue_number: prNumber, - labels: ['spam'] - }); - await github.rest.issues.createComment({ - owner, - repo, - issue_number: prNumber, - body: "Closing this PR because it contains the disclosure: \"This PR was written entirely using an LLM\"." - }); - await github.rest.pulls.update({ owner, repo, pull_number: prNumber, state: 'closed' }); - console.log(`Closed PR #${prNumber} because marker was found.`); - } else { - console.log(`Marker not found in PR #${prNumber}; nothing to do.`); - } diff --git a/.github/workflows/flag-prs-for-triage.yml b/.github/workflows/flag-prs-for-triage.yml new file mode 100644 index 000000000..5ef8fa24a --- /dev/null +++ b/.github/workflows/flag-prs-for-triage.yml @@ -0,0 +1,255 @@ +name: Flag PRs for triage +# Labels pull requests whose author's public activity suggests that an LLM is +# writing them without supervision, and records the evidence in the workflow +# run summary so that triaging one does not require reading a user profile. +# +# Four independent signals, any of which is enough to label. Each one abstains +# when the data it needs is unavailable, so a missing signal never counts +# against an author: +# +# - Rejection burst: pull requests of theirs closed unmerged elsewhere within +# the last month. Volume of rejections in absolute terms separates spraying +# from ordinary contribution far better than a merge ratio does, since +# ratios reward authors who accumulate merges in trivial repositories. +# - Spray breadth: unrelated repositories they open pull requests against +# within one week. Breadth catches an agent on its first day, before any of +# its pull requests have been closed, and it comes from the event feed, so it +# also covers authors that the search API refuses to return. +# - Assistant voice: their recent comments across GitHub read as assistant +# output rather than as a developer talking, by section headings, bullet +# lists, em dash density or stock acknowledgement phrases. +# - Agent branch: the branch name carries an agent prefix. +# +# Authors that the organisations behind this repository already trust are left +# alone before any of that runs: public members of those organisations, and +# authors with a track record of pull requests merged into their repositories. +# Trust from a merge record rather than from a list of names keeps the exemption +# in step with who is actually contributing. +# +# Deliberately not used: account age, fork age, follower count, total pull +# request count and cross-repository merge ratio. All of them were measured +# against hand-labelled pull requests and either failed to separate or, in the +# case of the merge ratio, inverted on held-out data. +# +# The label is advisory, and it says the author's history is worth a look +# before reviewing in depth; it does not say the pull request is bad. +# +# The workflow only reads pull request and public activity metadata through the +# API, it never checks out or runs pull request code, so pull_request_target is +# safe here. +on: # zizmor: ignore[dangerous-triggers] + pull_request_target: + types: [opened] +permissions: + contents: read + pull-requests: write +jobs: + flag-pr-for-triage: + name: Label PR if the author's activity suggests unsupervised LLM use + runs-on: ubuntu-latest + steps: + - name: Score the author and label the PR + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const LABEL = 'needs triage'; + const RETRIES = 5; + const RETRY_WAIT_MS = 60000; + const REJECTION_WINDOW_DAYS = 30; + const MIN_REJECTIONS = 1; + const MIN_COMMENTS = 2; + const MAX_REPOS_PER_WEEK = 2; + const VOICE = { structure: 0.10, emDashPerKChar: 0.30, acknowledgement: 0.40 }; + const EVENT_PAGES = 3; + const TRUSTED_ORGS = ['scrapy', 'scrapy-plugins', 'scrapinghub', 'zytedata']; + const MIN_TRUSTED_MERGES = 10; + const AGENT_BRANCH = /^(agent|codex|claude|cursor|devin|copilot|jules|bot)[\/_-]/i; + + const { owner, repo } = context.repo; + const pr = context.payload.pull_request; + const author = pr.user.login; + + if (pr.user.type === 'Bot' + || ['MEMBER', 'OWNER', 'COLLABORATOR'].includes(pr.author_association)) { + core.info(`Skipping PR #${pr.number} by ${author} (${pr.user.type}, ${pr.author_association}).`); + return; + } + + // Rate and abuse limits reset on the order of a minute, so waiting + // is enough; other errors are not worth retrying. + const retriable = new Set([403, 429, 500, 502, 503, 504]); + const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); + async function withRetries(description, call) { + for (let attempt = 1; ; attempt++) { + try { + return await call(); + } catch (error) { + if (!retriable.has(error.status) || attempt > RETRIES) throw error; + const reset = Number(error.response?.headers?.['x-ratelimit-reset']) * 1000 - Date.now(); + const after = Number(error.response?.headers?.['retry-after']) * 1000; + const wait = Math.min(Math.max(after || reset || RETRY_WAIT_MS, RETRY_WAIT_MS), 15 * RETRY_WAIT_MS); + core.info(`${description} failed with ${error.status}, retrying in ${Math.round(wait / 1000)}s (attempt ${attempt}/${RETRIES}).`); + await sleep(wait); + } + } + } + // Accounts excluded from search, deleted users and the like leave a + // signal unmeasurable rather than negative. + const orNull = promise => promise.catch(error => { + if ([404, 410, 422].includes(error.status)) return null; + throw error; + }); + + // author_association only reports membership of the organisation + // that owns this repository, and only when it is public, so trust + // in the author is established here instead. + const trustedOrg = (await Promise.all(TRUSTED_ORGS.map(org => + orNull(withRetries(`Checking public membership of ${org}`, () => + github.rest.orgs.checkPublicMembershipForUser({ org, username: author }), + )).then(response => response && org), + ))).find(Boolean); + if (trustedOrg) { + core.info(`Skipping PR #${pr.number} by ${author} (public member of ${trustedOrg}).`); + return; + } + // Repeating a qualifier narrows the search instead of widening it, + // hence the explicit disjunction. + const trustedMerges = await orNull(withRetries('Counting merged PRs in trusted organisations', () => + github.rest.search.issuesAndPullRequests({ + q: `author:${author} type:pr is:merged` + + ` (${TRUSTED_ORGS.map(org => `org:${org}`).join(' OR ')})`, + advanced_search: 'true', per_page: 1, + }).then(response => response.data.total_count), + )); + if (trustedMerges >= MIN_TRUSTED_MERGES) { + core.info(`Skipping PR #${pr.number} by ${author}` + + ` (${trustedMerges} PR(s) merged into ${TRUSTED_ORGS.join(', ')}).`); + return; + } + + const opened = new Date(pr.created_at); + const daysBefore = date => (opened - new Date(date)) / 86400000; + + // Signal 1: pull requests closed unmerged elsewhere, recently. + const search = await orNull(withRetries('Searching for PRs by the author', () => + github.rest.search.issuesAndPullRequests({ + q: `author:${author} type:pr`, advanced_search: 'true', + sort: 'created', order: 'desc', per_page: 100, + }).then(response => response.data), + )); + let rejections = null; + if (search) { + rejections = search.items.filter(item => { + const itemOwner = item.repository_url.split('/repos/')[1].split('/')[0].toLowerCase(); + return itemOwner !== author.toLowerCase() + && item.state === 'closed' && !item.pull_request?.merged_at + && daysBefore(item.created_at) >= 0 + && daysBefore(item.created_at) <= REJECTION_WINDOW_DAYS; + }).map(item => item.html_url); + } + + // Signal 2: how their recent comments across GitHub read. + const events = []; + for (let page = 1; page <= EVENT_PAGES; page++) { + const batch = await orNull(withRetries(`Reading public events page ${page}`, () => + github.rest.activity.listPublicEventsForUser({ + username: author, per_page: 100, page, + }).then(response => response.data), + )); + if (!batch?.length) break; + events.push(...batch); + if (batch.length < 100) break; + } + const comments = events + .filter(event => ['IssueCommentEvent', 'PullRequestReviewCommentEvent'].includes(event.type)) + .map(event => event.payload?.comment?.body) + .filter(Boolean); + + // Signal 3: how many unrelated projects they open pull requests + // against in a single week. Breadth rather than volume: a focused + // contributor sends many pull requests to few repositories, while + // an unattended agent sprays a few across many. Taken from the + // event feed, which unlike search covers authors that search + // refuses to return. + const weeks = {}; + for (const event of events) { + if (event.type !== 'PullRequestEvent' || event.payload?.action !== 'opened') continue; + const name = event.repo?.name; + if (!name || name.toLowerCase().startsWith(`${author.toLowerCase()}/`)) continue; + const week = Math.floor(new Date(event.created_at) / (7 * 86400000)); + (weeks[week] ??= new Set()).add(name); + } + const breadth = events.length + ? Math.max(0, ...Object.values(weeks).map(repos => repos.size)) + : null; + const STRUCTURE = [/^\s*#{2,3}\s/m, /^\s*[-*]\s.+\n\s*[-*]\s/m, /\*\*[^*]+\*\*/, /```/]; + const ACKNOWLEDGEMENT = [ + /thanks for (the )?(review|feedback|pointing|catching|flagging|clarif)/i, + /you'?re (absolutely )?right/i, /great catch/i, /that makes sense/i, + /i'?ll (continue|investigate|update|submit|look into|make sure)/i, + /let me know (if|whether)/i, /happy to (update|adjust|revise|change)/i, + /i understand that/i, /thanks for your time/i, /just following up/i, + /hope (this|that) helps/i, /please let me know/i, /i'?ve (updated|addressed|fixed)/i, + ]; + let voice = null; + if (comments.length >= MIN_COMMENTS) { + const chars = comments.reduce((total, body) => total + body.length, 0); + const rate = patterns => comments.filter(body => patterns.some(re => re.test(body))).length / comments.length; + voice = { + comments: comments.length, + structure: rate(STRUCTURE), + acknowledgement: rate(ACKNOWLEDGEMENT), + emDashPerKChar: 1000 * comments.reduce((total, body) => total + (body.match(/—/g) || []).length, 0) / chars, + }; + } + + const reasons = []; + if (rejections && rejections.length >= MIN_REJECTIONS) { + reasons.push(`${rejections.length} PR(s) of theirs closed unmerged elsewhere in the last` + + ` ${REJECTION_WINDOW_DAYS} days: ${rejections.slice(0, 10).join(' ')}`); + } + if (voice && (voice.structure > VOICE.structure + || voice.emDashPerKChar > VOICE.emDashPerKChar + || voice.acknowledgement > VOICE.acknowledgement)) { + reasons.push(`comment style over ${voice.comments} recent comments:` + + ` ${(100 * voice.structure).toFixed(0)}% structured,` + + ` ${(100 * voice.acknowledgement).toFixed(0)}% stock acknowledgements,` + + ` ${voice.emDashPerKChar.toFixed(2)} em dashes per 1000 characters`); + } + if (breadth !== null && breadth > MAX_REPOS_PER_WEEK) { + reasons.push(`opened pull requests against ${breadth} unrelated repositories within a week`); + } + if (AGENT_BRANCH.test(pr.head?.ref || '')) { + reasons.push(`branch name carries an agent prefix: ${pr.head.ref}`); + } + + await core.summary + .addHeading(`PR #${pr.number} by ${author}`, 3) + .addList([ + rejections === null + ? 'recent rejections elsewhere: unmeasurable, the author cannot be searched' + : `recent rejections elsewhere: ${rejections.length}`, + voice === null + ? `comment style: unmeasurable, fewer than ${MIN_COMMENTS} recent comments found` + : `comment style: ${(100 * voice.structure).toFixed(0)}% structured,` + + ` ${(100 * voice.acknowledgement).toFixed(0)}% stock acknowledgements,` + + ` ${voice.emDashPerKChar.toFixed(2)} em dashes per 1000 characters` + + ` over ${voice.comments} comments`, + breadth === null + ? 'repositories per week: unmeasurable, no public events found' + : `repositories per week, at most: ${breadth}`, + `branch: ${pr.head?.ref ?? 'unknown'}`, + `verdict: ${reasons.length ? `labelled "${LABEL}"` : 'not labelled'}`, + ]) + .addRaw(reasons.length ? `\n${reasons.map(reason => `- ${reason}`).join('\n')}\n` : '') + .write(); + + if (!reasons.length) { + core.info(`Not labelling PR #${pr.number}.`); + return; + } + await withRetries('Adding the label', () => + github.rest.issues.addLabels({ owner, repo, issue_number: pr.number, labels: [LABEL] }), + ); + core.info(`Labelled PR #${pr.number}: ${reasons.join(' | ')}`); diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index cd726a2fe..b58b2af7b 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -86,6 +86,7 @@ jobs: - python-version: pypy3.11-7.3.20 env: TOXENV: pypy3-extra-deps + coverage: true - python-version: "3.14" env: TOXENV: botocore diff --git a/.github/workflows/tests-vcs-deps.yml b/.github/workflows/tests-vcs-deps.yml new file mode 100644 index 000000000..93f262b62 --- /dev/null +++ b/.github/workflows/tests-vcs-deps.yml @@ -0,0 +1,56 @@ +name: VCS dependencies + +permissions: + contents: read + +on: + schedule: + - cron: '0 4 * * *' + workflow_dispatch: + +concurrency: + group: ${{github.workflow}}-${{ github.ref }} + cancel-in-progress: true + +jobs: + tests: + name: tests + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + # A development branch of a dependency can make a test hang forever, so + # tests get a time limit here that they do not need elsewhere. + PYTEST_ADDOPTS: -n auto --no-cov --timeout=120 + TOXENV: vcs-deps + UV_PYTHON_PREFERENCE: only-system + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.14" + + # Dependencies that ship wheels on PyPI are built from source here, so + # their build dependencies are needed: libxml2 and libxslt for lxml, + # libjpeg and zlib for Pillow, and autotools for the libuv bundled in + # uvloop. + - name: Install system libraries + run: | + sudo apt-get update + sudo apt-get install automake libjpeg-dev libtool libxml2-dev libxslt-dev zlib1g-dev + + - name: Set up uv + uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + with: + cache-dependency-glob: | + pyproject.toml + tox.ini + + - name: Install mitmproxy + run: uv tool install --python cpython mitmproxy + + - name: Run tests + run: uvx --with tox-uv tox diff --git a/docs/conf.py b/docs/conf.py index de722baac..ad55231bc 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -31,9 +31,14 @@ extensions = [ "sphinx_scrapy", "scrapyfixautodoc", # Must be after "sphinx.ext.autodoc" "sphinx.ext.coverage", + "sphinx_reredirects", "sphinx_rtd_dark_mode", ] +redirects = { + "topics/broad-crawls": "optimize.html#broad-crawls", +} + templates_path = ["_templates"] exclude_patterns = ["build", "Thumbs.db", ".DS_Store"] @@ -141,6 +146,8 @@ coverage_ignore_pyobjects = [ r"^scrapy\.linkextractors\.lxmlhtml\.LxmlParserLinkExtractor", ] +# -- Options for the autodoc extension ---------------------------------------- +autodoc_member_order = "bysource" # -- Options for the InterSphinx extension ----------------------------------- # https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html#configuration diff --git a/docs/faq.rst b/docs/faq.rst index 80658a5bf..ef3d07a2b 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -292,7 +292,7 @@ Does Scrapy manage cookies automatically? Yes, Scrapy receives and keeps track of cookies sent by servers, and sends them back on subsequent requests, like any regular web browser does. -For more info see :ref:`topics-request-response` and :ref:`cookies-mw`. +For more info see :ref:`cookies`. How can I see the cookies being sent and received from Scrapy? -------------------------------------------------------------- diff --git a/docs/index.rst b/docs/index.rst index 688cab81b..de06e3488 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -78,6 +78,7 @@ Basic concepts topics/item-pipeline topics/feed-exports topics/request-response + topics/cookies topics/link-extractors topics/settings topics/exceptions @@ -109,6 +110,9 @@ Basic concepts :doc:`topics/request-response` Understand the classes used to represent HTTP requests and responses. +:doc:`topics/cookies` + Send and receive cookies. + :doc:`topics/link-extractors` Convenient classes to extract links to follow from pages. @@ -152,7 +156,7 @@ Solving specific problems topics/contracts topics/practices topics/security - topics/broad-crawls + topics/optimize topics/developer-tools topics/dynamic-content topics/leaks @@ -180,8 +184,8 @@ Solving specific problems Understand the security implications of Scrapy defaults and how to harden them. -:doc:`topics/broad-crawls` - Tune Scrapy for crawling a lot domains in parallel. +:doc:`topics/optimize` + Find the bottleneck of your crawls and learn how to address it. :doc:`topics/developer-tools` Learn how to scrape with your browser's developer tools. diff --git a/docs/intro/install.rst b/docs/intro/install.rst index cba8c15a1..866c5abe2 100644 --- a/docs/intro/install.rst +++ b/docs/intro/install.rst @@ -111,8 +111,6 @@ The following extras are available: - Provides * - ``bpython`` - :ref:`bpython shell ` - * - ``brotli`` - - :ref:`Brotli response decompression ` * - ``gcs`` - :ref:`Google Cloud Storage ` for :ref:`feed exports ` and diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index eaf492c95..efade47e6 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -72,6 +72,11 @@ This will create a ``tutorial`` directory with the following contents:: spiders/ # a directory where you'll later put your spiders __init__.py +Before crawling anything, open ``settings.py`` and uncomment the +:setting:`USER_AGENT` line to identify yourself, e.g. a project name plus a URL +or an email address. Website owners who take issue with your crawler can then +ask you to adjust it, rather than block it. + Our first Spider ================ diff --git a/docs/news.rst b/docs/news.rst index 670843e0e..a5cc6c723 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -2042,6 +2042,13 @@ Backward-incompatible changes ``process_start_requests()`` has been replaced by ``process_start()``. (:issue:`6729`) +- The ``scrape_func`` callable passed to + ``scrapy.core.spidermw.SpiderMiddlewareManager.scrape_response()`` is now + called with 2 parameters, ``response`` and ``request``, instead of 3, and + must return a :class:`~twisted.internet.defer.Deferred` instead of an + iterable. + (:issue:`6787`) + - The now-deprecated ``start_requests()`` method, when it returns an iterable instead of being defined as a generator, is now executed *after* the :ref:`scheduler ` instance has been created. diff --git a/docs/requirements.in b/docs/requirements.in index 3783dd1dc..77514642a 100644 --- a/docs/requirements.in +++ b/docs/requirements.in @@ -3,6 +3,7 @@ pydantic scrapy-spider-metadata sphinx sphinx-notfound-page +sphinx-reredirects sphinx-rtd-theme sphinx-rtd-dark-mode sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.10 diff --git a/docs/requirements.txt b/docs/requirements.txt index 0f5969401..b0a6d0b04 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -134,6 +134,7 @@ sphinx==9.1.0 # sphinx-llms-txt # sphinx-markdown-builder # sphinx-notfound-page + # sphinx-reredirects # sphinx-rtd-theme # sphinx-scrapy # sphinxcontrib-jquery @@ -147,6 +148,8 @@ sphinx-markdown-builder @ git+https://github.com/zytedata/sphinx-markdown-builde # via sphinx-scrapy sphinx-notfound-page==1.1.0 # via -r docs/requirements.in +sphinx-reredirects==1.1.0 + # via -r docs/requirements.in sphinx-rtd-dark-mode==1.3.0 # via -r docs/requirements.in sphinx-rtd-theme==3.1.0 diff --git a/docs/topics/api.rst b/docs/topics/api.rst index db400ed8c..8ac52e082 100644 --- a/docs/topics/api.rst +++ b/docs/topics/api.rst @@ -35,6 +35,13 @@ how you :ref:`configure the downloader middlewares :class:`scrapy.Spider` subclass and a :class:`scrapy.settings.Settings` object. + The :attr:`engine`, :attr:`extensions`, :attr:`logformatter`, + :attr:`request_fingerprinter` and :attr:`stats` attributes get their value + when the crawl starts, and raise :exc:`RuntimeError` when read before that. + + .. versionchanged:: VERSION + Those attributes used to be ``None`` before getting their value. + .. attribute:: request_fingerprinter The request fingerprint builder of this crawler. diff --git a/docs/topics/broad-crawls.rst b/docs/topics/broad-crawls.rst deleted file mode 100644 index cace1f883..000000000 --- a/docs/topics/broad-crawls.rst +++ /dev/null @@ -1,194 +0,0 @@ -.. _topics-broad-crawls: - -============ -Broad Crawls -============ - -Scrapy defaults are optimized for crawling specific sites. These sites are -often handled by a single Scrapy spider, although this is not necessary or -required (for example, there are generic spiders that handle any given site -thrown at them). - -In addition to this "focused crawl", there is another common type of crawling -which covers a large (potentially unlimited) number of domains, and is only -limited by time or other arbitrary constraint, rather than stopping when the -domain was crawled to completion or when there are no more requests to perform. -These are called "broad crawls" and is the typical crawlers employed by search -engines. - -These are some common properties often found in broad crawls: - -* they crawl many domains (often, unbounded) instead of a specific set of sites - -* they don't necessarily crawl domains to completion, because it would be - impractical (or impossible) to do so, and instead limit the crawl by time or - number of pages crawled - -* they are simpler in logic (as opposed to very complex spiders with many - extraction rules) because data is often post-processed in a separate stage - -* they crawl many domains concurrently, which allows them to achieve faster - crawl speeds by not being limited by any particular site constraint (each site - is crawled slowly to respect politeness, but many sites are crawled in - parallel) - -As said above, Scrapy default settings are optimized for focused crawls, not -broad crawls. However, due to its asynchronous architecture, Scrapy is very -well suited for performing fast broad crawls. This page summarizes some things -you need to keep in mind when using Scrapy for doing broad crawls, along with -concrete suggestions of Scrapy settings to tune in order to achieve an -efficient broad crawl. - -.. _broad-crawls-scheduler-priority-queue: - -.. _broad-crawls-concurrency: - -Increase concurrency -==================== - -Concurrency is the number of requests that are processed in parallel. There is -a global limit (:setting:`CONCURRENT_REQUESTS`) and an additional limit that -can be set per domain (:setting:`CONCURRENT_REQUESTS_PER_DOMAIN`). - -The default global concurrency limit in Scrapy is not suitable for crawling -many different domains in parallel, so you will want to increase it. How much -to increase it will depend on how much CPU and memory your crawler will have -available. - -A good starting point is ``100``: - -.. code-block:: python - - CONCURRENT_REQUESTS = 100 - -But the best way to find out is by doing some trials and identifying at what -concurrency your Scrapy process gets CPU bounded. For optimum performance, you -should pick a concurrency where CPU usage is at 80-90%. - -Increasing concurrency also increases memory usage. If memory usage is a -concern, you might need to lower your global concurrency limit accordingly. - - -Increase Twisted IO thread pool maximum size -============================================ - -Currently Scrapy does DNS resolution in a blocking way with usage of thread -pool. With higher concurrency levels the crawling could be slow or even fail -hitting DNS resolver timeouts. Possible solution to increase the number of -threads handling DNS queries. The DNS queue will be processed faster speeding -up establishing of connection and crawling overall. - -To increase maximum thread pool size use: - -.. code-block:: python - - REACTOR_THREADPOOL_MAXSIZE = 20 - -Setup your own DNS -================== - -If you have multiple crawling processes and single central DNS, it can act -like DoS attack on the DNS server resulting to slow down of entire network or -even blocking your machines. To avoid this setup your own DNS server with -local cache and upstream to some large DNS like OpenDNS or Verizon. - -Reduce log level -================ - -When doing broad crawls you are often only interested in the crawl rates you -get and any errors found. These stats are reported by Scrapy when using the -``INFO`` log level. In order to save CPU (and log storage requirements) you -should not use ``DEBUG`` log level when performing large broad crawls in -production. Using ``DEBUG`` level when developing your (broad) crawler may be -fine though. - -To set the log level use: - -.. code-block:: python - - LOG_LEVEL = "INFO" - -Disable cookies -=============== - -Disable cookies unless you *really* need. Cookies are often not needed when -doing broad crawls (search engine crawlers ignore them), and they improve -performance by saving some CPU cycles and reducing the memory footprint of your -Scrapy crawler. - -To disable cookies use: - -.. code-block:: python - - COOKIES_ENABLED = False - -Disable retries -=============== - -Retrying failed HTTP requests can slow down the crawls substantially, especially -when sites causes are very slow (or fail) to respond, thus causing a timeout -error which gets retried many times, unnecessarily, preventing crawler capacity -to be reused for other domains. - -To disable retries use: - -.. code-block:: python - - RETRY_ENABLED = False - -Reduce download timeout -======================= - -Unless you are crawling from a very slow connection (which shouldn't be the -case for broad crawls) reduce the download timeout so that stuck requests are -discarded quickly and free up capacity to process the next ones. - -To reduce the download timeout use: - -.. code-block:: python - - DOWNLOAD_TIMEOUT = 15 - -Disable redirects -================= - -Consider disabling redirects, unless you are interested in following them. When -doing broad crawls it's common to save redirects and resolve them when -revisiting the site at a later crawl. This also help to keep the number of -request constant per crawl batch, otherwise redirect loops may cause the -crawler to dedicate too many resources on any specific domain. - -To disable redirects use: - -.. code-block:: python - - REDIRECT_ENABLED = False - -.. _broad-crawls-bfo: - -Crawl in BFO order -================== - -:ref:`Scrapy crawls in DFO order by default `. - -In broad crawls, however, page crawling tends to be faster than page -processing. As a result, unprocessed early requests stay in memory until the -final depth is reached, which can significantly increase memory usage. - -:ref:`Crawl in BFO order ` instead to save memory. - - -Be mindful of memory leaks -========================== - -If your broad crawl shows a high memory usage, in addition to :ref:`crawling in -BFO order ` and :ref:`lowering concurrency -` you should :ref:`debug your memory leaks -`. - - -Install a specific Twisted reactor -================================== - -If the crawl is exceeding the system's capabilities, you might want to try -installing a specific Twisted reactor, via the :setting:`TWISTED_REACTOR` setting. diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index 343193627..50da4593a 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -507,7 +507,7 @@ Supported options: * ``--cbkwargs``: additional keyword arguments that will be passed to the callback. This must be a valid json string. Example: --cbkwargs='{"foo" : "bar"}' -* ``--pipelines``: process items through pipelines +* ``--pipelines``: :ref:`process items through pipelines ` * ``--rules`` or ``-r``: use :class:`~scrapy.spiders.CrawlSpider` rules to discover the callback (i.e. spider method) to use for parsing the diff --git a/docs/topics/cookies.rst b/docs/topics/cookies.rst new file mode 100644 index 000000000..ab5f3dca0 --- /dev/null +++ b/docs/topics/cookies.rst @@ -0,0 +1,138 @@ +.. _cookies: +.. _cookies-mw: + +======= +Cookies +======= + +Scrapy keeps track of the cookies that websites set and sends them back on +later requests to those websites, just like a web browser does. That is the job +of :class:`~scrapy.downloadermiddlewares.cookies.CookiesMiddleware`, which is +enabled by default. + + +Setting cookies on a request +============================ + +.. invisible-code-block: python + + from scrapy import Request + +Use the ``cookies`` parameter of :class:`~scrapy.Request` to send cookies of +your own, either as a dict: + +.. code-block:: python + + request = Request( + url="https://example.com", + cookies={"currency": "USD", "country": "UY"}, + ) + +Or as a list of dicts, which also lets you set cookie attributes: + +.. code-block:: python + + request = Request( + url="https://example.com", + cookies=[ + { + "name": "currency", + "value": "USD", + "domain": "example.com", + "path": "/currency", + "secure": True, + }, + ], + ) + +Setting attributes is only useful if the cookies are stored for later requests, +i.e. if :reqmeta:`dont_merge_cookies` is not enabled. + +.. caution:: Cookies set through the ``Cookie`` header are not handled by + :class:`~scrapy.downloadermiddlewares.cookies.CookiesMiddleware`, which + drops that header. + +.. caution:: When a cookie name or value is a byte sequence that is not UTF-8 + encoded, the cookie is dropped and a warning is logged. See + :ref:`topics-logging-advanced-customization` to customize the logging + behavior. + + +.. reqmeta:: cookiejar + +Multiple cookie sessions per spider +=================================== + +By default all requests share a single cookie jar (session). To use different +ones, pass an identifier in the :reqmeta:`cookiejar` request meta key: + +.. skip: next +.. code-block:: python + + for i, url in enumerate(urls): + yield Request(url, meta={"cookiejar": i}, callback=self.parse_page) + +The :reqmeta:`cookiejar` meta key is not "sticky", so you need to keep passing +it along on subsequent requests: + +.. code-block:: python + + def parse_page(self, response): + return Request( + "https://example.com/otherpage", + meta={"cookiejar": response.meta["cookiejar"]}, + callback=self.parse_other_page, + ) + + +.. reqmeta:: dont_merge_cookies + +Skipping the cookie jar for a request +===================================== + +Set the :reqmeta:`dont_merge_cookies` request meta key to ``True`` to keep a +request from touching the cookie jar in either direction: no stored cookie is +sent with the request, and no cookie received in the response is stored. The +cookies of the request itself are ignored as well. + + +.. setting:: COOKIES_ENABLED + +COOKIES_ENABLED +=============== + +Default: ``True`` + +Whether to enable :class:`~scrapy.downloadermiddlewares.cookies.CookiesMiddleware`. +If disabled, no cookies are sent to web servers. + + +.. setting:: COOKIES_DEBUG + +COOKIES_DEBUG +============= + +Default: ``False`` + +If enabled, Scrapy logs all cookies sent in requests (i.e. the ``Cookie`` +header) and all cookies received in responses (i.e. the ``Set-Cookie`` +header):: + + 2011-04-06 14:35:10-0300 [scrapy.core.engine] INFO: Spider opened + 2011-04-06 14:35:10-0300 [scrapy.downloadermiddlewares.cookies] DEBUG: Sending cookies to: + Cookie: clientlanguage_nl=en_EN + 2011-04-06 14:35:14-0300 [scrapy.downloadermiddlewares.cookies] DEBUG: Received cookies from: <200 http://www.diningcity.com/netherlands/index.html> + Set-Cookie: JSESSIONID=B~FA4DC0C496C8762AE4F1A620EAB34F38; Path=/ + Set-Cookie: ip_isocode=US + Set-Cookie: clientlanguage_nl=en_EN; Expires=Thu, 07-Apr-2011 21:21:34 GMT; Path=/ + 2011-04-06 14:49:50-0300 [scrapy.core.engine] DEBUG: Crawled (200) (referer: None) + [...] + + +CookiesMiddleware +================= + +.. module:: scrapy.downloadermiddlewares.cookies + :synopsis: Cookies Downloader Middleware + +.. autoclass:: CookiesMiddleware diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index 34ab4f105..433a6d139 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -78,33 +78,15 @@ Writing your own download handler A download handler is a :ref:`component ` that defines the following API: -.. class:: SampleDownloadHandler - - .. attribute:: lazy - :type: bool - - If ``False``, the handler will be instantiated when Scrapy is - initialized. - - If ``True``, the handler will only be instantiated when the first - request handled by it needs to be downloaded. - - .. method:: download_request(request: Request) -> Response - :async: - - Download the given request and return a response. - - .. method:: close() -> None - :async: - - Clean up any resources used by the handler. +.. autoclass:: scrapy.core.downloader.handlers.DownloadHandlerProtocol + :members: An optional base class for custom handlers is provided: .. autoclass:: scrapy.core.downloader.handlers.base.BaseDownloadHandler :members: :undoc-members: - :member-order: bysource + :exclude-members: close, download_request, lazy .. _download-handlers-exceptions: @@ -148,17 +130,23 @@ using different handlers. Here is a comparison of some features of the built-in HTTP handlers, see the individual handler docs for more differences: -================== ================= ===================== ==================== -Feature H2DownloadHandler HTTP11DownloadHandler HttpxDownloadHandler -================== ================= ===================== ==================== -Requires asyncio No No Yes -Requires a reactor Yes Yes No -HTTP/1.1 No Yes Yes -HTTP/2 Yes No Yes -TLS implementation ``cryptography`` ``cryptography`` Stdlib ``ssl`` -HTTP proxies No Yes Yes -SOCKS proxies No No Yes -================== ================= ===================== ==================== +=================== ================= ===================== ==================== +Feature H2DownloadHandler HTTP11DownloadHandler HttpxDownloadHandler +=================== ================= ===================== ==================== +Requires asyncio No No Yes +Requires a reactor Yes Yes No +HTTP/1.1 No Yes Yes +HTTP/2 Yes No Yes +TLS implementation ``cryptography`` ``cryptography`` Stdlib ``ssl`` +HTTP proxies No Yes Yes +SOCKS proxies No No Yes +Bad header handling Not applicable Skip bad Fail +=================== ================= ===================== ==================== + +Bad header handling is what a handler does when a response has a bad header +line, e.g. one with no colon in it, which some servers send. Handlers that skip +bad header lines, like web browsers do, still parse the header lines that follow +them; other handlers also lose those, or cannot download such responses at all. You can find additional HTTP download handlers in the scrapy-download-handlers-incubator_ package. This package is made by the Scrapy @@ -209,6 +197,7 @@ Features and limitations HTTP proxies No (not implemented) SOCKS proxies No (not supported by the library) HTTP/2 Yes +Bad header handling Not applicable (HTTP/2 only) ``response.certificate`` :class:`twisted.internet.ssl.Certificate` object Per-request ``bindaddress`` Yes TLS implementation ``pyOpenSSL``/``cryptography`` @@ -221,9 +210,6 @@ Other limitations: - IPv6 support requires setting :setting:`TWISTED_DNS_RESOLVER` to ``scrapy.resolver.CachingHostnameResolver``. -- No support for the :signal:`bytes_received` and :signal:`headers_received` - signals. - Known limitations of the HTTP/2 support: - No support for HTTP/2 Cleartext (h2c), since no major browser supports @@ -260,11 +246,16 @@ Features and limitations HTTP proxies Yes SOCKS proxies No (not supported by the library) HTTP/2 No (implemented as a separate handler) +Bad header handling Skip bad, like web browsers do ``response.certificate`` :class:`twisted.internet.ssl.Certificate` object Per-request ``bindaddress`` Yes TLS implementation ``pyOpenSSL``/``cryptography`` =========================== ================================================ +.. versionchanged:: VERSION + Bad header lines with no colon in them are now skipped, instead of making + the whole response impossible to download. + Other limitations: - IPv6 support requires setting :setting:`TWISTED_DNS_RESOLVER` @@ -318,6 +309,7 @@ Features and limitations HTTP proxies Yes SOCKS proxies Yes (SOCKS5) HTTP/2 Yes +Bad header handling Fail (not supported by the library) ``response.certificate`` DER bytes Per-request ``bindaddress`` No (not supported by the library) TLS implementation Standard library ``ssl`` diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 10edab242..72661a52c 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -156,6 +156,61 @@ defines one or more of these methods: :param exception: the raised exception :type exception: an ``Exception`` object +.. _mw-download: + +Downloading a request from a downloader middleware +================================================== + +A downloader middleware can download a request of its own while it processes +another one, e.g. to fetch something that the request it is processing needs. +The built-in :ref:`robots.txt middleware ` does that: it +holds each request while it downloads the ``robots.txt`` file of its website. + +Use :meth:`crawler.engine.download_async() +` for that: + +.. code-block:: python + + from scrapy import Request + from scrapy.http.request import NO_CALLBACK + + + class TokenMiddleware: + def __init__(self, crawler): + self.crawler = crawler + self.token = None + + @classmethod + def from_crawler(cls, crawler): + return cls(crawler) + + async def process_request(self, request): + if request.meta.get("dont_obey_robotstxt"): + return + if self.token is None: + response = await self.crawler.engine.download_async( + Request( + "https://example.com/token", + callback=NO_CALLBACK, + meta={"dont_obey_robotstxt": True}, + ) + ) + self.token = response.text + request.headers["Authorization"] = self.token + +Requests that you download this way go through the downloader middleware chain +as well, including your own middleware and the :ref:`robots.txt middleware +`, which holds a request until the ``robots.txt`` file of +its website arrives. Be careful not to introduce deadlocks: a request that you +download must not end up waiting for the request that is waiting for it. Hence +:reqmeta:`dont_obey_robotstxt` above, which makes both middlewares let the token +request through. + +While the first token response is in transit, ``process_request`` runs for other +requests as well, and the middleware above downloads a token for each of them. +Cache the task that downloads the token, and not only its result, to download +the token only once. + .. _topics-downloader-middleware-ref: Built-in downloader middleware reference @@ -169,106 +224,10 @@ middleware, see the :ref:`downloader middleware usage guide For a list of the components enabled by default (and their orders) see the :setting:`DOWNLOADER_MIDDLEWARES_BASE` setting. -.. _cookies-mw: - CookiesMiddleware ----------------- -.. module:: scrapy.downloadermiddlewares.cookies - :synopsis: Cookies Downloader Middleware - -.. class:: CookiesMiddleware - - This middleware enables working with sites that require cookies, such as - those that use sessions. It keeps track of cookies sent by web servers, and - sends them back on subsequent requests (from that spider), just like web - browsers do. - - .. caution:: When non-UTF8 encoded byte sequences are passed to a - :class:`~scrapy.Request`, the ``CookiesMiddleware`` will log - a warning. Refer to :ref:`topics-logging-advanced-customization` - to customize the logging behaviour. - - .. caution:: Cookies set via the ``Cookie`` header are not considered by the - :ref:`cookies-mw`. If you need to set cookies for a request, use the - :class:`Request.cookies ` parameter. This is a known - current limitation that is being worked on. - -The following settings can be used to configure the cookie middleware: - -* :setting:`COOKIES_ENABLED` -* :setting:`COOKIES_DEBUG` - -.. reqmeta:: cookiejar - -Multiple cookie sessions per spider -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -There is support for keeping multiple cookie sessions per spider by using the -:reqmeta:`cookiejar` Request meta key. By default it uses a single cookie jar -(session), but you can pass an identifier to use different ones. - -For example: - -.. skip: next -.. code-block:: python - - for i, url in enumerate(urls): - yield scrapy.Request(url, meta={"cookiejar": i}, callback=self.parse_page) - -Keep in mind that the :reqmeta:`cookiejar` meta key is not "sticky". You need to keep -passing it along on subsequent requests. For example: - -.. code-block:: python - - def parse_page(self, response): - # do some processing - return scrapy.Request( - "http://www.example.com/otherpage", - meta={"cookiejar": response.meta["cookiejar"]}, - callback=self.parse_other_page, - ) - -.. setting:: COOKIES_ENABLED - -COOKIES_ENABLED -~~~~~~~~~~~~~~~ - -Default: ``True`` - -Whether to enable the cookies middleware. If disabled, no cookies will be sent -to web servers. - -Notice that despite the value of :setting:`COOKIES_ENABLED` setting if -``Request.``:reqmeta:`meta['dont_merge_cookies'] ` -evaluates to ``True`` the request cookies will **not** be sent to the -web server and received cookies in :class:`~scrapy.http.Response` will -**not** be merged with the existing cookies. - -For more detailed information see the ``cookies`` parameter in -:class:`~scrapy.Request`. - -.. setting:: COOKIES_DEBUG - -COOKIES_DEBUG -~~~~~~~~~~~~~ - -Default: ``False`` - -If enabled, Scrapy will log all cookies sent in requests (i.e. ``Cookie`` -header) and all cookies received in responses (i.e. ``Set-Cookie`` header). - -Here's an example of a log with :setting:`COOKIES_DEBUG` enabled:: - - 2011-04-06 14:35:10-0300 [scrapy.core.engine] INFO: Spider opened - 2011-04-06 14:35:10-0300 [scrapy.downloadermiddlewares.cookies] DEBUG: Sending cookies to: - Cookie: clientlanguage_nl=en_EN - 2011-04-06 14:35:14-0300 [scrapy.downloadermiddlewares.cookies] DEBUG: Received cookies from: <200 http://www.diningcity.com/netherlands/index.html> - Set-Cookie: JSESSIONID=B~FA4DC0C496C8762AE4F1A620EAB34F38; Path=/ - Set-Cookie: ip_isocode=US - Set-Cookie: clientlanguage_nl=en_EN; Expires=Thu, 07-Apr-2011 21:21:34 GMT; Path=/ - 2011-04-06 14:49:50-0300 [scrapy.core.engine] DEBUG: Crawled (200) (referer: None) - [...] +See :ref:`cookies`. DefaultHeadersMiddleware @@ -741,14 +700,13 @@ HttpCompressionMiddleware .. class:: HttpCompressionMiddleware - This middleware allows compressed (gzip, deflate) traffic to be + This middleware allows compressed (gzip, deflate, `brotli`_) traffic to be sent/received from web sites. - This middleware also supports decoding `brotli-compressed`_ responses with - the :ref:`brotli ` extra, and `zstd-compressed`_ - responses with the :ref:`zstd ` extra. + This middleware also supports decoding `zstd-compressed`_ responses with + the :ref:`zstd ` extra. -.. _brotli-compressed: https://www.ietf.org/rfc/rfc7932.txt +.. _brotli: https://www.ietf.org/rfc/rfc7932.txt .. _zstd-compressed: https://www.ietf.org/rfc/rfc8478.txt @@ -841,40 +799,9 @@ OffsiteMiddleware .. module:: scrapy.downloadermiddlewares.offsite :synopsis: Offsite Middleware -.. class:: OffsiteMiddleware +.. autoclass:: OffsiteMiddleware - .. versionadded:: 2.11.2 - - Filters out Requests for URLs outside the domains covered by the spider. - - This middleware filters out every request whose host names aren't in the - spider's :attr:`~scrapy.Spider.allowed_domains` attribute. - All subdomains of any domain in the list are also allowed. - E.g. the rule ``www.example.org`` will also allow ``bob.www.example.org`` - but not ``www2.example.com`` nor ``example.com``. - - When your spider returns a request for a domain not belonging to those - covered by the spider, this middleware will log a debug message similar to - this one:: - - DEBUG: Filtered offsite request to 'offsite.example': - - To avoid filling the log with too much noise, it will only print one of - these messages for each new domain filtered. So, for example, if another - request for ``offsite.example`` is filtered, no log message will be - printed. But if a request for ``other.example`` is filtered, a message - will be printed (but only for the first request filtered). - - If the spider doesn't define an - :attr:`~scrapy.Spider.allowed_domains` attribute, or the - attribute is empty, the offsite middleware will allow all requests. - - .. reqmeta:: allow_offsite - - If the request has the :attr:`~scrapy.Request.dont_filter` attribute set to - ``True`` or :attr:`Request.meta ` has ``allow_offsite`` - set to ``True``, then the OffsiteMiddleware will allow the request even if - its domain is not listed in allowed domains. + .. automethod:: should_follow RedirectMiddleware ------------------ diff --git a/docs/topics/exporters.rst b/docs/topics/exporters.rst index c43b7e20f..56b995e18 100644 --- a/docs/topics/exporters.rst +++ b/docs/topics/exporters.rst @@ -136,6 +136,70 @@ Example: return f"$ {str(value)}" return super().serialize_field(field, name, value) +.. _custom-exporters: + +Writing your own item exporter +============================== + +To write an item exporter, subclass :class:`BaseItemExporter` and implement +:meth:`~BaseItemExporter.export_item`, where +:meth:`~BaseItemExporter.get_serialized_fields` gives you the ``(name, value)`` +pairs to export. + +To make your exporter available to the :ref:`feed exports +`, list it in the :setting:`FEED_EXPORTERS` setting. Feed +exports :ref:`build ` it with the output file as the first +positional argument, and with the ``fields``, ``encoding`` and ``indent`` +:ref:`feed options ` and every key of ``item_export_kwargs`` as +keyword arguments, so your ``__init__`` method must forward unknown keyword +arguments to :class:`BaseItemExporter`. + +The file object belongs to whoever opened it, i.e. to the feed storage in the +case of feed exports, which also closes it. If you need a text file, for +example to use :func:`csv.writer` or another Python API that does not accept a +binary file, wrap it with :class:`io.TextIOWrapper` and call +:meth:`~io.TextIOBase.detach` on the wrapper in +:meth:`~BaseItemExporter.finish_exporting`; otherwise the wrapper closes the +underlying file when it is garbage-collected. + +For example, the following item exporter writes items as blocks of +``name: value`` lines: + +.. code-block:: python + + from io import TextIOWrapper + + from scrapy.exporters import BaseItemExporter + + + class TextItemExporter(BaseItemExporter): + def __init__(self, file, item_separator="\n", **kwargs): + super().__init__(**kwargs) + self.item_separator = item_separator + self.stream = TextIOWrapper( + file, encoding=self.encoding or "utf-8", write_through=True + ) + + def export_item(self, item): + for name, value in self.get_serialized_fields(item): + print(f"{name}: {value}", file=self.stream) + self.stream.write(self.item_separator) + + def finish_exporting(self): + self.stream.detach() + +To use it as the ``txt`` feed format: + +.. code-block:: python + + FEED_EXPORTERS = {"txt": "myproject.exporters.TextItemExporter"} + FEEDS = { + "items.txt": { + "format": "txt", + "item_export_kwargs": {"item_separator": "---\n"}, + }, + } + .. _topics-exporters-reference: Built-in Item Exporters reference @@ -168,6 +232,8 @@ BaseItemExporter Exports the given item. This method must be implemented in subclasses. + .. automethod:: BaseItemExporter.get_serialized_fields + .. method:: serialize_field(field, name, value) Return the serialized value for the given field. You can override this diff --git a/docs/topics/extensions.rst b/docs/topics/extensions.rst index 78b38cc3f..6d61cc342 100644 --- a/docs/topics/extensions.rst +++ b/docs/topics/extensions.rst @@ -374,8 +374,8 @@ This extension periodically logs rich stat data as a JSON object:: "elapsed": 360.008903, "log_interval": 60.0, "log_interval_real": 60.006694, - "start_time": "2023-08-03 23:24:57", - "utcnow": "2023-08-03 23:30:57" + "start_time": "2023-08-03T23:24:57.148903+00:00", + "utcnow": "2023-08-03T23:30:57.157806+00:00" } } diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 2f686fd0f..467abc989 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -104,7 +104,8 @@ storage backend types which are defined by the URI scheme. The storages backends supported out of the box are: - :ref:`topics-feed-storage-fs` -- :ref:`topics-feed-storage-ftp` +- :ref:`feed-storage-ftp` +- :ref:`feed-storage-ftps` - :ref:`topics-feed-storage-s3` (requires the :ref:`s3 ` extra) - :ref:`topics-feed-storage-gcs` (requires the :ref:`gcs ` extra) - :ref:`topics-feed-storage-stdout` @@ -168,6 +169,7 @@ you specify a path (e.g. ``/tmp/export.csv``). Alternatively you can also use a :class:`pathlib.Path` object. .. _topics-feed-storage-ftp: +.. _feed-storage-ftp: FTP --- @@ -178,6 +180,9 @@ The feeds are stored in a FTP server. - Example URI: ``ftp://user:pass@ftp.example.com/path/to/export.csv`` - Required external libraries: none +FTP sends credentials and data in cleartext. Use :ref:`feed-storage-ftps` +instead where possible. + FTP supports two different connection modes: `active or passive `_. Scrapy uses the passive connection mode by default. To use the active connection mode instead, set the @@ -192,6 +197,28 @@ storage backend is: ``True``. This storage backend uses :ref:`delayed file delivery `. +.. _feed-storage-ftps: + +FTPS +---- + +The feeds are stored in a FTP server, over a TLS connection, with the +certificate of the server verified. + +.. versionadded:: VERSION + +- URI scheme: ``ftps`` +- Example URI: ``ftps://user:pass@ftp.example.com/path/to/export.csv`` +- Required external libraries: none + +See :ref:`feed-storage-ftp` for connection modes, the ``overwrite`` default and +file delivery. + +.. note:: For SFTP, an unrelated protocol built on SSH, use + `scrapy-feedexporter-sftp + `_. + + .. _topics-feed-storage-s3: S3 @@ -502,7 +529,7 @@ as a fallback value if that key is not provided for a specific feed definition: - :ref:`topics-feed-storage-fs`: ``False`` - - :ref:`topics-feed-storage-ftp`: ``True`` + - :ref:`feed-storage-ftp` and :ref:`feed-storage-ftps`: ``True`` .. note:: Some FTP servers may not support appending to files (the ``APPE`` FTP command). @@ -624,6 +651,7 @@ Default: "s3": "scrapy.extensions.feedexport.S3FeedStorage", "gs": "scrapy.extensions.feedexport.GCSFeedStorage", "ftp": "scrapy.extensions.feedexport.FTPFeedStorage", + "ftps": "scrapy.extensions.feedexport.FTPFeedStorage", } A dict containing the built-in feed storage backends supported by Scrapy. You diff --git a/docs/topics/item-pipeline.rst b/docs/topics/item-pipeline.rst index 951c0f485..a6aac78ac 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -47,9 +47,17 @@ Additionally, they may also implement the following methods: This method is called when the spider is opened. + .. versionchanged:: VERSION + Added support for :exc:`~scrapy.exceptions.CloseSpider`. + + It may raise :exc:`~scrapy.exceptions.CloseSpider` to close the spider before + it starts crawling, e.g. if a resource that the pipeline needs is + unavailable. + .. method:: close_spider(self) - This method is called when the spider is closed. + This method is called when the spider is closed, before the + :signal:`spider_closed` signal is sent. Any of these methods may be defined as a coroutine function (``async def``). @@ -330,6 +338,36 @@ passes through ``PricePipeline`` before it reaches the :ref:`feed exports .. _books.toscrape.com: https://books.toscrape.com/ +.. _test-item-pipeline: + +Testing an item pipeline +======================== + +To send the items from a single URL through your item pipelines, use the +:command:`parse` command with the ``--pipelines`` option:: + + scrapy parse --pipelines "https://books.toscrape.com/" + +To test specific item data instead, add a callback that builds an item out of +its keyword arguments: + +.. skip: next +.. code-block:: python + + class BooksSpider(scrapy.Spider): + # ... + + def parse_item(self, response, **fields): + yield BookItem(**fields) + +and pass those keyword arguments in the command line:: + + scrapy parse --pipelines -c parse_item --cbkwargs '{"title": "Test", "price": 10}' "https://books.toscrape.com/" + +Pass any URL that your spider handles; it is downloaded even though the +callback ignores it. + + Common pitfalls =============== diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index b16066d0c..576feae7e 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -178,6 +178,37 @@ By overriding ``file_path`` like this: For more information about the ``file_path`` method, see :ref:`topics-media-pipeline-override`. +.. _file-naming-response: + +Naming files after the response +------------------------------- + +``file_path`` also receives the ``response``, which allows naming files after +response data. For example, to determine the file extension from the +``Content-Type`` header, for URLs that do not end in a file name: + +.. code-block:: python + + import mimetypes + + from scrapy.pipelines.files import FilesPipeline + + + class ContentTypeFilesPipeline(FilesPipeline): + def file_path(self, request, response=None, info=None, *, item=None): + path = super().file_path(request, response, info, item=item) + if response is None: + return path + content_type = response.headers["Content-Type"].decode() + return path + (mimetypes.guess_extension(content_type) or "") + +This requires setting :setting:`FILES_EXPIRES` to ``0``. To find out whether a +file has already been downloaded, Scrapy calls ``file_path`` before the +download, with ``response`` set to ``None``, and checks the age of the file at +the resulting path. A path that depends on the response can never match that +check, and :setting:`FILES_EXPIRES` set to ``0`` disables it, at the cost of +downloading every file on every run. + .. _topics-supported-storage: Supported Storage @@ -543,7 +574,7 @@ See here the methods that you can override in your custom Files Pipeline: return "files/" + PurePosixPath(urlparse_cached(request).path).name Similarly, you can use the ``item`` to determine the file path based on some item - property. + property, or the ``response``, see :ref:`file-naming-response`. By default the :meth:`file_path` method returns ``full/.``. @@ -693,7 +724,7 @@ See here the methods that you can override in your custom Images Pipeline: return "files/" + PurePosixPath(urlparse_cached(request).path).name Similarly, you can use the ``item`` to determine the file path based on some item - property. + property, or the ``response``, see :ref:`file-naming-response`. By default the :meth:`file_path` method returns ``full/.``. diff --git a/docs/topics/optimize.rst b/docs/topics/optimize.rst new file mode 100644 index 000000000..cea49fca3 --- /dev/null +++ b/docs/topics/optimize.rst @@ -0,0 +1,353 @@ +.. _optimize: + +============ +Optimization +============ + +A crawl goes as fast as its slowest part allows. :ref:`Find out which part that +is ` before changing any setting. + +:ref:`Broad crawls ` have their own set of recommended +adjustments. + +.. _optimize-bottleneck: + +Finding the bottleneck +====================== + +The bottleneck depends on the spider: on the same machine, one crawl can be +limited by its own parsing code and another by the target website. So measure +the crawl that you want to optimize. + +:class:`~scrapy.extensions.logstats.LogStats` reports crawl speed every +:setting:`LOGSTATS_INTERVAL` seconds: + +.. code-block:: text + + [scrapy.extensions.logstats] INFO: Crawled 1200 pages (at 60 pages/min), scraped 1150 items (at 58 items/min) + +A rate that stays flat as you raise :setting:`CONCURRENT_REQUESTS` means +something else is the limit. + + +Reading the engine status +------------------------- + +The :ref:`telnet console ` reports, through ``est()``, +what every part of the engine is doing at a given moment: + +.. code-block:: text + + len(engine.downloader.active) : 16 + len(engine._slot.scheduler.mqs) : 92 + len(engine.scraper.slot.active) : 0 + engine.scraper.slot.active_size : 0 + engine.scraper.slot.needs_backout() : False + +Take a few readings at different points of the crawl: + +- ``len(engine.downloader.active)`` stays at :setting:`CONCURRENT_REQUESTS`: + the downloader is the limit. You are waiting on the network or on the + target website. See :ref:`optimize-concurrency`. + +- ``len(engine.downloader.active)`` stays below + :setting:`CONCURRENT_REQUESTS` while the scheduler queues (``mqs``, + ``dqs``) hold requests: something throttles those requests before they + reach the downloader, usually :setting:`CONCURRENT_REQUESTS_PER_DOMAIN`, + :setting:`DOWNLOAD_DELAY` or :ref:`AutoThrottle `. + +- Both the downloader and the scheduler queues stay near empty: your spider + is not producing requests fast enough. A crawl that walks pagination one + page at a time cannot use more concurrency than it creates. See + :ref:`optimize-requests`. + +- ``needs_backout()`` is ``True``, or ``active_size`` approaches + :setting:`SCRAPER_SLOT_MAX_ACTIVE_SIZE`: responses arrive faster than your + callbacks and :ref:`item pipelines ` handle them. The + bottleneck is your own code. + +- ``len(engine._slot.scheduler.mqs)`` grows without settling: the crawl + discovers requests faster than it downloads them. This is what makes long + crawls run out of memory. + + +Reading resource usage +---------------------- + +CPU + Scrapy runs in a single process, and everything except DNS resolution and + code you explicitly move to a thread runs in a single thread. One CPU core + is the ceiling; a process sitting at 100% of a core is CPU-bound no matter + how many cores the machine has. + + Use a sampling profiler, such as py-spy_, to find out which code is + spending that CPU. :ref:`Selectors ` and item pipelines + are the usual answer. + + .. _py-spy: https://github.com/benfred/py-spy + +Memory + The :ref:`memory usage extension ` records + :stat:`memusage/startup` and :stat:`memusage/max`. A :stat:`memusage/max` + far above :stat:`memusage/startup` is expected; what matters is whether it + keeps growing for as long as the crawl runs. + + Growth that tracks ``len(engine._slot.scheduler.mqs)`` is a scheduling + problem, covered in :ref:`optimize-memory`. Growth that does not is a + :ref:`memory leak `. + +Network + Compare :stat:`downloader/response_bytes` over the crawl time against your + available bandwidth. Saturated bandwidth caps concurrency regardless of any + setting. + + DNS resolution is separate: it runs on a thread pool of + :setting:`REACTOR_THREADPOOL_MAXSIZE` threads, and results are cached + (:setting:`DNSCACHE_ENABLED`, :setting:`DNSCACHE_SIZE`). It only becomes a + limit of its own when there are many different domains to resolve, as in + :ref:`broad crawls `, where it shows up as slow starts and + DNS timeouts. + +Disk + :ref:`Feed exports ` write to disk on most crawls, + although item data is usually small enough for that not to matter. The ones + to suspect are + :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware` and + the :ref:`media pipelines `, which write whole + responses, and :setting:`JOBDIR`, which writes every scheduled request. + + +.. _optimize-concurrency: + +Sending more requests at a time +=============================== + +:setting:`CONCURRENT_REQUESTS` caps how many requests are being downloaded at +any given moment, :setting:`CONCURRENT_REQUESTS_PER_DOMAIN` caps how many of +those may target the same domain, and :setting:`DOWNLOAD_DELAY` sets a minimum +wait between two consecutive requests to the same domain. A project generated by +:command:`startproject` gets one request per second per domain out of these. + +Raise them to crawl a single website faster, and see +:ref:`broad-crawls-concurrency` to spread requests across many websites +instead. + +The limit that matters, though, is the one the target website tolerates. +Exceeding it gets you throttled, served errors or banned, all of which make the +crawl slower than a lower concurrency would have been. To find that limit: + +- Read the :ref:`robots.txt ` file of the website. Scrapy + does not act on its ``Crawl-delay`` and ``Request-rate`` directives, so when + they are present, translate them into :setting:`DOWNLOAD_DELAY` and + concurrency settings yourself. + +- Check the traffic that the website already gets, using a service like + `SimilarWeb`_ or `Cloudflare Radar`_. A rate that is a rounding error next + to what the website serves anyway is unlikely to be a problem for it. + + .. _SimilarWeb: https://www.similarweb.com/ + .. _Cloudflare Radar: https://radar.cloudflare.com/ + +- Look for a documented way in. An API, a bulk export or a search endpoint is + both faster for you and cheaper for the website than crawling its pages, and + the terms of service may state a rate. + +- Crawl when the website is idle, in its own timezone, so that the capacity + you take is capacity nobody else wanted. + +- Raise concurrency gradually and watch the website respond. + :stat:`downloader/response_status_count/{status_code}` counts for 429, 503 + or the ban page of the website, growing :stat:`retry/count`, or a + :ref:`download latency ` that climbs as you push harder, + all mean you have gone past the limit. + + +.. _optimize-requests: + +Producing requests faster +========================= + +A spider that discovers its requests one response at a time keeps the +downloader idle no matter how high you set :setting:`CONCURRENT_REQUESTS`. To +put more requests in the scheduler earlier: + +- Request every page at once when you can work out how many there are, e.g. + from a page count or from a result count and a page size in the first + response, instead of following a link to the next page on every response. + +- Get URLs from a source that lists many of them at once, such as a sitemap + or a search or export endpoint of the target website. For a crawl that + needs nothing else, :class:`~scrapy.spiders.SitemapSpider` reads sitemaps + for you. + +- Raise the :attr:`~scrapy.Request.priority` of pagination requests, so that + they are downloaded before the requests that they compete with, and + discover the rest of the crawl sooner. + +Each of these trades memory for speed: a request produced before the downloader +can take it waits in the scheduler, or on disk if you set :setting:`JOBDIR`. +Pushed far enough, they turn memory or disk into your new bottleneck, which is +why :ref:`optimize-memory` recommends the reverse of the last point. + + +.. _optimize-resources: + +Lowering resource usage +======================= + +.. _optimize-memory: + +Lowering memory usage +--------------------- + +- Lower :setting:`SCRAPER_SLOT_MAX_ACTIVE_SIZE`. + +- Lower :setting:`DOWNLOAD_MAXSIZE`, which allows a single response to take up + to 1 GiB of memory by default, multiplied by your concurrency. Set + :setting:`DOWNLOAD_WARNSIZE` first to find out whether the website actually + serves responses that big. + +- Lower the number of :ref:`scheduled requests ` held in + memory: + + - Increase the :attr:`~scrapy.Request.priority` of requests whose + :attr:`~scrapy.Request.callback` cannot yield additional requests. + + For example, the following spider uses a higher priority (1) for book + requests than for pagination requests: + + .. code-block:: python + + from scrapy import Spider + + + class BooksToScrapeComSpider(Spider): + name = "books_toscrape_com" + start_urls = [ + "http://books.toscrape.com/catalogue/category/books/mystery_3/index.html" + ] + + def parse(self, response): + next_page_links = response.css(".next a") + yield from response.follow_all(next_page_links) + book_links = response.css("article a") + yield from response.follow_all(book_links, callback=self.parse_book, priority=1) + + def parse_book(self, response): + yield { + "name": response.css("h1::text").get(), + "price": response.css(".price_color::text").re_first("£(.*)"), + "url": response.url, + } + + .. note:: If the number of request-yielding, low-priority requests + scheduled at any given time is lower than concurrency settings + (:setting:`CONCURRENT_REQUESTS_PER_DOMAIN` or + :setting:`CONCURRENT_REQUESTS`), as in the example above, this can + slow down your crawl by turning those requests into a bottleneck. + + - If you have many :ref:`start requests `, consider + :ref:`delaying their iteration `. + + - Set :setting:`JOBDIR` to offload all scheduled requests to disk. + +- Be on the lookout for :ref:`memory leaks `. + + +Lowering network usage +---------------------- + +- Install brotli_ and zstandard_ to support brotli-compressed_ and + zstd-compressed_ responses. + + .. _brotli-compressed: https://www.ietf.org/rfc/rfc7932.txt + .. _brotli: https://pypi.org/project/Brotli/ + .. _zstd-compressed: https://www.ietf.org/rfc/rfc8478.txt + .. _zstandard: https://pypi.org/project/zstandard/ + +- Enable :class:`~scrapy.downloadermiddlewares.httpcache.HttpCacheMiddleware` + while developing your spider, so that re-runs do not download the same + responses again. + + +Lowering CPU usage +------------------ + +- Set :setting:`LOG_LEVEL` to ``"INFO"`` or higher. + +- Restrict what you parse. A :ref:`selector ` over a + smaller part of the response, or a single query whose result you reuse, + beats repeated queries over the whole document. + + +Other tips +---------- + +- Try :ref:`using the asyncio reactor ` with uvloop_ as + :ref:`custom event loop `, i.e. setting + :setting:`ASYNCIO_EVENT_LOOP` to ``"uvloop.Loop"``. + + .. _uvloop: https://github.com/MagicStack/uvloop + + Alternatively, try :ref:`switching to a non-asyncio reactor + `. + +- Disable unused :ref:`components `. + + For example, set :setting:`COOKIES_ENABLED` to ``False`` unless you need + cookies. + +- Split the crawl across separate processes to use more than one CPU core. + See :ref:`distributed-crawls`. + + +.. _broad-crawls: +.. _topics-broad-crawls: + +Speeding up broad crawls +======================== + +While Scrapy is well suited for **broad crawls**, i.e. crawls that target many +websites, the default :ref:`settings ` are optimized for +crawls targeting a single website. + +For broad crawls, consider these adjustments: + +- .. _broad-crawls-concurrency: + + Increase the global concurrency: + + - Set :setting:`CONCURRENT_REQUESTS` as close to + :setting:`CONCURRENT_REQUESTS_PER_DOMAIN` × [number of target domains] + (e.g. 8 × 10 domains = 80 concurrent requests) as your CPU and memory + allow. + + - Increase :setting:`SCRAPER_SLOT_MAX_ACTIVE_SIZE` when increasing + :setting:`CONCURRENT_REQUESTS` stops making a difference. + +- .. _broad-crawls-bfo: + + If memory is a bottleneck, see if :ref:`crawling in BFO order ` lowers + memory usage. + +- Improve DNS resolution speed: + + - Set up your own DNS server, with a local cache and upstream to a `large + DNS server`_, to avoid slowing down your network. + + .. _large DNS server: https://en.wikipedia.org/wiki/Public_recursive_name_server#Notable_public_DNS_service_operators + + - Increase :setting:`REACTOR_THREADPOOL_MAXSIZE` to the minimum value + that avoids DNS resolution timeouts and makes a noticeable positive + impact in crawl speed. + +- Lower the negative impact of some responses: + + - Set :setting:`RETRY_ENABLED` to ``False`` or, if you need retries, + consider lowering :setting:`RETRY_TIMES`. + + - Lower :setting:`DOWNLOAD_TIMEOUT` to a more reasonable value, to + discard stuck requests more quickly. + + - Set :setting:`REDIRECT_ENABLED` to ``False`` unless you want to follow + redirects. diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index dfa1e21f6..971bb9106 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -458,6 +458,18 @@ finishes before starting the next one: should not have a different value per spider, and :ref:`pre-crawler settings ` cannot be defined per spider. +Every other setting applies to each crawler separately. This includes +concurrency and politeness settings, such as :setting:`CONCURRENT_REQUESTS`, +:setting:`CONCURRENT_REQUESTS_PER_DOMAIN` and :setting:`DOWNLOAD_DELAY`, and +:ref:`AutoThrottle ` also throttles each crawler +separately. When crawling simultaneously, divide those values by the number of +crawlers to keep the combined load on your hardware and on target websites +unchanged. + +Because of this, running the same spider several times in the same process +multiplies those limits instead of increasing crawling capacity. To crawl +faster, raise :setting:`CONCURRENT_REQUESTS` on a single crawler. + .. seealso:: :ref:`run-from-script`. .. skip: end @@ -518,32 +530,41 @@ modules by separating them with commas. Avoiding getting banned ======================= -Some websites implement certain measures to prevent bots from crawling them, -with varying degrees of sophistication. Getting around those measures can be -difficult and tricky, and may sometimes require special infrastructure. Please -consider contacting `commercial support`_ if in doubt. +Websites tell regular visitors and crawlers apart by how their traffic looks: +the headers it carries, how fast it arrives, how many requests come from the +same place. Traffic that stands out can be blocked even when the crawling +itself would be welcome. -Here are some tips to keep in mind when dealing with these kinds of sites: +Where the website allows crawling, the most effective thing you can do is make +yourself known: set :setting:`USER_AGENT` to a value that identifies you and +lets its owners reach you, so that they can ask you to adjust your crawler +rather than block it. -* rotate your user agent from a pool of well-known ones from browsers (Google - around to get a list of them) -* disable cookies (see :setting:`COOKIES_ENABLED`) as some sites may use - cookies to spot bot behaviour -* use download delays (2 or higher). See :setting:`DOWNLOAD_DELAY` setting. -* if possible, use `Common Crawl`_ to fetch pages, instead of hitting the sites - directly -* use a pool of rotating IPs. For example, the free `Tor project`_ or paid +Where that is not enough, the following make your traffic resemble that of a +regular visitor: + +* rotate your user agent among those of common browsers, so that your requests + do not all look alike (search the web for an up-to-date list) +* disable cookies (see :setting:`COOKIES_ENABLED`), so that a session + identifier does not tie all your requests together +* space out your requests, 2 seconds apart or more, with the + :setting:`DOWNLOAD_DELAY` setting, to keep your pace closer to that of a + person browsing +* where possible, read pages from `Common Crawl`_, which sends no traffic to + the website at all +* spread your requests over a pool of IP addresses, so that none of them + accounts for your whole crawl. For example, the free `Tor project`_ or paid services like `ProxyMesh`_. -* for HTTPS websites, if blocking appears related to TLS behavior, consider - adjusting the :setting:`DOWNLOAD_TLS_MIN_VERSION` and - :setting:`DOWNLOAD_TLS_MAX_VERSION` settings, since some websites may respond - differently depending on the TLS method used by the client. -* use a ban avoidance service, such as `Zyte API`_, which provides a `Scrapy - plugin `__ and additional +* match the TLS behavior of a browser: some websites respond differently + depending on the TLS version of the client, which you can adjust with the + :setting:`DOWNLOAD_TLS_MIN_VERSION` and :setting:`DOWNLOAD_TLS_MAX_VERSION` + settings. +* let a service take care of all of the above, such as `Zyte API`_, which + provides a `Scrapy plugin + `__ and additional features, like `AI web scraping `__ -If you are still unable to prevent your bot getting banned, consider contacting -`commercial support`_. +If your crawler still gets blocked, consider contacting `commercial support`_. .. _static-analysis: diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 75158440b..1d97e39b6 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -53,65 +53,13 @@ Request objects ``None`` is passed as value, the HTTP header will not be sent at all. .. caution:: Cookies set via the ``Cookie`` header are not considered by the - :ref:`cookies-mw`. If you need to set cookies for a request, use the - ``cookies`` argument. This is a known current limitation that is being - worked on. + :ref:`cookie middleware `. If you need to set cookies for a + request, use the ``cookies`` argument. :type headers: dict - :param cookies: the request cookies. These can be sent in two forms. - - .. invisible-code-block: python - - from scrapy import Request - - 1. Using a dict: - - .. code-block:: python - - request_with_cookies = Request( - url="http://www.example.com", - cookies={"currency": "USD", "country": "UY"}, - ) - - 2. Using a list of dicts: - - .. code-block:: python - - request_with_cookies = Request( - url="https://www.example.com", - cookies=[ - { - "name": "currency", - "value": "USD", - "domain": "example.com", - "path": "/currency", - "secure": True, - }, - ], - ) - - The latter form allows for customizing the ``domain`` and ``path`` - attributes of the cookie. This is only useful if the cookies are saved - for later requests. - - .. reqmeta:: dont_merge_cookies - - When some site returns cookies (in a response) those are stored in the - cookies for that domain and will be sent again in future requests. - That's the typical behaviour of any regular web browser. - - Note that setting the :reqmeta:`dont_merge_cookies` key to ``True`` in - :attr:`request.meta ` causes custom cookies to be - ignored. - - For more info see :ref:`cookies-mw`. - - .. caution:: Cookies set via the ``Cookie`` header are not considered by the - :ref:`cookies-mw`. If you need to set cookies for a request, use the - :class:`scrapy.Request.cookies ` parameter. This is a known - current limitation that is being worked on. - + :param cookies: the request cookies, as a dict of cookie names and values + or as a list of dicts with a cookie each. See :ref:`cookies`. :type cookies: dict or list :param encoding: the encoding of this request (defaults to ``'utf-8'``). @@ -770,6 +718,10 @@ is raise while processing it. It receives a :exc:`~twisted.python.failure.Failure` as first parameter and can be used to track connection establishment timeouts, DNS errors etc. +If an errback raises an exception, Scrapy logs it and sends the +:signal:`spider_error` signal, unless the exception is the one that the errback +received, which Scrapy logs as a download error instead. + Here's an example spider logging all errors and catching some specific errors if needed: @@ -1428,9 +1380,6 @@ TextResponse objects .. automethod:: TextResponse.json() - Returns a Python object from deserialized JSON document. - The result is cached after the first call. - .. method:: TextResponse.urljoin(url) Constructs an absolute url by combining the Response's base url with diff --git a/docs/topics/security.rst b/docs/topics/security.rst index 2ca270045..5348aae23 100644 --- a/docs/topics/security.rst +++ b/docs/topics/security.rst @@ -36,6 +36,77 @@ their input in an unsafe way, such as :func:`eval`, :func:`exec`, or :func:`pickle.loads`, and be careful when writing response data to paths derived from the response itself. +.. _security-response-size: + +Memory use when parsing responses +================================= + +Parsing a response with :ref:`selectors ` builds an in-memory +tree of the whole response body, which takes several times as much memory as +the body itself. Scrapy parses without the size limits that libxml2 applies by +default, so the size of that tree is bound only by the size of the response, as +controlled by :setting:`DOWNLOAD_MAXSIZE` (default: 1 GiB). + +XML entities are left unresolved, so the tree stays proportional to the +response body even for input crafted as an `XML bomb +`_. A server can still +make a crawler allocate a lot of memory by returning a very large response, +though, so if you know the size of the responses you care about, lower the +limit: + +.. code-block:: python + + DOWNLOAD_MAXSIZE = 32 * 1024 * 1024 # 32 MiB + +* **Pro:** a server cannot make the crawler allocate more memory than the limit + allows, whether by returning a large response or by crafting one that is + expensive to parse. + +* **Con:** you can no longer scrape sites that legitimately serve responses + above the limit, as those responses are dropped. + +.. _security-parser-limits: + +Parser limits +------------- + +The limits that libxml2 applies by default, such as 256 nesting levels and +10 MB per text node, can be restored by overriding +:attr:`~scrapy.http.TextResponse.selector` in a response subclass and swapping +responses in a :ref:`downloader middleware `: + +.. code-block:: python + + from functools import cached_property + + from scrapy import Selector + from scrapy.http import HtmlResponse + + + class LimitedHtmlResponse(HtmlResponse): + @cached_property + def selector(self): + return Selector(self, huge_tree=False) + + + class LimitedParsingMiddleware: + def process_response(self, request, response, spider): + if isinstance(response, HtmlResponse): + return response.replace(cls=LimitedHtmlResponse) + return response + +Do the same with :class:`~scrapy.http.XmlResponse` if you also parse XML. + +These limits apply per node, so :setting:`DOWNLOAD_MAXSIZE` remains your bound +on total memory: a response made of many small elements is parsed in full and +uses as much memory either way. + +* **Pro:** deeply nested responses, and responses with very large individual + nodes, become cheaper to parse. + +* **Con:** parsing stops at those limits without raising, so a legitimate page + that exceeds them yields incomplete data and no error. + TLS connections =============== diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 65ee77258..27ef3f7ef 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -69,9 +69,10 @@ Example:: precedence and override the project ones. .. note:: :ref:`Pre-crawler settings ` cannot be defined - per spider, and :ref:`reactor settings ` should not have - a different value per spider when :ref:`running multiple spiders in the - same process `. + per spider, and :ref:`reactor settings ` and + :ref:`logging settings ` are subject to restrictions when + :ref:`running multiple spiders in the same process + `. One way to do so is by setting their :attr:`~scrapy.Spider.custom_settings` attribute: @@ -329,32 +330,41 @@ Reactor settings **Reactor settings** are settings tied to the :doc:`Twisted reactor `. -These settings can be defined from a spider. However, because only 1 reactor -can be used per process, these settings cannot use a different value per spider -when :ref:`running multiple spiders in the same process -`. +Because only 1 reactor can be used per process, these settings cannot use a +different value per spider when :ref:`running multiple spiders in the same +process `. -In general, if different spiders define different values, the first defined -value is used. However, if two spiders request a different reactor, an -exception is raised. - -These settings are: +These settings are used upon installing the reactor: - :setting:`ASYNCIO_EVENT_LOOP` (not possible to set per-spider when using :class:`~scrapy.crawler.AsyncCrawlerProcess`, see below) +- :setting:`TWISTED_REACTOR` (ignored when using + :class:`~scrapy.crawler.AsyncCrawlerProcess`, see below) + +They can be :ref:`set from a spider `, but only the values +from the first spider that runs are used, since that is when the reactor is +installed. If a later spider asks for a different reactor or a different event +loop, an exception is raised. With +:class:`~scrapy.crawler.CrawlerRunner` and +:class:`~scrapy.crawler.AsyncCrawlerRunner` the reactor must be installed +beforehand, so these settings are only used to check that the installed reactor +and event loop match them. + +These settings are applied when starting the reactor: + - :setting:`TWISTED_DNS_RESOLVER` and settings used by the corresponding component, e.g. :setting:`DNSCACHE_ENABLED`, :setting:`DNSCACHE_SIZE` and :setting:`DNS_TIMEOUT` for the default one. - :setting:`REACTOR_THREADPOOL_MAXSIZE` -- :setting:`TWISTED_REACTOR` (ignored when using - :class:`~scrapy.crawler.AsyncCrawlerProcess`, see below) - -:setting:`ASYNCIO_EVENT_LOOP` and :setting:`TWISTED_REACTOR` are used upon -installing the reactor. The rest of the settings are applied when starting -the reactor. +They are read from the settings of the +:class:`~scrapy.crawler.CrawlerProcess` or +:class:`~scrapy.crawler.AsyncCrawlerProcess` object, so setting them from a +spider or an :ref:`add-on ` has no effect. They are ignored +altogether when using :class:`~scrapy.crawler.CrawlerRunner` or +:class:`~scrapy.crawler.AsyncCrawlerRunner`, which do not start the reactor. There is an additional restriction for :setting:`TWISTED_REACTOR` and :setting:`ASYNCIO_EVENT_LOOP` when using @@ -654,9 +664,13 @@ The default headers used for Scrapy HTTP Requests. They're populated in the :class:`~scrapy.downloadermiddlewares.defaultheaders.DefaultHeadersMiddleware`. .. caution:: Cookies set via the ``Cookie`` header are not considered by the - :ref:`cookies-mw`. If you need to set cookies for a request, use the - :class:`Request.cookies ` parameter. This is a known - current limitation that is being worked on. + :ref:`cookie middleware `. If you need to set cookies for a + request, use the :class:`Request.cookies ` parameter. + +.. caution:: A ``Referer`` header defined here only reaches requests for which + :class:`~scrapy.spidermiddlewares.referer.RefererMiddleware` does not set + one, such as start requests. To send it on every request, set + :setting:`REFERRER_POLICY` to ``"no-referrer"``. .. setting:: DEPTH_LIMIT @@ -749,6 +763,11 @@ Default: ``60`` Timeout for processing of DNS queries in seconds. Float is supported. +The timeout starts when the query is queued into the Twisted reactor thread +pool, not when it is sent. If that thread pool is saturated, queries can time +out before being sent, in which case increasing +:setting:`REACTOR_THREADPOOL_MAXSIZE` helps more than increasing this setting. + .. note:: This setting is only used by :class:`~scrapy.resolver.CachingThreadedResolver`. It has no effect when @@ -1374,7 +1393,7 @@ FEED_TEMPDIR Default: ``None`` The Feed Temp dir allows you to set a custom folder to save crawler -temporary files before uploading with :ref:`FTP feed storage ` and +temporary files before uploading with :ref:`FTP feed storage ` and :ref:`Amazon S3 `. .. setting:: FEED_STORAGE_GCS_ACL @@ -1903,6 +1922,7 @@ Type of in-memory queue used by the scheduler. Other available type is: .. setting:: SCHEDULER_PRIORITY_QUEUE +.. _broad-crawls-scheduler-priority-queue: SCHEDULER_PRIORITY_QUEUE ------------------------ @@ -2328,6 +2348,11 @@ also used by :class:`~scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware if :setting:`ROBOTSTXT_USER_AGENT` setting is ``None`` and there is no overriding User-Agent header specified for the request. +Set it to a value that identifies you, including a URL or an email address +where website owners can reach you, e.g. ``"MyProject +(+https://example.com/bot)"``, so that they can ask you to adjust your crawler +rather than block it. + .. setting:: WARN_ON_GENERATOR_RETURN_VALUE WARN_ON_GENERATOR_RETURN_VALUE diff --git a/docs/topics/shell.rst b/docs/topics/shell.rst index 6f7e67cf9..42c5bd169 100644 --- a/docs/topics/shell.rst +++ b/docs/topics/shell.rst @@ -144,6 +144,32 @@ Those objects are: - ``settings`` - the current :ref:`Scrapy settings ` +.. _shell-update-vars: + +Adding your own objects +----------------------- + +To define additional objects, or to run code every time a response is fetched, +write a :ref:`custom project command ` in a module called +``shell``, which overrides the :command:`shell` command, and override its +``update_vars`` method. It is called on start and after every ``fetch``, and it +receives the mapping of variable names to objects: + +.. code-block:: python + + from scrapy.commands.shell import Command as ShellCommand + + + class Command(ShellCommand): + def update_vars(self, vars): + from myproject.utils import parse_product + + vars["parse_product"] = parse_product + if vars["response"] is not None: + vars["product"] = parse_product(vars["response"]) + +``response`` is ``None`` when the shell is started without a URL. + Example of shell session ======================== diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index f7f9f5cca..0a85c3c05 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -44,6 +44,15 @@ Here is a simple example showing how you can catch signals and perform some acti def parse(self, response): pass +.. _signal-order: + +Handler order +============= + +The order in which the handlers of a signal run is undefined, and +:ref:`asynchronous handlers ` run concurrently. If two actions +must happen in a given order, run both from a single handler, in that order. + .. _signal-deferred: Asynchronous signal handlers @@ -149,6 +158,15 @@ scheduler_empty See :ref:`start-requests-lazy` for an example. + .. warning:: Only wait for this signal from + :meth:`~scrapy.Spider.start`. While no request can be sent, e.g. while + the responses being parsed exceed + :setting:`SCRAPER_SLOT_MAX_ACTIVE_SIZE`, the engine does not ask the + scheduler for requests, and hence this signal is not sent. So waiting + for it from a :ref:`callback ` can hang the crawl, + because the response being parsed is itself one of the responses that + may be blocking requests. + This signal does not support :ref:`asynchronous handlers `. @@ -272,6 +290,13 @@ spider_opened reserve per-spider resources, but can be used for any task that needs to be performed when a spider is opened. + .. versionchanged:: VERSION + Added support for :exc:`~scrapy.exceptions.CloseSpider`. + + You may raise a :exc:`~scrapy.exceptions.CloseSpider` exception to close the + spider before it starts crawling, e.g. if a resource that the spider needs + is unavailable. + This signal supports :ref:`asynchronous handlers `. :param spider: the spider which has been opened @@ -320,15 +345,22 @@ spider_error .. signal:: spider_error .. function:: spider_error(failure, response, spider) - Sent when a spider callback generates an error (i.e. raises an exception). + Sent when a spider callback or the :meth:`~scrapy.Spider.start` method of a + spider generates an error (i.e. raises an exception). + + .. versionchanged:: VERSION + Exceptions from :meth:`~scrapy.Spider.start` are also reported, see + :ref:`start-error`. This signal does not support :ref:`asynchronous handlers `. :param failure: the exception raised :type failure: twisted.python.failure.Failure - :param response: the response being processed when the exception was raised - :type response: :class:`~scrapy.http.Response` object + :param response: the response being processed when the exception was + raised, or ``None`` if the exception came from + :meth:`~scrapy.Spider.start`. + :type response: :class:`~scrapy.http.Response` | ``None`` :param spider: the spider which raised the exception :type spider: :class:`~scrapy.Spider` object diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index aa14f6801..1c9ee0c77 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -122,6 +122,9 @@ one or more of these methods: This method is an :term:`asynchronous generator` called with the results from the spider after the spider has processed the response. + *result* is lazy: a generator callback runs as *result* is iterated, so + code that runs before that iteration runs before the callback body. + .. seealso:: :ref:`universal-spider-middleware`. :param response: the response which generated this output from the @@ -142,8 +145,9 @@ one or more of these methods: .. method:: process_spider_exception(response, exception) - This method is called when a spider or :meth:`process_spider_output` - method (from a previous spider middleware) raises an exception. + This method is called when a spider callback or a + :meth:`process_spider_output` method (from a previous spider + middleware) raises an exception. :meth:`process_spider_exception` should return either ``None`` or an iterable of :class:`~scrapy.Request` or :ref:`item ` @@ -224,25 +228,7 @@ DepthMiddleware .. module:: scrapy.spidermiddlewares.depth :synopsis: Depth Spider Middleware -.. class:: DepthMiddleware - - DepthMiddleware is used for tracking the depth of each Request inside the - site being scraped. It works by setting ``request.meta['depth'] = 0`` whenever - there is no value previously set (usually just the first Request) and - incrementing it by 1 otherwise. - - It can be used to limit the maximum depth to scrape, control Request - priority based on their depth, and things like that. - - The :class:`DepthMiddleware` can be configured through the following - settings (see the settings documentation for more info): - - * :setting:`DEPTH_LIMIT` - The maximum depth that will be allowed to - crawl for any site. If zero, no limit will be imposed. - * :setting:`DEPTH_STATS_VERBOSE` - Whether to collect the number of - requests for each depth. - * :setting:`DEPTH_PRIORITY` - Whether to prioritize the requests based on - their depth. +.. autoclass:: DepthMiddleware HttpErrorMiddleware ------------------- diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 8fbf0c52d..95c80d5dc 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -59,9 +59,16 @@ scrapy.Spider :class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware` is enabled. + .. versionchanged:: VERSION + Changes to this attribute during a crawl are now taken into account. + Let's say your target url is ``https://www.example.com/1.html``, then add ``'example.com'`` to the list. + You may modify this attribute while the spider runs, e.g. to allow + domains that you only learn about from an earlier response. The change + affects requests scheduled after it. + .. autoattribute:: start_urls .. attribute:: custom_settings @@ -389,8 +396,12 @@ Start requests Delaying start request iteration -------------------------------- -You can override the :meth:`~scrapy.Spider.start` method as follows to pause -its iteration whenever there are scheduled requests: +Scrapy iterates :meth:`~scrapy.Spider.start` as fast as it yields, so all start +requests reach the scheduler early in the crawl, however many they are. To +minimize the number of requests in the scheduler at any given time, and with it +resource usage (memory, or disk when using :setting:`JOBDIR`), override +:meth:`~scrapy.Spider.start` to pause its iteration whenever there are +scheduled requests: .. code-block:: python @@ -400,9 +411,37 @@ its iteration whenever there are scheduled requests: await self.crawler.signals.wait_for(signals.scheduler_empty) yield item_or_request -This can help minimize the number of requests in the scheduler at any given -time, to minimize resource usage (memory or disk, depending on -:setting:`JOBDIR`). +.. _start-error: + +Handling start errors +--------------------- + +An exception raised by :meth:`~scrapy.Spider.start` ends its iteration, so any +remaining start items and requests are never sent. Scrapy logs the exception, +sends the :signal:`spider_error` signal, and, once the already scheduled +requests are done, closes the spider with the ``start_error`` +:stat:`finish_reason`. + +.. versionchanged:: VERSION + The close reason used to be ``finished``, and neither the + :signal:`spider_error` signal nor the :stat:`spider_exceptions/count` stat + reported the exception. + +To keep the iteration going, catch the exception yourself: + +.. code-block:: python + + async def start(self): + for url in self.start_urls: + try: + request = Request(url) + except ValueError: + self.logger.exception(f"Skipping start URL {url}") + else: + yield request + +To stop the crawl instead, and choose your own :stat:`finish_reason`, raise +:exc:`~scrapy.exceptions.CloseSpider`. .. _builtin-spiders: diff --git a/docs/topics/stats.rst b/docs/topics/stats.rst index 31053ae6e..d96bf431e 100644 --- a/docs/topics/stats.rst +++ b/docs/topics/stats.rst @@ -121,6 +121,13 @@ one per actual value of the placeholder. :meth:`~scrapy.statscollectors.StatsCollector.get_stats` output is equivalent to a counter of 0. +.. stat:: depth/request_ignored_count + +``depth/request_ignored_count`` + Number of requests dropped for exceeding :setting:`DEPTH_LIMIT`. + + Set by :class:`~scrapy.spidermiddlewares.depth.DepthMiddleware`. + .. stat:: downloader/exception_count ``downloader/exception_count`` @@ -298,6 +305,10 @@ one per actual value of the placeholder. - ``shutdown``: the crawl was interrupted, e.g. by a system signal such as ``SIGINT`` (:kbd:`Ctrl-C`). + - ``start_error``: :meth:`~scrapy.Spider.start` raised an exception, so + some :ref:`start requests ` may never have been sent, + see :ref:`start-error`. + Third-party components and your own code may use any other reason, e.g. by raising :exc:`~scrapy.exceptions.CloseSpider` with it. @@ -718,18 +729,21 @@ one per actual value of the placeholder. .. stat:: spider_exceptions/count ``spider_exceptions/count`` - Number of unhandled exceptions raised by spider callbacks. + Number of unhandled exceptions raised by spider callbacks or by + :meth:`~scrapy.Spider.start`. - Set by the :ref:`scraper `. + Set by the :ref:`engine ` and the :ref:`scraper + `. .. stat:: spider_exceptions/{exception} ``spider_exceptions/{exception}`` - Number of unhandled exceptions raised by spider callbacks, per exception, - where ``{exception}`` is the class name of the exception, e.g. + Same as :stat:`spider_exceptions/count`, per exception, where + ``{exception}`` is the class name of the exception, e.g. ``spider_exceptions/ValueError``. - Set by the :ref:`scraper `. + Set by the :ref:`engine ` and the :ref:`scraper + `. .. stat:: start_time diff --git a/pyproject.toml b/pyproject.toml index 13267e427..0bdcf6b51 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ dependencies = [ "parsel>=1.5.0", "protego>=0.1.15", "pyOpenSSL>=22.0.0", - "queuelib>=1.4.2", + "queuelib>=1.6.1", "service_identity>=23.1.0", "tldextract", "w3lib>=1.17.0", @@ -26,6 +26,8 @@ dependencies = [ # Platform-specific dependencies 'PyDispatcher>=2.0.5; platform_python_implementation == "CPython"', 'PyPyDispatcher>=2.1.0; platform_python_implementation == "PyPy"', + 'brotli>=1.2.0; implementation_name != "pypy"', + 'brotlicffi>=1.2.0.0; implementation_name == "pypy"', ] classifiers = [ "Development Status :: 5 - Production/Stable", @@ -62,10 +64,6 @@ Tracker = "https://github.com/scrapy/scrapy/issues" [project.optional-dependencies] bpython = ["bpython>=0.7.1"] -brotli = [ - "brotli>=1.2.0; implementation_name != 'pypy'", - "brotlicffi>=1.2.0.0; implementation_name == 'pypy'", -] gcs = ["google-cloud-storage>=1.29.0"] httpx = ["httpx2[http2,socks]>=2.0.0"] images = ["Pillow>=8.3.2"] @@ -125,23 +123,11 @@ module = [ "tests.test_downloaderslotssettings", "tests.test_dupefilters", "tests.test_engine_loop", - "tests.test_exporters", "tests.test_extension_statsmailer", "tests.test_extension_throttle", - "tests.test_feedexport", - "tests.test_feedexport_postprocess", - "tests.test_feedexport_storages", - "tests.test_feedexport_uri_params", - "tests.test_item", "tests.test_linkextractors", - "tests.test_loader", "tests.test_logformatter", "tests.test_mail", - "tests.test_pipeline_crawl", - "tests.test_pipeline_files", - "tests.test_pipeline_images", - "tests.test_pipeline_media", - "tests.test_pipelines", "tests.test_pqueues", "tests.test_scheduler_base", "tests.test_settings", @@ -333,6 +319,9 @@ markers = [ ] filterwarnings = [ "ignore::DeprecationWarning:twisted.web.static", + # Jobs that do not report coverage disable it with --no-cov, which pytest-cov + # warns about because the coverage options below stay in place. + "ignore::pytest_cov.CovDisabledWarning", # Twisted doesn't close failed sockets after CannotListenError: https://github.com/twisted/twisted/issues/6108 "ignore:Exception ignored in. list[Any]: items, requests, opts, depth, spider, callback = args if opts.pipelines: - assert self.pcrawler.engine itemproc = self.pcrawler.engine.scraper.itemproc if hasattr(itemproc, "process_item_async"): for item in items: diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 19138ffd0..52be1aadf 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -27,7 +27,6 @@ if TYPE_CHECKING: class Command(ScrapyCommand): default_settings: ClassVar[dict[str, Any]] = { "DUPEFILTER_CLASS": "scrapy.dupefilters.BaseDupeFilter", - "KEEP_ALIVE": True, "LOGSTATS_INTERVAL": 0, } diff --git a/scrapy/core/downloader/handlers/__init__.py b/scrapy/core/downloader/handlers/__init__.py index fb27cdb8b..84dc6216b 100644 --- a/scrapy/core/downloader/handlers/__init__.py +++ b/scrapy/core/downloader/handlers/__init__.py @@ -39,11 +39,23 @@ logger = logging.getLogger(__name__) class DownloadHandlerProtocol(Protocol): + """Interface that :ref:`download handlers ` must + implement. + + Besides implementing this protocol, the contract of a download handler + includes **never** calling :meth:`crawler.engine.download_async() + `. + """ + lazy: bool + """Whether to delay instantiation of the handler; see :ref:`lazy + `.""" - async def download_request(self, request: Request) -> Response: ... + async def download_request(self, request: Request) -> Response: + """Download *request* and return a response.""" - async def close(self) -> None: ... + async def close(self) -> None: + """Clean up any resources used by the handler.""" class DownloadHandlers: diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 17dca5acb..c288ae792 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -17,12 +17,19 @@ from twisted.internet.defer import Deferred, succeed from twisted.internet.endpoints import TCP4ClientEndpoint from twisted.internet.protocol import Factory, Protocol, connectionDone from twisted.python.failure import Failure +from twisted.web._newclient import ( + HEADER, + STATUS, + HTTP11ClientProtocol, + HTTPClientParser, +) from twisted.web.client import ( URI, Agent, HTTPConnectionPool, ResponseDone, ResponseFailed, + _HTTP11ClientFactory, ) from twisted.web.client import Response as TxResponse from twisted.web.http import PotentialDataLoss, _DataLoss @@ -60,7 +67,8 @@ from ._base_http import BaseHttpDownloadHandler if TYPE_CHECKING: from twisted.internet.base import ReactorBase - from twisted.internet.interfaces import IConsumer + from twisted.internet.interfaces import IAddress, IConsumer + from twisted.web._newclient import Request as TxRequest # typing.NotRequired requires Python 3.11 from typing_extensions import NotRequired @@ -95,7 +103,7 @@ class HTTP11DownloadHandler(BaseHttpDownloadHandler): self._pool.maxPersistentPerHost = crawler.settings.getint( "CONCURRENT_REQUESTS_PER_DOMAIN" ) - self._pool._factory.noisy = False + self._pool._factory = _LenientHTTP11ClientFactory self._contextFactory: IPolicyForHTTPS = _load_context_factory_from_settings( crawler @@ -548,7 +556,8 @@ class _ScrapyAgent: txresponse._transport._producer.abortConnection() raise DownloadCancelledError(warning_msg) - if warnsize and expected_size > warnsize: + reached_warnsize = bool(warnsize and expected_size > warnsize) + if reached_warnsize: logger.warning( get_warnsize_msg(expected_size, warnsize, request, expected=True) ) @@ -561,6 +570,7 @@ class _ScrapyAgent: request=request, maxsize=maxsize, warnsize=warnsize, + reached_warnsize=reached_warnsize, fail_on_dataloss=fail_on_dataloss, crawler=self._crawler, tls_verbose_logging=self._tls_verbose_logging, @@ -625,6 +635,7 @@ class _ResponseReader(Protocol): fail_on_dataloss: bool, crawler: Crawler, *, + reached_warnsize: bool = False, tls_verbose_logging: bool = False, ): self._finished: Deferred[_ResultT] = finished @@ -634,7 +645,7 @@ class _ResponseReader(Protocol): self._maxsize: int = maxsize self._warnsize: int = warnsize self._fail_on_dataloss: bool = fail_on_dataloss - self._reached_warnsize: bool = False + self._reached_warnsize: bool = reached_warnsize self._bytes_received: int = 0 self._certificate: ssl.Certificate | None = None self._ip_address: ipaddress.IPv4Address | ipaddress.IPv6Address | None = None @@ -737,3 +748,77 @@ class _ResponseReader(Protocol): reason = Failure(exc) self._finished.errback(reason) + + +class _LenientHTTPClientParser(HTTPClientParser): + """Response parser that skips bad response header lines, those with no + colon in them, instead of failing to parse the whole response. + + Some servers send such lines, and web browsers skip them and keep parsing + the header lines that follow. See + https://github.com/scrapy/scrapy/issues/210. + """ + + def lineReceived(self, line: bytes) -> None: + # A copy of twisted.web._newclient.HTTPParser.lineReceived() where the + # header name and value are only extracted from header lines that have + # a colon. + + # Handle the normal CR LF case. + if line[-1:] == b"\r": + line = line[:-1] + + if self.state == STATUS: + self.statusReceived(line) # type: ignore[no-untyped-call] + self.state = HEADER + return + + # HEADER is the only other state in which lines are received, as the + # parser switches to raw mode for the response body. + if not line or line[0] not in b" \t": + if self._partialHeader is not None: + header = b"".join(self._partialHeader) + if b":" in header: + name, value = header.split(b":", 1) + self.headerReceived(name, value.strip()) # type: ignore[no-untyped-call] + else: + logger.debug( + f"Skipping the bad response header line {header!r}, as " + f"it has no colon." + ) + if not line: + # Empty line means the header section is over. + self.allHeadersReceived() # type: ignore[no-untyped-call] + else: + # Line not beginning with LWS is another header. + self._partialHeader = [line] + else: + # A line beginning with LWS is a continuation of a header begun on + # a previous line. + self._partialHeader.append(line) # type: ignore[union-attr] + + +class _LenientHTTP11ClientProtocol(HTTP11ClientProtocol): + """Protocol that parses responses with :class:`_LenientHTTPClientParser`.""" + + def request(self, request: TxRequest) -> Deferred[IResponse]: + d: Deferred[IResponse] = super().request(request) + # HTTP11ClientProtocol.request() hardcodes the parser class, so the + # only way to use a different one is to replace the class of the parser + # object that it creates. This is safe because + # _LenientHTTPClientParser defines no additional state. The parser is + # always there because HTTPConnectionPool only reuses connections whose + # protocol is in the QUIESCENT state, for which request() always + # creates a parser. + assert self._parser is not None + self._parser.__class__ = _LenientHTTPClientParser + return d + + +class _LenientHTTP11ClientFactory(_HTTP11ClientFactory): + """Factory that builds :class:`_LenientHTTP11ClientProtocol` protocols.""" + + noisy = False + + def buildProtocol(self, addr: IAddress | None) -> HTTP11ClientProtocol: + return _LenientHTTP11ClientProtocol(self._quiescentCallback) # type: ignore[no-untyped-call] diff --git a/scrapy/core/downloader/handlers/http2.py b/scrapy/core/downloader/handlers/http2.py index f60c58d1b..9b3d4fbd4 100644 --- a/scrapy/core/downloader/handlers/http2.py +++ b/scrapy/core/downloader/handlers/http2.py @@ -40,7 +40,7 @@ class H2DownloadHandler(BaseDownloadHandler): from twisted.internet import reactor - self._pool = H2ConnectionPool(reactor, crawler.settings) + self._pool = H2ConnectionPool(reactor, crawler) self._context_factory = _load_context_factory_from_settings(crawler) self._bind_address = crawler.settings.get("DOWNLOAD_BIND_ADDRESS") diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 53903536d..318a36a22 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -121,7 +121,6 @@ class ExecutionEngine: self.crawler: Crawler = crawler self.settings: Settings = crawler.settings self.signals: SignalManager = crawler.signals - assert crawler.logformatter self.logformatter: LogFormatter = crawler.logformatter self._slot: _Slot | None = None self.spider: Spider | None = None @@ -134,6 +133,9 @@ class ExecutionEngine: ] = spider_closed_callback self.start_time: float | None = None self._start: AsyncIterator[Any] | None = None + # Whether Spider.start() raised, i.e. some start items or requests may + # never have reached the engine. + self._start_error: bool = False self._closewait: Deferred[None] | None = None self._start_request_processing_awaitable: ( asyncio.Future[None] | Deferred[None] | None @@ -255,7 +257,7 @@ class ExecutionEngine: ) return deferred_from_coro(self.close_async()) - async def close_async(self) -> None: + async def close_async(self, *, reason: str = "shutdown") -> None: """ Gracefully close the execution engine. If it has already been started, stop it. In all cases, close the spider and the downloader. @@ -263,9 +265,7 @@ class ExecutionEngine: if self.running: await self.stop_async() # will also close spider and downloader elif self.spider is not None: - await self.close_spider_async( - reason="shutdown" - ) # will also close downloader + await self.close_spider_async(reason=reason) # will also close downloader elif hasattr(self, "downloader"): self.downloader.close() @@ -286,13 +286,29 @@ class ExecutionEngine: item_or_request = await anext(self._start) except StopAsyncIteration: self._start = None + except CloseSpider as exception: + self._start = None + _schedule_coro( + self.close_spider_async(reason=exception.reason or "cancelled") + ) except Exception as exception: self._start = None + self._start_error = True exception_traceback = format_exc() logger.error( f"Error while reading start items and requests: {exception}.\n{exception_traceback}", exc_info=True, ) + self.signals.send_catch_log( + signal=signals.spider_error, + failure=Failure(), + response=None, + spider=self.spider, + ) + self.crawler.stats.inc_value("spider_exceptions/count") + self.crawler.stats.inc_value( + f"spider_exceptions/{type(exception).__name__}" + ) else: if not self.spider: return # spider already closed @@ -548,24 +564,39 @@ class ExecutionEngine: nextcall = CallLaterOnce(self._start_scheduled_requests) scheduler = build_from_crawler(self.scheduler_cls, self.crawler) self._slot = _Slot(close_if_idle, nextcall, scheduler) - self._start = await self.scraper.spidermw.process_start() - if hasattr(scheduler, "open") and (d := scheduler.open(self.crawler.spider)): - await maybe_deferred_to_future(d) - await self.scraper.open_spider_async() - assert self.crawler.stats - if argument_is_required(self.crawler.stats.open_spider, "spider"): + # A component that fails to start can ask for the spider to be closed. + # The rest of the startup runs anyway, so that components that are + # started also get stopped, and the request is honored once the spider + # is open. + close_spider_exc: CloseSpider | None = None + try: + self._start = await self.scraper.spidermw.process_start() + if hasattr(scheduler, "open") and ( + d := scheduler.open(self.crawler.spider) + ): + await maybe_deferred_to_future(d) + await self.scraper.open_spider_async() + except CloseSpider as exc: + close_spider_exc = exc + stats = self.crawler.stats + if argument_is_required(stats.open_spider, "spider"): warnings.warn( - f"The open_spider() method of {global_object_name(type(self.crawler.stats))} requires a spider argument," + f"The open_spider() method of {global_object_name(type(stats))} requires a spider argument," f" this is deprecated and the argument will not be passed in future Scrapy versions.", ScrapyDeprecationWarning, stacklevel=2, ) - self.crawler.stats.open_spider(spider=self.crawler.spider) + stats.open_spider(spider=self.crawler.spider) else: - self.crawler.stats.open_spider() - await self.signals.send_catch_log_async( - signals.spider_opened, spider=self.crawler.spider + stats.open_spider() + results = await self.signals.send_catch_log_async( + signals.spider_opened, spider=self.crawler.spider, dont_log=CloseSpider ) + for _, result in results: + if isinstance(result, CloseSpider): + close_spider_exc = close_spider_exc or result + if close_spider_exc is not None: + raise close_spider_exc def _spider_idle(self) -> None: """ @@ -588,7 +619,8 @@ class ExecutionEngine: if DontCloseSpider in detected_ex: return if self.spider_is_idle(): - ex = detected_ex.get(CloseSpider, CloseSpider(reason="finished")) + default_reason = "start_error" if self._start_error else "finished" + ex = detected_ex.get(CloseSpider, CloseSpider(reason=default_reason)) assert isinstance(ex, CloseSpider) # typing _schedule_coro(self.close_spider_async(reason=ex.reason)) @@ -668,20 +700,18 @@ class ExecutionEngine: extra={"spider": spider}, ) - assert self.crawler.stats try: - if argument_is_required(self.crawler.stats.close_spider, "spider"): + stats = self.crawler.stats + if argument_is_required(stats.close_spider, "spider"): warnings.warn( - f"The close_spider() method of {global_object_name(type(self.crawler.stats))} requires a spider argument," + f"The close_spider() method of {global_object_name(type(stats))} requires a spider argument," f" this is deprecated and the argument will not be passed in future Scrapy versions.", ScrapyDeprecationWarning, stacklevel=2, ) - self.crawler.stats.close_spider( - spider=self.crawler.spider, reason=reason - ) + stats.close_spider(spider=self.crawler.spider, reason=reason) else: - self.crawler.stats.close_spider(reason=reason) + stats.close_spider(reason=reason) except Exception: logger.error("Stats close failure") diff --git a/scrapy/core/http2/agent.py b/scrapy/core/http2/agent.py index aa55e29a0..042557208 100644 --- a/scrapy/core/http2/agent.py +++ b/scrapy/core/http2/agent.py @@ -21,8 +21,8 @@ if TYPE_CHECKING: from twisted.internet.base import ReactorBase from twisted.internet.endpoints import HostnameEndpoint + from scrapy.crawler import Crawler from scrapy.http import Request, Response - from scrapy.settings import Settings from scrapy.spiders import Spider @@ -30,9 +30,9 @@ ConnectionKeyT = tuple[bytes, bytes, int] class H2ConnectionPool: - def __init__(self, reactor: ReactorBase, settings: Settings) -> None: + def __init__(self, reactor: ReactorBase, crawler: Crawler) -> None: self._reactor = reactor - self.settings = settings + self._crawler = crawler # Store a dictionary which is used to get the respective # H2ClientProtocolInstance using the key as Tuple(scheme, hostname, port) @@ -43,7 +43,7 @@ class H2ConnectionPool: ConnectionKeyT, deque[Deferred[H2ClientProtocol]] ] = {} - self._tls_verbose_logging: bool = settings.getbool( + self._tls_verbose_logging: bool = crawler.settings.getbool( "DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING" ) @@ -77,7 +77,7 @@ class H2ConnectionPool: factory = H2ClientFactory( uri, - self.settings, + self._crawler, conn_lost_deferred, tls_verbose_logging=self._tls_verbose_logging, ) diff --git a/scrapy/core/http2/protocol.py b/scrapy/core/http2/protocol.py index 7136e829e..2d59aba31 100644 --- a/scrapy/core/http2/protocol.py +++ b/scrapy/core/http2/protocol.py @@ -44,7 +44,7 @@ if TYPE_CHECKING: from twisted.python.failure import Failure from twisted.web.client import URI - from scrapy.settings import Settings + from scrapy.crawler import Crawler from scrapy.spiders import Spider @@ -90,7 +90,7 @@ class H2ClientProtocol(Protocol, TimeoutMixin): def __init__( self, uri: URI, - settings: Settings, + crawler: Crawler, conn_lost_deferred: Deferred[list[BaseException]], *, tls_verbose_logging: bool = False, @@ -100,11 +100,12 @@ class H2ClientProtocol(Protocol, TimeoutMixin): uri -- URI of the base url to which HTTP/2 Connection will be made. uri is used to verify that incoming client requests have correct base URL. - settings -- Scrapy project settings + crawler -- The crawler the requests belong to conn_lost_deferred -- Deferred that fires with the list of underlying exceptions to notify that connection was lost tls_verbose_logging -- Whether to log TLS details """ + self._crawler: Crawler = crawler self._conn_lost_deferred: Deferred[list[BaseException]] = conn_lost_deferred self._tls_verbose_logging: bool = tls_verbose_logging @@ -140,8 +141,8 @@ class H2ClientProtocol(Protocol, TimeoutMixin): # Both ip_address and uri are used by the Stream before # initiating the request to verify that the base address # Variables taken from Project Settings - "default_download_maxsize": settings.getint("DOWNLOAD_MAXSIZE"), - "default_download_warnsize": settings.getint("DOWNLOAD_WARNSIZE"), + "default_download_maxsize": crawler.settings.getint("DOWNLOAD_MAXSIZE"), + "default_download_warnsize": crawler.settings.getint("DOWNLOAD_WARNSIZE"), # Counter to keep track of opened streams. This counter # is used to make sure that not more than MAX_CONCURRENT_STREAMS # streams are opened which leads to ProtocolError @@ -208,6 +209,7 @@ class H2ClientProtocol(Protocol, TimeoutMixin): stream_id=next(self._stream_id_generator), request=request, protocol=self, + crawler=self._crawler, download_maxsize=getattr( spider, "download_maxsize", self.metadata["default_download_maxsize"] ), @@ -461,20 +463,20 @@ class H2ClientFactory(Factory): def __init__( self, uri: URI, - settings: Settings, + crawler: Crawler, conn_lost_deferred: Deferred[list[BaseException]], *, tls_verbose_logging: bool = False, ) -> None: self.uri = uri - self.settings = settings + self.crawler = crawler self.conn_lost_deferred = conn_lost_deferred self.tls_verbose_logging = tls_verbose_logging def buildProtocol(self, addr: IAddress) -> H2ClientProtocol: return H2ClientProtocol( self.uri, - self.settings, + self.crawler, self.conn_lost_deferred, tls_verbose_logging=self.tls_verbose_logging, ) diff --git a/scrapy/core/http2/stream.py b/scrapy/core/http2/stream.py index c6226bbca..4fc300d90 100644 --- a/scrapy/core/http2/stream.py +++ b/scrapy/core/http2/stream.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from contextlib import suppress from enum import Enum from io import BytesIO from typing import TYPE_CHECKING, Any @@ -12,9 +13,11 @@ from twisted.internet.error import ConnectionClosed from twisted.python.failure import Failure from twisted.web.client import ResponseFailed -from scrapy.exceptions import DownloadCancelledError +from scrapy import signals +from scrapy.exceptions import DownloadCancelledError, StopDownload from scrapy.http.headers import Headers from scrapy.utils._download_handlers import ( + check_stop_download, get_maxsize_msg, get_warnsize_msg, make_response, @@ -25,6 +28,7 @@ if TYPE_CHECKING: from collections.abc import Sequence from scrapy.core.http2.protocol import H2ClientProtocol + from scrapy.crawler import Crawler from scrapy.http import Request, Response @@ -82,6 +86,9 @@ class StreamCloseReason(Enum): # Actual response body size is more than allowed limit MAXSIZE_EXCEEDED_ACTUAL = 8 + # A signal handler raised StopDownload + STOP_DOWNLOAD = 9 + class Stream: """Represents a single HTTP/2 Stream. @@ -99,6 +106,7 @@ class Stream: stream_id: int, request: Request, protocol: H2ClientProtocol, + crawler: Crawler, download_maxsize: int = 0, download_warnsize: int = 0, ) -> None: @@ -107,10 +115,13 @@ class Stream: stream_id -- Unique identifier for the stream within a single HTTP/2 connection request -- The HTTP request associated to the stream protocol -- Parent H2ClientProtocol instance + crawler -- The crawler the request belongs to """ self.stream_id: int = stream_id self._request: Request = request self._protocol: H2ClientProtocol = protocol + self._crawler: Crawler = crawler + self._stop_download: StopDownload | None = None self._download_maxsize = self._request.meta.get( "download_maxsize", download_maxsize @@ -338,6 +349,13 @@ class Stream: self._response["body"].write(data) self._response["flow_controlled_size"] += flow_controlled_length + if stop_download := check_stop_download( + signals.bytes_received, self._crawler, self._request, data=data + ): + self._stop_download = stop_download + self.reset_stream(StreamCloseReason.STOP_DOWNLOAD) + return + # We check maxsize here in case the Content-Length header was not received if ( self._download_maxsize @@ -369,8 +387,20 @@ class Stream: else: self._response["headers"].appendlist(name, value) - # Check if we exceed the allowed max data size which can be received expected_size = int(self._response["headers"].get(b"Content-Length", -1)) + + if stop_download := check_stop_download( + signals.headers_received, + self._crawler, + self._request, + headers=self._response["headers"], + body_length=expected_size if expected_size >= 0 else None, + ): + self._stop_download = stop_download + self.reset_stream(StreamCloseReason.STOP_DOWNLOAD) + return + + # Check if we exceed the allowed max data size which can be received if self._download_maxsize and expected_size > self._download_maxsize: self.reset_stream(StreamCloseReason.MAXSIZE_EXCEEDED) return @@ -387,11 +417,18 @@ class Stream: if self.metadata["stream_closed_local"]: raise StreamClosedError(self.stream_id) - # Clear buffer earlier to avoid keeping data in memory for a long time - self._response["body"].truncate(0) + # The data received so far is the body of the response built for a + # stopped download, otherwise the buffer is cleared early to avoid + # keeping data in memory for a long time + if reason is not StreamCloseReason.STOP_DOWNLOAD: + self._response["body"].truncate(0) self.metadata["stream_closed_local"] = True - self._protocol.conn.reset_stream(self.stream_id, ErrorCodes.REFUSED_STREAM) + # The remote peer may have ended the stream already, e.g. because the + # whole response arrived within the data that triggered this reset, in + # which case there is nothing left to reset + with suppress(StreamClosedError): + self._protocol.conn.reset_stream(self.stream_id, ErrorCodes.REFUSED_STREAM) self.close(reason) def close( @@ -444,7 +481,7 @@ class Stream: logger.error(error_msg) self._deferred_response.errback(DownloadCancelledError(error_msg)) - elif reason is StreamCloseReason.ENDED: + elif reason in {StreamCloseReason.ENDED, StreamCloseReason.STOP_DOWNLOAD}: self._fire_response_deferred() # Stream was abruptly ended here @@ -495,13 +532,18 @@ class Stream: and fires the response deferred callback with the generated response instance""" - response = make_response( - url=self._request.url, - status=self._response["status"], - headers=self._response["headers"], - body=self._response["body"].getvalue(), - certificate=self._protocol.metadata["certificate"], - ip_address=self._protocol.metadata["ip_address"], - protocol="h2", - ) - self._deferred_response.callback(response) + try: + response = make_response( + url=self._request.url, + status=self._response["status"], + headers=self._response["headers"], + body=self._response["body"].getvalue(), + certificate=self._protocol.metadata["certificate"], + ip_address=self._protocol.metadata["ip_address"], + protocol="h2", + stop_download=self._stop_download, + ) + except StopDownload as exc: + self._deferred_response.errback(exc) + else: + self._deferred_response.callback(response) diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 58e37ce5e..6426b1751 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -120,7 +120,6 @@ class Scraper: self.concurrent_items: int = crawler.settings.getint("CONCURRENT_ITEMS") self.crawler: Crawler = crawler self.signals: SignalManager = crawler.signals - assert crawler.logformatter self.logformatter: LogFormatter = crawler.logformatter def _check_deprecated_itemproc_method(self, method: str) -> None: @@ -355,7 +354,6 @@ class Scraper: assert self.crawler.spider exc = _failure.value if isinstance(exc, CloseSpider): - assert self.crawler.engine is not None # typing _schedule_coro( self.crawler.engine.close_spider_async(reason=exc.reason or "cancelled") ) @@ -374,11 +372,9 @@ class Scraper: response=response, spider=self.crawler.spider, ) - assert self.crawler.stats - self.crawler.stats.inc_value("spider_exceptions/count") - self.crawler.stats.inc_value( - f"spider_exceptions/{_failure.value.__class__.__name__}" - ) + stats = self.crawler.stats + stats.inc_value("spider_exceptions/count") + stats.inc_value(f"spider_exceptions/{_failure.value.__class__.__name__}") def handle_spider_output( self, @@ -456,7 +452,6 @@ class Scraper: Items are sent to the item pipelines, requests are scheduled. """ if isinstance(output, Request): - assert self.crawler.engine is not None # typing self.crawler.engine.crawl(request=output) return if output is not None: diff --git a/scrapy/crawler.py b/scrapy/crawler.py index e6996ed53..5267b0c87 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -8,14 +8,14 @@ import signal import warnings from abc import ABC, abstractmethod from functools import partial -from typing import TYPE_CHECKING, Any, TypeVar +from typing import TYPE_CHECKING, Any, Generic, TypeVar, overload from twisted.internet.defer import Deferred, DeferredList, inlineCallbacks from scrapy import Spider from scrapy.addons import AddonManager from scrapy.core.engine import ExecutionEngine -from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.exceptions import CloseSpider, ScrapyDeprecationWarning from scrapy.extension import ExtensionManager from scrapy.settings import SETTINGS_PRIORITIES, Settings, overridden_settings from scrapy.signalmanager import SignalManager @@ -58,7 +58,56 @@ logger = logging.getLogger(__name__) _T = TypeVar("_T") +class _LateAttribute(Generic[_T]): + """Descriptor for a :class:`Crawler` attribute that only gets a value once + the crawl starts. + + The value is kept in an attribute of the same name prefixed with an + underscore, and reading it before it is set raises :exc:`RuntimeError`. + This way the public attribute can be annotated as always set, and its + users, both in Scrapy and in third-party code, do not need to narrow its + type on every use. Code that runs before the crawl starts reads the + underscore-prefixed attribute instead. + """ + + def __set_name__(self, owner: type[Crawler], name: str) -> None: + self._name = name + self._private_name = f"_{name}" + + @overload + def __get__(self, instance: None, owner: type[Crawler]) -> _LateAttribute[_T]: ... + + @overload + def __get__(self, instance: Crawler, owner: type[Crawler]) -> _T: ... + + def __get__( + self, instance: Crawler | None, owner: type[Crawler] + ) -> _LateAttribute[_T] | _T: + if instance is None: + return self + value: _T | None = getattr(instance, self._private_name) + if value is None: + raise RuntimeError( + f"Crawler.{self._name} is not set yet. It is set when the " + "crawl starts, so it can only be used from then on, e.g. " + "from the spider_opened signal handler onwards." + ) + return value + + def __set__(self, instance: Crawler, value: _T) -> None: + setattr(instance, self._private_name, value) + + class Crawler: + #: Running instance of :class:`~scrapy.core.engine.ExecutionEngine`. + engine: _LateAttribute[ExecutionEngine] = _LateAttribute() + extensions: _LateAttribute[ExtensionManager] = _LateAttribute() + logformatter: _LateAttribute[LogFormatter] = _LateAttribute() + request_fingerprinter: _LateAttribute[RequestFingerprinterProtocol] = ( + _LateAttribute() + ) + stats: _LateAttribute[StatsCollector] = _LateAttribute() + def __init__( self, spidercls: type[Spider], @@ -83,14 +132,13 @@ class Crawler: self.crawling: bool = False self._started: bool = False - self.extensions: ExtensionManager | None = None - self.stats: StatsCollector | None = None - self.logformatter: LogFormatter | None = None - self.request_fingerprinter: RequestFingerprinterProtocol | None = None self.spider: Spider | None = None - #: Running instance of :class:`~scrapy.core.engine.ExecutionEngine`. - self.engine: ExecutionEngine | None = None + self._engine: ExecutionEngine | None = None + self._extensions: ExtensionManager | None = None + self._logformatter: LogFormatter | None = None + self._request_fingerprinter: RequestFingerprinterProtocol | None = None + self._stats: StatsCollector | None = None def _update_root_log_handler(self) -> None: if get_scrapy_root_handler() is not None: @@ -223,12 +271,16 @@ class Crawler: self._apply_settings() self._update_root_log_handler() self.engine = self._create_engine() - yield deferred_from_coro(self.engine.open_spider_async()) - yield deferred_from_coro(self.engine.start_async()) + try: + yield deferred_from_coro(self.engine.open_spider_async()) + except CloseSpider as exc: + yield deferred_from_coro(self.engine.close_async(reason=exc.reason)) + else: + yield deferred_from_coro(self.engine.start_async()) except Exception: self.crawling = False - if self.engine is not None: - yield deferred_from_coro(self.engine.close_async()) + if self._engine is not None: + yield deferred_from_coro(self._engine.close_async()) raise async def crawl_async(self, *args: Any, **kwargs: Any) -> None: @@ -253,12 +305,16 @@ class Crawler: self._apply_settings() self._update_root_log_handler() self.engine = self._create_engine() - await self.engine.open_spider_async() - await self.engine.start_async() + try: + await self.engine.open_spider_async() + except CloseSpider as exc: + await self.engine.close_async(reason=exc.reason) + else: + await self.engine.start_async() except Exception: self.crawling = False - if self.engine is not None: - await self.engine.close_async() + if self._engine is not None: + await self._engine.close_async() raise def _create_spider(self, *args: Any, **kwargs: Any) -> Spider: @@ -284,7 +340,6 @@ class Crawler: """ if self.crawling: self.crawling = False - assert self.engine if self.engine.running: await self.engine.stop_async() @@ -315,7 +370,7 @@ class Crawler: This method can only be called after the crawl engine has been created, e.g. at signals :signal:`engine_started` or :signal:`spider_opened`. """ - if not self.engine: + if self._engine is None: raise RuntimeError( "Crawler.get_downloader_middleware() can only be called after " "the crawl engine has been created." @@ -333,7 +388,7 @@ class Crawler: created, e.g. at signals :signal:`engine_started` or :signal:`spider_opened`. """ - if not self.extensions: + if self._extensions is None: raise RuntimeError( "Crawler.get_extension() can only be called after the " "extension manager has been created." @@ -350,7 +405,7 @@ class Crawler: This method can only be called after the crawl engine has been created, e.g. at signals :signal:`engine_started` or :signal:`spider_opened`. """ - if not self.engine: + if self._engine is None: raise RuntimeError( "Crawler.get_item_pipeline() can only be called after the " "crawl engine has been created." @@ -367,7 +422,7 @@ class Crawler: This method can only be called after the crawl engine has been created, e.g. at signals :signal:`engine_started` or :signal:`spider_opened`. """ - if not self.engine: + if self._engine is None: raise RuntimeError( "Crawler.get_spider_middleware() can only be called after the " "crawl engine has been created." diff --git a/scrapy/downloadermiddlewares/httpcache.py b/scrapy/downloadermiddlewares/httpcache.py index e7ca0ac0e..3ebd98027 100644 --- a/scrapy/downloadermiddlewares/httpcache.py +++ b/scrapy/downloadermiddlewares/httpcache.py @@ -55,7 +55,6 @@ class HttpCacheMiddleware: @classmethod def from_crawler(cls, crawler: Crawler) -> Self: - assert crawler.stats o = cls(crawler.settings, crawler.stats) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index b99c323c5..0045ddcaa 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -30,27 +30,7 @@ if TYPE_CHECKING: logger = getLogger(__name__) -ACCEPTED_ENCODINGS: list[bytes] = [b"gzip", b"deflate"] - -try: - try: - import brotli - except ImportError: - import brotlicffi as brotli -except ImportError: - pass -else: - try: - brotli.Decompressor.can_accept_more_data # noqa: B018 - except AttributeError: # pragma: no cover - warnings.warn( - "You have brotli installed. But 'br' encoding support now requires " - "brotli's or brotlicffi's version >= 1.2.0. Please upgrade " - "brotli/brotlicffi to make Scrapy decode 'br' encoded responses.", - stacklevel=2, - ) - else: - ACCEPTED_ENCODINGS.append(b"br") +ACCEPTED_ENCODINGS: list[bytes] = [b"gzip", b"deflate", b"br"] if find_spec("zstandard") is not None: ACCEPTED_ENCODINGS.append(b"zstd") @@ -205,8 +185,6 @@ class HttpCompressionMiddleware: f"{self.__class__.__name__} cannot decode the response for {response.url} " f"from unsupported encoding(s) '{encodings_str}'." ) - if b"br" in encodings: - msg += " You need to install brotli or brotlicffi >= 1.2.0 to decode 'br'." if b"zstd" in encodings: msg += " You need to install zstandard to decode 'zstd'." logger.warning(msg) diff --git a/scrapy/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py index db85b62a1..b9a26df3a 100644 --- a/scrapy/downloadermiddlewares/offsite.py +++ b/scrapy/downloadermiddlewares/offsite.py @@ -21,15 +21,46 @@ logger = logging.getLogger(__name__) class OffsiteMiddleware: + """Filter out requests for URLs outside the domains covered by the spider. + + .. versionadded:: 2.11.2 + + A request is allowed if its host name is in the + :attr:`~scrapy.Spider.allowed_domains` attribute of the spider, or is a + subdomain of one of those domains. E.g. ``www.example.org`` also allows + ``bob.www.example.org``, but neither ``www2.example.org`` nor + ``example.org``. See :meth:`should_follow` to use a different policy. + + If the spider does not define :attr:`~scrapy.Spider.allowed_domains`, or + the attribute is empty, every request is allowed. + + Filtered requests are logged as follows:: + + DEBUG: Filtered offsite request to 'offsite.example': + + Only the first request filtered for a given domain is logged, to keep the + log readable. + + .. reqmeta:: allow_offsite + + allow_offsite + ------------- + + Requests with the ``allow_offsite`` :attr:`~scrapy.Request.meta` key set to + ``True``, or with :attr:`~scrapy.Request.dont_filter` set to ``True``, are + allowed regardless of their host name. + """ + crawler: Crawler + host_regex: re.Pattern[str] def __init__(self, stats: StatsCollector): self.stats = stats self.domains_seen: set[str] = set() + self._allowed_domains: list[str] | None = None @classmethod def from_crawler(cls, crawler: Crawler) -> Self: - assert crawler.stats o = cls(crawler.stats) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.request_scheduled, signal=signals.request_scheduled) @@ -37,7 +68,13 @@ class OffsiteMiddleware: return o def spider_opened(self, spider: Spider) -> None: - self.host_regex: re.Pattern[str] = self.get_host_regex(spider) + self._update_host_regex(spider) + + def _update_host_regex(self, spider: Spider) -> None: + allowed_domains = list(getattr(spider, "allowed_domains", None) or []) + if allowed_domains != self._allowed_domains: + self._allowed_domains = allowed_domains + self.host_regex = self.get_host_regex(spider) def request_scheduled(self, request: Request, spider: Spider) -> None: self.process_request(request) @@ -64,13 +101,30 @@ class OffsiteMiddleware: raise IgnoreRequest(f"Filtered offsite request to {domain!r}") def should_follow(self, request: Request, spider: Spider) -> bool: + """Return ``True`` if *request* is on site, ``False`` if it must be + filtered out. + + Override this method to implement a different offsite policy. For + example, to allow the domains in + :attr:`~scrapy.Spider.allowed_domains` but none of their subdomains: + + .. code-block:: python + + from scrapy.downloadermiddlewares.offsite import OffsiteMiddleware + from scrapy.utils.httpobj import urlparse_cached + + + class RootOnlyOffsiteMiddleware(OffsiteMiddleware): + def should_follow(self, request, spider): + return urlparse_cached(request).hostname in spider.allowed_domains + """ + self._update_host_regex(spider) regex = self.host_regex # hostname can be None for wrong urls (like javascript links) host = urlparse_cached(request).hostname or "" return bool(regex.search(host)) def get_host_regex(self, spider: Spider) -> re.Pattern[str]: - """Override this method to implement a different offsite policy""" allowed_domains = getattr(spider, "allowed_domains", None) if not allowed_domains: return re.compile("") # allow all by default diff --git a/scrapy/downloadermiddlewares/retry.py b/scrapy/downloadermiddlewares/retry.py index f910d07c8..dd2897ee4 100644 --- a/scrapy/downloadermiddlewares/retry.py +++ b/scrapy/downloadermiddlewares/retry.py @@ -94,7 +94,6 @@ def get_retry_request( retry-related job stats """ settings = spider.crawler.settings - assert spider.crawler.stats stats = spider.crawler.stats retry_times = request.meta.get("retry_times", 0) + 1 if max_retry_times is None: diff --git a/scrapy/downloadermiddlewares/robotstxt.py b/scrapy/downloadermiddlewares/robotstxt.py index 9b1e1b71f..5d540ea6d 100644 --- a/scrapy/downloadermiddlewares/robotstxt.py +++ b/scrapy/downloadermiddlewares/robotstxt.py @@ -27,6 +27,7 @@ if TYPE_CHECKING: from scrapy import Spider from scrapy.crawler import Crawler from scrapy.robotstxt import RobotParser + from scrapy.statscollectors import StatsCollector logger = logging.getLogger(__name__) @@ -73,6 +74,7 @@ class RobotsTxtMiddleware: self._default_useragent: str = crawler.settings["USER_AGENT"] self._robotstxt_useragent: str | None = crawler.settings["ROBOTSTXT_USER_AGENT"] self.crawler: Crawler = crawler + self._stats: StatsCollector = crawler.stats self._parsers: dict[str, RobotParser | Deferred[RobotParser | None] | None] = {} self._parserimpl: RobotParser = load_object( crawler.settings.get("ROBOTSTXT_PARSER") @@ -129,8 +131,7 @@ class RobotsTxtMiddleware: {"request": request}, extra={"spider": self.crawler.spider}, ) - assert self.crawler.stats - self.crawler.stats.inc_value("robotstxt/forbidden") + self._stats.inc_value("robotstxt/forbidden") if request.meta.get("is_start_request"): self._start_request_denied = True raise IgnoreRequest("Forbidden by robots.txt") @@ -148,8 +149,6 @@ class RobotsTxtMiddleware: meta={"dont_obey_robotstxt": True}, callback=NO_CALLBACK, ) - assert self.crawler.engine - assert self.crawler.stats try: resp = await self.crawler.engine.download_async(robotsreq) await self._parse_robots(resp, netloc, request) @@ -162,7 +161,7 @@ class RobotsTxtMiddleware: extra={"spider": self.crawler.spider}, ) self._robots_error(e, netloc) - self.crawler.stats.inc_value("robotstxt/request_count") + self._stats.inc_value("robotstxt/request_count") parser = self._parsers[netloc] if isinstance(parser, Deferred): @@ -172,11 +171,8 @@ class RobotsTxtMiddleware: async def _parse_robots( self, response: Response, netloc: str, request: Request ) -> None: - assert self.crawler.stats - self.crawler.stats.inc_value("robotstxt/response_count") - self.crawler.stats.inc_value( - f"robotstxt/response_status_count/{response.status}" - ) + self._stats.inc_value("robotstxt/response_count") + self._stats.inc_value(f"robotstxt/response_status_count/{response.status}") rp = self._parserimpl.from_crawler(self.crawler, response.body) await self.crawler.signals.send_catch_log_async( signal=signals.robots_parsed, @@ -191,8 +187,7 @@ class RobotsTxtMiddleware: def _robots_error(self, exc: Exception, netloc: str) -> None: if not isinstance(exc, IgnoreRequest): key = f"robotstxt/exception_count/{type(exc)}" - assert self.crawler.stats - self.crawler.stats.inc_value(key) + self._stats.inc_value(key) rp_dfd = self._parsers[netloc] assert isinstance(rp_dfd, Deferred) self._parsers[netloc] = None diff --git a/scrapy/downloadermiddlewares/stats.py b/scrapy/downloadermiddlewares/stats.py index bafa931de..07de1c2e7 100644 --- a/scrapy/downloadermiddlewares/stats.py +++ b/scrapy/downloadermiddlewares/stats.py @@ -43,7 +43,6 @@ class DownloaderStats: def from_crawler(cls, crawler: Crawler) -> Self: if not crawler.settings.getbool("DOWNLOADER_STATS"): raise NotConfigured - assert crawler.stats return cls(crawler.stats) @_warn_spider_arg diff --git a/scrapy/dupefilters.py b/scrapy/dupefilters.py index 36fb0f97d..09f0be63b 100644 --- a/scrapy/dupefilters.py +++ b/scrapy/dupefilters.py @@ -95,7 +95,6 @@ class RFPDupeFilter(BaseDupeFilter): @classmethod def from_crawler(cls, crawler: Crawler) -> Self: - assert crawler.request_fingerprinter debug = crawler.settings.getbool("DUPEFILTER_DEBUG") return cls( job_dir(crawler.settings), @@ -134,5 +133,4 @@ class RFPDupeFilter(BaseDupeFilter): self.logger.debug(msg, {"request": request}, extra={"spider": spider}) self.logdupes = False - assert spider.crawler.stats spider.crawler.stats.inc_value("dupefilter/filtered") diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index ccaccddf2..cd2560df1 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -56,8 +56,11 @@ class DontCloseSpider(Exception): class CloseSpider(Exception): - """Raised from a :ref:`spider callback ` to request the - spider to be closed/stopped. + """Raised from a :ref:`spider callback `, or while the + spider is starting, to request the spider to be closed/stopped. + + .. versionchanged:: VERSION + Added support for raising it while the spider is starting. *reason* is a string with the reason for closing. diff --git a/scrapy/exporters.py b/scrapy/exporters.py index ea600d1a8..7f8aaf059 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -85,11 +85,16 @@ class BaseItemExporter(ABC): declared = (name for name in adapter.field_names() if name in populated) return dict.fromkeys([*declared, *adapter.keys()]) - def _get_serialized_fields( + def get_serialized_fields( self, item: Any, default_value: Any = None, include_empty: bool | None = None ) -> Iterable[tuple[str, Any]]: - """Return the fields to export as an iterable of tuples - (name, serialized_value) + """Return the fields of *item* to export, as an iterable of + ``(name, serialized_value)`` tuples, taking :attr:`fields_to_export` + into account and applying :meth:`serialize_field` to every value. + + Fields missing from *item* are exported with *default_value*. + + *include_empty* overrides :attr:`export_empty_fields`. """ item = ItemAdapter(item) @@ -136,7 +141,7 @@ class JsonLinesItemExporter(BaseItemExporter): self.encoder: JSONEncoder = ScrapyJSONEncoder(**self._kwargs) def export_item(self, item: Any) -> None: - itemdict = dict(self._get_serialized_fields(item)) + itemdict = dict(self.get_serialized_fields(item)) data = self.encoder.encode(itemdict) + "\n" self.file.write(to_bytes(data, self.encoding)) @@ -176,7 +181,7 @@ class JsonItemExporter(BaseItemExporter): self.file.write(b"]") def export_item(self, item: Any) -> None: - itemdict = dict(self._get_serialized_fields(item)) + itemdict = dict(self.get_serialized_fields(item)) data = to_bytes(self.encoder.encode(itemdict), self.encoding) self._add_comma_after_first() self.file.write(data) @@ -216,7 +221,7 @@ class XmlItemExporter(BaseItemExporter): self._beautify_indent(depth=1) self.xg.startElement(self.item_element, AttributesImpl({})) self._beautify_newline() - for name, value in self._get_serialized_fields(item, default_value=""): + for name, value in self.get_serialized_fields(item, default_value=""): self._export_xml_field(name, value, depth=2) self._beautify_indent(depth=1) self.xg.endElement(self.item_element) @@ -310,7 +315,7 @@ class CsvItemExporter(BaseItemExporter): f"See: https://docs.scrapy.org/en/latest/topics/feed-exports.html#feed-export-fields", ) self._data_loss_warned = True - fields = self._get_serialized_fields(item, default_value="", include_empty=True) + fields = self.get_serialized_fields(item, default_value="", include_empty=True) values = list(self._build_row(x for _, x in fields)) self.csv_writer.writerow(values) @@ -347,7 +352,7 @@ class PickleItemExporter(BaseItemExporter): self.protocol: int = protocol def export_item(self, item: Any) -> None: - d = dict(self._get_serialized_fields(item)) + d = dict(self.get_serialized_fields(item)) pickle.dump(d, self.file, self.protocol) @@ -365,7 +370,7 @@ class MarshalItemExporter(BaseItemExporter): self.file: BytesIO = file def export_item(self, item: Any) -> None: - marshal.dump(dict(self._get_serialized_fields(item)), self.file) + marshal.dump(dict(self.get_serialized_fields(item)), self.file) class PprintItemExporter(BaseItemExporter): @@ -374,7 +379,7 @@ class PprintItemExporter(BaseItemExporter): self.file: BytesIO = file def export_item(self, item: Any) -> None: - itemdict = dict(self._get_serialized_fields(item)) + itemdict = dict(self.get_serialized_fields(item)) self.file.write(to_bytes(pprint.pformat(itemdict) + "\n")) @@ -417,5 +422,5 @@ class PythonItemExporter(BaseItemExporter): yield key, self._serialize_value(value) def export_item(self, item: Any) -> dict[str | bytes, Any]: # type: ignore[override] - result: dict[str | bytes, Any] = dict(self._get_serialized_fields(item)) + result: dict[str | bytes, Any] = dict(self.get_serialized_fields(item)) return result diff --git a/scrapy/extensions/closespider.py b/scrapy/extensions/closespider.py index 9cb792e30..5d40e5f9f 100644 --- a/scrapy/extensions/closespider.py +++ b/scrapy/extensions/closespider.py @@ -102,7 +102,6 @@ class CloseSpider: self._close_spider("closespider_pagecount_no_item") def spider_opened(self, spider: Spider) -> None: - assert self.crawler.engine self.task = call_later( self.close_on["timeout"], self._close_spider, "closespider_timeout" ) @@ -146,5 +145,4 @@ class CloseSpider: self._close_spider("closespider_timeout_no_item") def _close_spider(self, reason: str) -> None: - assert self.crawler.engine _schedule_coro(self.crawler.engine.close_spider_async(reason=reason)) diff --git a/scrapy/extensions/corestats.py b/scrapy/extensions/corestats.py index 6a5e55992..b464942af 100644 --- a/scrapy/extensions/corestats.py +++ b/scrapy/extensions/corestats.py @@ -26,7 +26,6 @@ class CoreStats: @classmethod def from_crawler(cls, crawler: Crawler) -> Self: - assert crawler.stats o = cls(crawler.stats) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) diff --git a/scrapy/extensions/debug.py b/scrapy/extensions/debug.py index 5def7509e..8802a3b58 100644 --- a/scrapy/extensions/debug.py +++ b/scrapy/extensions/debug.py @@ -45,7 +45,6 @@ class StackTraceDump: return cls(crawler) def dump_stacktrace(self, signum: int, frame: FrameType | None) -> None: - assert self.crawler.engine log_args = { "stackdumps": self._thread_stacks(), "enginestatus": format_engine_status(self.crawler.engine), diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 118462bc9..874023e25 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -149,7 +149,7 @@ class BlockingFeedStorage(ABC): return NamedTemporaryFile(prefix="feed-", dir=path) - def store(self, file: IO[bytes]) -> Deferred[None] | None: + def store(self, file: IO[bytes]) -> Deferred[None]: return deferred_from_coro(run_in_thread(self._store_in_thread, file)) @abstractmethod @@ -340,7 +340,7 @@ class GCSFeedStorage(BlockingFeedStorage): from google.cloud.storage import Client # noqa: PLC0415 client = Client(project=self.project_id) - bucket = client.get_bucket(self.bucket_name) + bucket = client.bucket(self.bucket_name) blob = bucket.blob(self.blob_name) blob.upload_from_file(file, predefined_acl=self.acl) finally: @@ -363,6 +363,7 @@ class FTPFeedStorage(BlockingFeedStorage): self.username: str = u.username or "" self.password: str = unquote(u.password or "") self.path: str = u.path + self.tls: bool = u.scheme == "ftps" self.use_active_mode: bool = use_active_mode self.overwrite: bool = not feed_options or feed_options.get("overwrite", True) @@ -390,6 +391,7 @@ class FTPFeedStorage(BlockingFeedStorage): password=self.password, use_active_mode=self.use_active_mode, overwrite=self.overwrite, + tls=self.tls, ) @@ -611,7 +613,6 @@ class FeedExporter: logmsg = f"{slot.format} feed ({slot.itemcount} items) in: {slot.uri}" slot_type = type(slot.storage).__name__ - assert self.crawler.stats try: await ensure_awaitable(slot.storage.store(self._get_file(slot))) except Exception: diff --git a/scrapy/extensions/httpcache.py b/scrapy/extensions/httpcache.py index dbb79b02d..d008d0f67 100644 --- a/scrapy/extensions/httpcache.py +++ b/scrapy/extensions/httpcache.py @@ -261,7 +261,6 @@ class DbmCacheStorage: extra={"spider": spider}, ) - assert spider.crawler.request_fingerprinter self._fingerprinter: RequestFingerprinterProtocol = ( spider.crawler.request_fingerprinter ) @@ -326,7 +325,6 @@ class FilesystemCacheStorage: extra={"spider": spider}, ) - assert spider.crawler.request_fingerprinter self._fingerprinter = spider.crawler.request_fingerprinter def close_spider(self, spider: Spider) -> None: diff --git a/scrapy/extensions/logstats.py b/scrapy/extensions/logstats.py index 6c94d947e..b818569ba 100644 --- a/scrapy/extensions/logstats.py +++ b/scrapy/extensions/logstats.py @@ -37,7 +37,6 @@ class LogStats: interval: float = crawler.settings.getfloat("LOGSTATS_INTERVAL") if not interval: raise NotConfigured - assert crawler.stats o = cls(crawler.stats, interval) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) diff --git a/scrapy/extensions/memdebug.py b/scrapy/extensions/memdebug.py index 1fde6b296..35ff90e3b 100644 --- a/scrapy/extensions/memdebug.py +++ b/scrapy/extensions/memdebug.py @@ -29,7 +29,6 @@ class MemoryDebugger: def from_crawler(cls, crawler: Crawler) -> Self: if not crawler.settings.getbool("MEMDEBUG_ENABLED"): raise NotConfigured - assert crawler.stats o = cls(crawler.stats) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) return o diff --git a/scrapy/extensions/memusage.py b/scrapy/extensions/memusage.py index e0e289ce8..ec761cfbf 100644 --- a/scrapy/extensions/memusage.py +++ b/scrapy/extensions/memusage.py @@ -27,6 +27,7 @@ if TYPE_CHECKING: from typing_extensions import Self from scrapy.crawler import Crawler + from scrapy.statscollectors import StatsCollector logger = logging.getLogger(__name__) @@ -43,6 +44,7 @@ class MemoryUsage: raise NotConfigured from exc self.crawler: Crawler = crawler + self._stats: StatsCollector = crawler.stats self.warned: bool = False self.notify_mails: list[str] = crawler.settings.getlist("MEMUSAGE_NOTIFY_MAIL") if self.notify_mails: # pragma: no cover @@ -77,8 +79,7 @@ class MemoryUsage: return size def engine_started(self) -> None: - assert self.crawler.stats - self.crawler.stats.set_value("memusage/startup", self.get_virtual_size()) + self._stats.set_value("memusage/startup", self.get_virtual_size()) self.tasks: list[AsyncioLoopingCall | LoopingCall] = [] tsk = create_looping_call(self.update) self.tasks.append(tsk) @@ -98,15 +99,12 @@ class MemoryUsage: tsk.stop() def update(self) -> None: - assert self.crawler.stats - self.crawler.stats.max_value("memusage/max", self.get_virtual_size()) + self._stats.max_value("memusage/max", self.get_virtual_size()) def _check_limit(self) -> None: - assert self.crawler.engine - assert self.crawler.stats peak_mem_usage = self.get_virtual_size() if peak_mem_usage > self.limit: - self.crawler.stats.set_value("memusage/limit_reached", 1) + self._stats.set_value("memusage/limit_reached", 1) mem = self.limit / 1024 / 1024 logger.error( "Memory usage exceeded %(memusage)dMiB. Shutting down Scrapy...", @@ -119,7 +117,7 @@ class MemoryUsage: f"memory usage exceeded {mem}MiB at {socket.gethostname()}" ) self._send_report(self.notify_mails, subj) - self.crawler.stats.set_value("memusage/limit_notified", 1) + self._stats.set_value("memusage/limit_notified", 1) if self.crawler.engine.spider is not None: _schedule_coro( @@ -136,9 +134,8 @@ class MemoryUsage: def _check_warning(self) -> None: if self.warned: # warn only once return - assert self.crawler.stats if self.get_virtual_size() > self.warning: - self.crawler.stats.set_value("memusage/warning_reached", 1) + self._stats.set_value("memusage/warning_reached", 1) self.crawler.signals.send_catch_log(signal=signals.memusage_warning_reached) mem = self.warning / 1024 / 1024 logger.warning( @@ -152,16 +149,13 @@ class MemoryUsage: f"memory usage reached {mem}MiB at {socket.gethostname()}" ) self._send_report(self.notify_mails, subj) - self.crawler.stats.set_value("memusage/warning_notified", 1) + self._stats.set_value("memusage/warning_notified", 1) self.warned = True def _send_report(self, rcpts: list[str], subject: str) -> None: # pragma: no cover """send notification mail with some additional useful info""" - assert self.crawler.engine - assert self.crawler.stats - stats = self.crawler.stats - s = f"Memory usage at engine startup : {stats.get_value('memusage/startup') / 1024 / 1024}M\r\n" - s += f"Maximum memory usage : {stats.get_value('memusage/max') / 1024 / 1024}M\r\n" + s = f"Memory usage at engine startup : {self._stats.get_value('memusage/startup') / 1024 / 1024}M\r\n" + s += f"Maximum memory usage : {self._stats.get_value('memusage/max') / 1024 / 1024}M\r\n" s += f"Current memory usage : {self.get_virtual_size() / 1024 / 1024}M\r\n" s += ( diff --git a/scrapy/extensions/periodic_log.py b/scrapy/extensions/periodic_log.py index cbcc8b70e..adffbcbc4 100644 --- a/scrapy/extensions/periodic_log.py +++ b/scrapy/extensions/periodic_log.py @@ -87,7 +87,6 @@ class PeriodicLog: ) if not (ext_stats or ext_delta or ext_timing_enabled): raise NotConfigured - assert crawler.stats assert ext_stats is not None assert ext_delta is not None o = cls( diff --git a/scrapy/extensions/statsmailer.py b/scrapy/extensions/statsmailer.py index f05595806..7647cf33d 100644 --- a/scrapy/extensions/statsmailer.py +++ b/scrapy/extensions/statsmailer.py @@ -42,7 +42,6 @@ class StatsMailer: if not recipients: raise NotConfigured mail: MailSender = MailSender.from_crawler(crawler) - assert crawler.stats o = cls(crawler.stats, recipients, mail) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) return o diff --git a/scrapy/extensions/telnet.py b/scrapy/extensions/telnet.py index 3be24c53f..392f79299 100644 --- a/scrapy/extensions/telnet.py +++ b/scrapy/extensions/telnet.py @@ -52,6 +52,7 @@ class TelnetConsole(protocol.ServerFactory): self.crawler: Crawler = crawler self.noisy: bool = False + self.port: Port | None = None self.portrange: list[int] = [ int(x) for x in crawler.settings.getlist("TELNETCONSOLE_PORT") ] @@ -71,7 +72,7 @@ class TelnetConsole(protocol.ServerFactory): return cls(crawler) def start_listening(self) -> None: - self.port: Port = listen_tcp(self.portrange, self.host, self) + self.port = listen_tcp(self.portrange, self.host, self) h = self.port.getHost() logger.info( "Telnet console listening on %(host)s:%(port)d", @@ -80,7 +81,10 @@ class TelnetConsole(protocol.ServerFactory): ) def stop_listening(self) -> None: - self.port.stopListening() + # The port is unset if start_listening() failed, e.g. because every + # port in TELNETCONSOLE_PORT was taken. + if self.port is not None: + self.port.stopListening() def protocol(self) -> telnet.TelnetTransport: class Portal: @@ -104,7 +108,6 @@ class TelnetConsole(protocol.ServerFactory): def _get_telnet_vars(self) -> dict[str, Any]: # Note: if you add entries here also update topics/telnetconsole.rst - assert self.crawler.engine telnet_vars: dict[str, Any] = { "engine": self.crawler.engine, "spider": self.crawler.engine.spider, diff --git a/scrapy/extensions/throttle.py b/scrapy/extensions/throttle.py index cde73f12e..f44aee03d 100644 --- a/scrapy/extensions/throttle.py +++ b/scrapy/extensions/throttle.py @@ -45,7 +45,6 @@ class AutoThrottle: def _spider_opened(self, spider: Spider) -> None: self.mindelay = self._min_delay() self.maxdelay = self._max_delay() - assert self.crawler.engine self.crawler.engine.downloader._delay = self._start_delay() def _min_delay(self) -> float: @@ -98,7 +97,6 @@ class AutoThrottle: key: str | None = request.meta.get("download_slot") if key is None: return None, None - assert self.crawler.engine return key, self.crawler.engine.downloader.slots.get(key) def _adjust_delay(self, slot: Slot, latency: float, response: Response) -> None: diff --git a/scrapy/http/cookies.py b/scrapy/http/cookies.py index 8edeae01c..555d930e6 100644 --- a/scrapy/http/cookies.py +++ b/scrapy/http/cookies.py @@ -54,9 +54,9 @@ class CookieJar: if not IPV4_RE.search(req_host): hosts = potential_domain_matches(req_host) if "." not in req_host: - hosts.append(req_host + ".local") + hosts += potential_domain_matches(req_host + ".local") else: - hosts = [req_host] + hosts = [req_host, "." + req_host] cookies = [] for host in hosts: diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index f1db11488..f91cb2095 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -305,6 +305,11 @@ class Response(object_ref): :class:`~.TextResponse` provides a :meth:`~.TextResponse.follow_all` method which supports selectors in addition to absolute/relative URLs and Link objects. + + .. caution:: Every returned request gets its own *meta* and + *cb_kwargs* dictionaries, but the values within them are shared. + Mutating one of those values, e.g. appending to a list, affects + all the returned requests. """ if not hasattr(urls, "__iter__"): raise TypeError("'urls' argument must be an iterable") diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index d01e23e47..a8e452f7e 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -84,9 +84,21 @@ class TextResponse(Response): ) def json(self) -> Any: - """Deserialize a JSON document to a Python object.""" + """Deserialize a JSON document to a Python object. + + .. versionchanged:: VERSION + Bodies that cannot be decoded as UTF-8, UTF-16 or UTF-32, as the + JSON specification requires, are now decoded using + :attr:`TextResponse.encoding` instead of raising + :exc:`UnicodeDecodeError`. + + The result is cached after the first call. + """ if self._cached_decoded_json is _NONE: - self._cached_decoded_json = json.loads(self.body) + try: + self._cached_decoded_json = json.loads(self.body) + except UnicodeDecodeError: + self._cached_decoded_json = json.loads(self.text) return self._cached_decoded_json @property @@ -264,6 +276,9 @@ class TextResponse(Response): using the ``css`` or ``xpath`` parameters, this method will not produce requests for selectors from which links cannot be obtained (for instance, anchor tags without an ``href`` attribute) + + .. seealso:: :meth:`.Response.follow_all`, for a caution about mutable + *meta* and *cb_kwargs* values. """ arguments = [x for x in (urls, css, xpath) if x is not None] if len(arguments) != 1: diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 3fb741d7a..75f9753e7 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -282,6 +282,12 @@ class LxmlLinkExtractor: if m: return m.group(1) + ``process_value`` is called before the filtering parameters, such as + ``allow`` and ``deny``, which match the value that it returns. To drop + links based on their final URL, use the ``process_links`` parameter of + :class:`~scrapy.spiders.Rule`, which only receives links that those + parameters kept. + :type process_value: collections.abc.Callable :param strip: whether to strip whitespaces from extracted attributes. @@ -326,7 +332,7 @@ class LxmlLinkExtractor: unique=unique, process=process_value, strip=strip, - canonicalized=not canonicalize, + canonicalized=True, ) self.allow_res: list[re.Pattern[str]] = self._compile_regexes(allow) self.deny_res: list[re.Pattern[str]] = self._compile_regexes(deny) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 55a3676e5..e666f4ddd 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -697,9 +697,9 @@ class FilesPipeline(MediaPipeline): } def inc_stats(self, status: str) -> None: - assert self.crawler.stats - self.crawler.stats.inc_value("file_count") - self.crawler.stats.inc_value(f"file_status_count/{status}") + stats = self.crawler.stats + stats.inc_value("file_count") + stats.inc_value(f"file_status_count/{status}") async def _file_downloaded( self, diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 79b6c4f27..5e7a4b409 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -29,7 +29,7 @@ from scrapy.utils.defer import ensure_awaitable from scrapy.utils.python import to_bytes if TYPE_CHECKING: - from collections.abc import Iterable + from collections.abc import Iterator from os import PathLike from PIL import Image @@ -180,7 +180,7 @@ class ImagesPipeline(FilesPipeline): info: MediaPipeline.SpiderInfo, *, item: Any = None, - ) -> Iterable[tuple[str, Image.Image, BytesIO]]: + ) -> Iterator[tuple[str, Image.Image, BytesIO]]: path = self.file_path(request, response=response, info=info, item=item) orig_image = self._Image.open(BytesIO(response.body)) transposed_image = self._ImageOps.exif_transpose(orig_image) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 764f82a78..d4025bf3b 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -100,7 +100,6 @@ class MediaPipeline(ABC): stacklevel=2, ) self.crawler: Crawler = crawler - assert crawler.request_fingerprinter self._fingerprinter: RequestFingerprinterProtocol = ( crawler.request_fingerprinter ) @@ -228,7 +227,6 @@ class MediaPipeline(ABC): ) -> FileInfo: try: self._modify_media_request(request) - assert self.crawler.engine response = await self.crawler.engine.download_async(request) return await ensure_awaitable( self.media_downloaded(response, request, info, item=item) diff --git a/scrapy/pqueues.py b/scrapy/pqueues.py index 41411ceaf..f4c35c0fe 100644 --- a/scrapy/pqueues.py +++ b/scrapy/pqueues.py @@ -2,6 +2,8 @@ from __future__ import annotations import hashlib import logging +from contextlib import suppress +from pathlib import Path from typing import TYPE_CHECKING, Protocol, cast from scrapy.utils.misc import build_from_crawler @@ -263,7 +265,6 @@ class ScrapyPriorityQueue: class DownloaderInterface: def __init__(self, crawler: Crawler): - assert crawler.engine self.downloader: Downloader = crawler.engine.downloader def stats(self, possible_slots: Iterable[str]) -> list[tuple[int, str]]: @@ -409,6 +410,11 @@ class DownloaderAwarePriorityQueue: request = queue.pop() if len(queue) == 0: del self.pqueues[slot] + if self.key: + # Reclaim the slot directory; rmdir leaves it alone if the + # downstream queues did not remove all their files. + with suppress(OSError): + Path(self.key, _path_safe(slot)).rmdir() return request def push(self, request: Request) -> None: diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index f6334c32c..fa91e2904 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -46,8 +46,10 @@ class Selector(_ParselSelector, object_ref): ``"json"``, ``"text"`` or ``None`` (default). It's passed to :class:`parsel.Selector` and its meaning is defined there. However, when ``type`` is ``None``, it is set to ``"xml"`` for an - :class:`~scrapy.http.XmlResponse` and to ``"html"`` otherwise before - passing it to :class:`parsel.Selector`. + :class:`~scrapy.http.XmlResponse` and to ``"html"`` for an + :class:`~scrapy.http.HtmlResponse` or for ``text`` before passing it to + :class:`parsel.Selector`, which for any other response is left to + determine the type from the response body. .. note:: JSON selector support requires ``parsel`` 1.8.0 or higher. With older versions setting ``type`` to ``"json"`` or ``"text"`` is not @@ -70,8 +72,13 @@ class Selector(_ParselSelector, object_ref): f"{self.__class__.__name__}.__init__() received both response and text" ) + # A response that is neither HTML nor XML, e.g. a JSON one, keeps type + # unset, so that parsel determines it from the body. if type is None: - type = "xml" if isinstance(response, XmlResponse) else "html" # noqa: A001 + if isinstance(response, XmlResponse): + type = "xml" # noqa: A001 + elif response is None or isinstance(response, HtmlResponse): + type = "html" # noqa: A001 if text is not None: response = _response_from_text(text, type) diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index a44b36c8a..d7b30b849 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -378,6 +378,7 @@ FEED_STORAGES_BASE = { "": "scrapy.extensions.feedexport.FileFeedStorage", "file": "scrapy.extensions.feedexport.FileFeedStorage", "ftp": "scrapy.extensions.feedexport.FTPFeedStorage", + "ftps": "scrapy.extensions.feedexport.FTPFeedStorage", "gs": "scrapy.extensions.feedexport.GCSFeedStorage", "s3": "scrapy.extensions.feedexport.S3FeedStorage", "stdout": "scrapy.extensions.feedexport.StdoutFeedStorage", diff --git a/scrapy/shell.py b/scrapy/shell.py index dfea00c46..8f78af571 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -193,7 +193,6 @@ class Shell: """ if not self.spider: await self._open_spider(spider) - assert self.crawler.engine is not None # send the request to the engine self.crawler.engine.crawl(request) # this will fire when the request callback runs (via the callback hijacking in _request_deferred()) @@ -204,7 +203,6 @@ class Shell: spider = self.crawler.spider or self.crawler._create_spider() self.crawler.spider = spider - assert self.crawler.engine await self.crawler.engine.open_spider_async(close_if_idle=False) self.spider = spider diff --git a/scrapy/spidermiddlewares/depth.py b/scrapy/spidermiddlewares/depth.py index 054804119..49683168e 100644 --- a/scrapy/spidermiddlewares/depth.py +++ b/scrapy/spidermiddlewares/depth.py @@ -28,6 +28,27 @@ logger = logging.getLogger(__name__) class DepthMiddleware(BaseSpiderMiddleware): + """Track the depth of each request within the site being scraped, setting + ``request.meta["depth"]`` to 0 when there is no value previously set + (usually just the first request) and incrementing it by 1 otherwise. + + It can be used to limit the maximum depth to scrape, control request + priority based on their depth, and things like that, through the + :setting:`DEPTH_LIMIT`, :setting:`DEPTH_STATS_VERBOSE` and + :setting:`DEPTH_PRIORITY` settings. + + .. reqmeta:: depth_reset + + depth_reset + ----------- + + .. versionadded:: VERSION + + :attr:`~scrapy.Request.meta` key that, set to ``True``, gives a request + depth 0 instead of the depth of its source response plus 1, e.g. to keep + :setting:`DEPTH_LIMIT` from applying across a domain change. + """ + crawler: Crawler def __init__( # pylint: disable=super-init-not-called @@ -41,6 +62,7 @@ class DepthMiddleware(BaseSpiderMiddleware): self.stats = stats self.verbose_stats = verbose_stats self.prio = prio + self._ignored_logged = False @classmethod def from_crawler(cls, crawler: Crawler) -> Self: @@ -48,7 +70,6 @@ class DepthMiddleware(BaseSpiderMiddleware): maxdepth = settings.getint("DEPTH_LIMIT") verbose = settings.getbool("DEPTH_STATS_VERBOSE") prio = settings.getint("DEPTH_PRIORITY") - assert crawler.stats o = cls(maxdepth, crawler.stats, verbose, prio) o.crawler = crawler return o @@ -86,19 +107,25 @@ class DepthMiddleware(BaseSpiderMiddleware): def get_processed_request( self, request: Request, response: Response | None ) -> Request | None: + # Consumed here so that it cannot reach response.meta and, from there, + # spread to further requests through a meta copy. + depth_reset = request.meta.pop("depth_reset", False) if response is None: # start requests return request - depth = response.meta["depth"] + 1 + depth = 0 if depth_reset else response.meta["depth"] + 1 request.meta["depth"] = depth if self.prio: request.priority -= depth * self.prio if self.maxdepth and depth > self.maxdepth: - logger.debug( - "Ignoring link (depth > %(maxdepth)d): %(requrl)s ", - {"maxdepth": self.maxdepth, "requrl": request.url}, - extra={"spider": self.crawler.spider}, - ) + if not self._ignored_logged: + logger.debug( + f"Ignoring link (depth > {self.maxdepth}): {request.url}" + " - no more ignored links will be shown", + extra={"spider": self.crawler.spider}, + ) + self._ignored_logged = True + self.stats.inc_value("depth/request_ignored_count") return None if self.verbose_stats: self.stats.inc_value(f"request_depth_count/{depth}") diff --git a/scrapy/spidermiddlewares/httperror.py b/scrapy/spidermiddlewares/httperror.py index 156b73e7e..116a02733 100644 --- a/scrapy/spidermiddlewares/httperror.py +++ b/scrapy/spidermiddlewares/httperror.py @@ -78,9 +78,9 @@ class HttpErrorMiddleware: self, response: Response, exception: Exception, spider: Spider | None = None ) -> Iterable[Any] | None: if isinstance(exception, HttpError): - assert self.crawler.stats - self.crawler.stats.inc_value("httperror/response_ignored_count") - self.crawler.stats.inc_value( + stats = self.crawler.stats + stats.inc_value("httperror/response_ignored_count") + stats.inc_value( f"httperror/response_ignored_status_count/{response.status}" ) logger.info( diff --git a/scrapy/spidermiddlewares/metacopy.py b/scrapy/spidermiddlewares/metacopy.py index aa5a120c6..2bef974d6 100644 --- a/scrapy/spidermiddlewares/metacopy.py +++ b/scrapy/spidermiddlewares/metacopy.py @@ -15,8 +15,7 @@ logger = logging.getLogger(__name__) class MetaCopyDetectionMiddleware(BaseSpiderMiddleware): """Warn when a spider yields a request with internal meta keys that should - not be copied from response.meta, or when two requests share the same meta - dict object. + not be copied from response.meta. Each warning is emitted at most once per crawl. """ diff --git a/scrapy/spidermiddlewares/urllength.py b/scrapy/spidermiddlewares/urllength.py index f325ce7a0..86bdd2ed6 100644 --- a/scrapy/spidermiddlewares/urllength.py +++ b/scrapy/spidermiddlewares/urllength.py @@ -48,6 +48,5 @@ class UrlLengthMiddleware(BaseSpiderMiddleware): {"maxlength": self.maxlength, "url": request.url}, extra={"spider": self.crawler.spider}, ) - assert self.crawler.stats self.crawler.stats.inc_value("urllength/request_ignored_count") return None diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 4767c29f2..ac98a61c8 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -2,11 +2,10 @@ import contextlib import zlib from io import BytesIO -with contextlib.suppress(ImportError): - try: - import brotli - except ImportError: - import brotlicffi as brotli +try: + import brotli +except ImportError: + import brotlicffi as brotli with contextlib.suppress(ImportError): import zstandard diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index a3e7a4306..f08e5303d 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -1,7 +1,8 @@ import posixpath from contextlib import closing -from ftplib import FTP, error_perm +from ftplib import FTP, FTP_TLS, error_perm from posixpath import dirname +from ssl import create_default_context from typing import IO @@ -29,13 +30,20 @@ def ftp_store_file( password: str, use_active_mode: bool = False, overwrite: bool = True, + tls: bool = False, ) -> None: - """Opens a FTP connection with passed credentials,sets current directory - to the directory extracted from given path, then uploads the file to server + """Opens a FTP connection with passed credentials, sets current directory + to the directory extracted from given path, then uploads the file to server. + + If *tls* is ``True``, the connection is secured with TLS (FTPS), and the + certificate of the server is verified. """ - with FTP() as ftp, closing(file): + ftp = FTP_TLS(context=create_default_context()) if tls else FTP() + with ftp, closing(file): ftp.connect(host, port) ftp.login(username, password) + if isinstance(ftp, FTP_TLS): + ftp.prot_p() if use_active_mode: ftp.set_pasv(False) file.seek(0) diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 7645b235e..bfa39169f 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -239,13 +239,12 @@ class LogCounterHandler(logging.Handler): def emit(self, record: logging.LogRecord) -> None: sname = f"log_count/{record.levelname}" - assert self.crawler.stats self.crawler.stats.inc_value(sname) def logformatter_adapter( logkws: LogFormatterResult, -) -> tuple[int, str, dict[str, Any] | tuple[Any, ...]]: +) -> tuple[Any, ...]: """ Helper that takes the dictionary output from the methods in LogFormatter and adapts it into a tuple of positional arguments for logger.log calls. @@ -253,10 +252,14 @@ def logformatter_adapter( level = logkws.get("level", logging.INFO) message = logkws.get("msg") or "" - # NOTE: This also handles 'args' being an empty dict, that case doesn't - # play well in logger.log calls - args = cast("dict[str, Any]", logkws) if not logkws.get("args") else logkws["args"] - + args = logkws.get("args") + # logging interpolates the message whenever it receives any positional + # argument, so empty args are left out. Tuple args become one positional + # argument each, while a dict is a single positional argument. + if not args: + return (level, message) + if isinstance(args, tuple): + return (level, message, *args) return (level, message, args) diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index 7747a7b9b..b3622c159 100644 --- a/scrapy/utils/response.py +++ b/scrapy/utils/response.py @@ -90,6 +90,10 @@ def open_in_browser( def parse_details(self, response): if "item name" not in response.text: open_in_browser(response) + + On the Windows Subsystem for Linux, set the ``BROWSER`` environment + variable to `wslview `_ to open the + response in a Windows browser, which cannot read Linux paths otherwise. """ # circular imports from scrapy.http import HtmlResponse, TextResponse # noqa: PLC0415 diff --git a/scrapy/utils/serialize.py b/scrapy/utils/serialize.py index 5d06bbe30..803b63bef 100644 --- a/scrapy/utils/serialize.py +++ b/scrapy/utils/serialize.py @@ -10,18 +10,11 @@ from scrapy.http import Request, Response class ScrapyJSONEncoder(json.JSONEncoder): - DATE_FORMAT = "%Y-%m-%d" - TIME_FORMAT = "%H:%M:%S" - def default(self, o: Any) -> Any: if isinstance(o, set): return list(o) - if isinstance(o, datetime.datetime): - return o.strftime(f"{self.DATE_FORMAT} {self.TIME_FORMAT}") - if isinstance(o, datetime.date): - return o.strftime(self.DATE_FORMAT) - if isinstance(o, datetime.time): - return o.strftime(self.TIME_FORMAT) + if isinstance(o, (datetime.datetime, datetime.date, datetime.time)): + return o.isoformat() if isinstance(o, decimal.Decimal): return str(o) if isinstance(o, defer.Deferred): diff --git a/scrapy/utils/trackref.py b/scrapy/utils/trackref.py index 0bbe68def..2882b7f96 100644 --- a/scrapy/utils/trackref.py +++ b/scrapy/utils/trackref.py @@ -14,7 +14,6 @@ This library has a minimal performance impact. from __future__ import annotations -from collections import defaultdict from operator import itemgetter from time import monotonic_ns from types import NoneType @@ -28,8 +27,8 @@ if TYPE_CHECKING: from typing_extensions import Self -live_refs: defaultdict[type, WeakKeyDictionary[object, float]] = defaultdict( - WeakKeyDictionary +live_refs: WeakKeyDictionary[type, WeakKeyDictionary[object, float]] = ( + WeakKeyDictionary() ) @@ -41,7 +40,11 @@ class object_ref: def __new__(cls, *args: Any, **kwargs: Any) -> Self: obj = object.__new__(cls) - live_refs[cls][obj] = monotonic_ns() + try: + refs = live_refs[cls] + except KeyError: + refs = live_refs[cls] = WeakKeyDictionary() + refs[obj] = monotonic_ns() return obj diff --git a/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py b/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py index 55d2ef711..8181c4d17 100644 --- a/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py +++ b/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py @@ -8,7 +8,10 @@ class CachingHostnameResolverSpider(scrapy.Spider): """ name = "caching_hostname_resolver_spider" - start_urls = ["http://[::1]"] + + async def start(self): + # w3lib older than 2.4.1 strips the brackets, making the URL invalid. + yield scrapy.Request("http://[::1]", meta={"verbatim_url": True}) if __name__ == "__main__": diff --git a/tests/AsyncCrawlerProcess/default_name_resolver.py b/tests/AsyncCrawlerProcess/default_name_resolver.py index 4c8897f8f..7cc59594b 100644 --- a/tests/AsyncCrawlerProcess/default_name_resolver.py +++ b/tests/AsyncCrawlerProcess/default_name_resolver.py @@ -9,7 +9,10 @@ class IPv6Spider(scrapy.Spider): """ name = "ipv6_spider" - start_urls = ["http://[::1]"] + + async def start(self): + # w3lib older than 2.4.1 strips the brackets, making the URL invalid. + yield scrapy.Request("http://[::1]", meta={"verbatim_url": True}) if __name__ == "__main__": diff --git a/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py b/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py index da9c16cb8..f6f865e3e 100644 --- a/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py +++ b/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py @@ -8,7 +8,10 @@ class CachingHostnameResolverSpider(scrapy.Spider): """ name = "caching_hostname_resolver_spider" - start_urls = ["http://[::1]"] + + async def start(self): + # w3lib older than 2.4.1 strips the brackets, making the URL invalid. + yield scrapy.Request("http://[::1]", meta={"verbatim_url": True}) if __name__ == "__main__": diff --git a/tests/CrawlerProcess/default_name_resolver.py b/tests/CrawlerProcess/default_name_resolver.py index f4c129fdf..12b894030 100644 --- a/tests/CrawlerProcess/default_name_resolver.py +++ b/tests/CrawlerProcess/default_name_resolver.py @@ -9,7 +9,10 @@ class IPv6Spider(scrapy.Spider): """ name = "ipv6_spider" - start_urls = ["http://[::1]"] + + async def start(self): + # w3lib older than 2.4.1 strips the brackets, making the URL invalid. + yield scrapy.Request("http://[::1]", meta={"verbatim_url": True}) if __name__ == "__main__": diff --git a/tests/benchmarks/__init__.py b/tests/benchmarks/__init__.py index 7b5ca0cb9..a066ed00a 100644 --- a/tests/benchmarks/__init__.py +++ b/tests/benchmarks/__init__.py @@ -1,14 +1,53 @@ from __future__ import annotations +import asyncio from typing import TYPE_CHECKING, Any +from scrapy.http import Response from scrapy.utils.test import get_crawler if TYPE_CHECKING: - from scrapy import Spider + from scrapy import Request, Spider from scrapy.crawler import Crawler +class NullDownloadHandler: + """Download handler that returns an empty response without doing any I/O. + + It lets benchmarks measure the engine, the scheduler and the middlewares + without also measuring HTTP parsing and socket handling, and reach as many + hostnames as they need without DNS resolution. + + It yields control to the event loop once per request, so that requests can + be in progress at the same time and concurrency limits apply. The peak + number of requests in progress is tracked in the + ``benchmark/peak_concurrency`` stat. + """ + + lazy = False + + def __init__(self, crawler: Crawler): + self._crawler = crawler + self._active = 0 + + @classmethod + def from_crawler(cls, crawler: Crawler) -> NullDownloadHandler: + return cls(crawler) + + async def download_request(self, request: Request) -> Response: + self._active += 1 + assert self._crawler.stats + self._crawler.stats.max_value("benchmark/peak_concurrency", self._active) + try: + await asyncio.sleep(0) + return Response(request.url, request=request) + finally: + self._active -= 1 + + async def close(self) -> None: + pass + + def crawl(spidercls: type[Spider], settings: dict[str, Any], **kwargs: Any) -> Crawler: """Run a crawl to completion and return its crawler. diff --git a/tests/benchmarks/test_crawl.py b/tests/benchmarks/test_crawl.py index 0fdfe742b..f79b66a5b 100644 --- a/tests/benchmarks/test_crawl.py +++ b/tests/benchmarks/test_crawl.py @@ -1,5 +1,7 @@ from __future__ import annotations +import asyncio +from collections import Counter from typing import TYPE_CHECKING, Any from urllib.parse import urlencode @@ -7,13 +9,14 @@ import pytest from scrapy import Field, Item, Request, Spider from scrapy.linkextractors import LinkExtractor -from tests.benchmarks import crawl +from tests.benchmarks import NullDownloadHandler, crawl if TYPE_CHECKING: from collections.abc import AsyncIterator from pytest_codspeed import BenchmarkFixture # type: ignore[import-not-found] + from scrapy.crawler import Crawler from scrapy.http import Response from tests.mockserver.http import MockServer @@ -22,6 +25,34 @@ pytest.importorskip("pytest_codspeed", reason="Benchmarks require pytest-codspee PAGES = 100 LINKS_PER_PAGE = 5 +# Requests per crawl of the benchmarks that use NullDownloadHandler. The broad +# crawl scenarios split them differently between hostnames and pages per +# hostname. +REQUESTS = 200 +BROAD_DEEP_PAGES = 10 + +# Requests per crawl and delay of the benchmarks that wait, where wall time, +# unlike in the other benchmarks, is a function of the delay. +DELAYED_REQUESTS = 50 +DELAY = 0.005 + +# Requests per crawl and items per response of the benchmarks that measure item +# processing, which reaches fewer pages than the other benchmarks because every +# page costs it several items. +ITEM_REQUESTS = 20 +ITEMS_PER_RESPONSE = 100 + +# Item concurrency limits of the benchmarks that measure item processing. The +# high limit is above the number of items that a response yields in any of +# them. +HIGH_CONCURRENT_ITEMS = 1000 +DELAYED_CONCURRENT_ITEMS = 50 + +NULL_SETTINGS: dict[str, Any] = { + "DOWNLOAD_HANDLERS": {"http": NullDownloadHandler}, + "LOG_ENABLED": False, +} + class _Page(Item): url = Field() @@ -45,11 +76,83 @@ class _FollowSpider(Spider): yield Request(link.url) +class _TreeSpider(Spider): + """Crawl *pages* pages on each of *domains* hostnames, yielding *items* + items from every page. + + Pages are numbered from 1, and page *n* links to pages *2n* and *2n+1*, so + that requests also reach the scheduler from callbacks, and not only from + :meth:`~scrapy.Spider.start`. + """ + + name = "benchmark-tree" + domains: int = 1 + pages: int = 1 + items: int = 0 + + async def start(self) -> AsyncIterator[Any]: + for domain in range(self.domains): + yield Request(f"http://d{domain}.example.com/1") + + def parse(self, response: Response) -> Any: + page = int(response.url.rpartition("/")[2]) + for child in (page * 2, page * 2 + 1): + if child <= self.pages: + yield Request(response.urljoin(f"/{child}")) + for _ in range(self.items): + yield _Page(url=response.url) + + class _Pipeline: def process_item(self, item: Any) -> Any: return item +class _DelayedPipeline: + """Item pipeline that waits, so that the item concurrency limit applies. + + The peak number of items of a same response in progress is tracked in the + ``benchmark/peak_items`` stat. Items are counted per response because the + limit is per response, and the items of a response are processed while + later responses are already being downloaded. + """ + + def __init__(self, crawler: Crawler): + self._crawler = crawler + self._active: Counter[str] = Counter() + + @classmethod + def from_crawler(cls, crawler: Crawler) -> _DelayedPipeline: + return cls(crawler) + + async def process_item(self, item: Any) -> Any: + url = item["url"] + self._active[url] += 1 + assert self._crawler.stats + self._crawler.stats.max_value("benchmark/peak_items", self._active[url]) + try: + await asyncio.sleep(DELAY) + return item + finally: + self._active[url] -= 1 + + +def _crawl_tree( + settings: dict[str, Any], *, domains: int, pages: int, items: int = 0 +) -> Crawler: + crawler = crawl( + _TreeSpider, + {**NULL_SETTINGS, **settings}, + domains=domains, + pages=pages, + items=items, + ) + assert crawler.stats + assert crawler.stats.get_value("downloader/response_count") == domains * pages + assert crawler.stats.get_value("item_scraped_count", 0) == domains * pages * items + return crawler + + def test_overhead_http(benchmark: BenchmarkFixture, mockserver: MockServer) -> None: """Per-request overhead of a crawl over HTTP. @@ -67,3 +170,101 @@ def test_overhead_http(benchmark: BenchmarkFixture, mockserver: MockServer) -> N assert crawler.stats.get_value("item_scraped_count") == PAGES + 1 benchmark(run) + + +def test_overhead_engine(benchmark: BenchmarkFixture) -> None: + """Per-request overhead of a crawl of a single hostname without any I/O.""" + + def run() -> None: + crawler = _crawl_tree({}, domains=1, pages=REQUESTS) + assert crawler.stats + assert crawler.stats.get_value("benchmark/peak_concurrency") > 1 + + benchmark(run) + + +@pytest.mark.parametrize( + ("domains", "pages"), + [ + pytest.param(REQUESTS, 1, id="shallow"), + pytest.param(REQUESTS // BROAD_DEEP_PAGES, BROAD_DEEP_PAGES, id="deep"), + ], +) +def test_overhead_broad(benchmark: BenchmarkFixture, domains: int, pages: int) -> None: + """Per-request overhead of a broad crawl. + + The shallow scenario, which reaches a single page of every hostname, pays + the cost of tracking a hostname for the first time on every request, and + gets its requests from :meth:`~scrapy.Spider.start`. The deep scenario, + which reaches the same number of pages spread over fewer hostnames, + amortizes that cost, and instead keeps several requests per hostname + waiting in the scheduler. + """ + benchmark(lambda: _crawl_tree({}, domains=domains, pages=pages)) + + +def test_overhead_concurrency(benchmark: BenchmarkFixture) -> None: + """Overhead of a crawl limited to 1 request at a time on a single hostname.""" + settings = {"CONCURRENT_REQUESTS_PER_DOMAIN": 1} + benchmark(lambda: _crawl_tree(settings, domains=1, pages=REQUESTS)) + + +def test_overhead_delay(benchmark: BenchmarkFixture) -> None: + """Overhead of a crawl where every request waits for a download delay. + + The delay is not randomized, so that wall time, and hence the number of + reactor iterations that the crawl needs, does not change between runs. + """ + settings = {"DOWNLOAD_DELAY": DELAY, "RANDOMIZE_DOWNLOAD_DELAY": False} + benchmark(lambda: _crawl_tree(settings, domains=1, pages=DELAYED_REQUESTS)) + + +@pytest.mark.parametrize( + ("items", "settings"), + [ + pytest.param(1, {}, id="single"), + pytest.param(ITEMS_PER_RESPONSE, {}, id="many"), + pytest.param( + 1, + {"CONCURRENT_ITEMS": HIGH_CONCURRENT_ITEMS}, + id="high-limit", + ), + ], +) +def test_overhead_items( + benchmark: BenchmarkFixture, items: int, settings: dict[str, Any] +) -> None: + """Overhead of sending the items of a callback through the item pipeline. + + The single and many scenarios, which use the default + :setting:`CONCURRENT_ITEMS` value, measure how that overhead grows with the + number of items that a response yields. The high-limit scenario instead + raises :setting:`CONCURRENT_ITEMS` well above that number. + """ + benchmark( + lambda: _crawl_tree(settings, domains=1, pages=ITEM_REQUESTS, items=items) + ) + + +def test_overhead_item_concurrency(benchmark: BenchmarkFixture) -> None: + """Overhead of a crawl where item processing waits. + + Every response yields more items than :setting:`CONCURRENT_ITEMS` allows in + parallel, so that the item pipeline gets them in several batches, and wall + time, unlike in most of the other benchmarks, is a function of the delay. + """ + settings = { + "CONCURRENT_ITEMS": DELAYED_CONCURRENT_ITEMS, + "ITEM_PIPELINES": {_DelayedPipeline: 100}, + } + + def run() -> None: + crawler = _crawl_tree( + settings, domains=1, pages=ITEM_REQUESTS, items=ITEMS_PER_RESPONSE + ) + assert crawler.stats + assert ( + crawler.stats.get_value("benchmark/peak_items") == DELAYED_CONCURRENT_ITEMS + ) + + benchmark(run) diff --git a/tests/benchmarks/test_urls.py b/tests/benchmarks/test_urls.py new file mode 100644 index 000000000..acc1d4d80 --- /dev/null +++ b/tests/benchmarks/test_urls.py @@ -0,0 +1,152 @@ +from __future__ import annotations + +from html import escape +from pathlib import Path +from typing import TYPE_CHECKING, Any + +import pytest + +from scrapy import Request +from scrapy.http import HtmlResponse +from scrapy.linkextractors import LinkExtractor +from scrapy.utils.request import fingerprint + +if TYPE_CHECKING: + from pytest_codspeed import BenchmarkFixture # type: ignore[import-not-found] + +pytest.importorskip("pytest_codspeed", reason="Benchmarks require pytest-codspeed") + +RESPONSE_URL = "https://www.example.com/catalogue/page-1.html" + +# Links that each scenario returns for the benchmark page. They are fewer than +# the anchors of the page because links to images, to other non-crawlable files +# and to non-HTTP schemes are rejected, and, except in the scenario that keeps +# duplicates, because the links that the navigation repeats are collapsed. +LINKS = 63 +DUPLICATE_LINKS = 88 +CANONICAL_LINKS = 60 +FILTERED_LINKS = 45 + +# Requests built from LINKS links that point to a different resource. +# Canonicalization maps the rest to one that another link already covers, e.g. +# two fragments of a page, or two spellings of one percent-escape. +FINGERPRINTS = 60 + + +def _read_corpus() -> tuple[list[str], list[str]]: + """Return the URLs of ``urls.txt``, and its first group of URLs. + + The first group is the site navigation, which the benchmark page repeats. + """ + groups: list[list[str]] = [[]] + for line in (Path(__file__).parent / "urls.txt").read_text().splitlines(): + line = line.strip() + if not line or line.startswith("#"): + if groups[-1]: + groups.append([]) + continue + groups[-1].append(line) + urls = [url for group in groups for url in group] + return urls, groups[0] + + +def _build_page(urls: list[str], navigation: list[str]) -> bytes: + """Return an HTML page that links to *urls*. + + Every link is surrounded by the markup of a product listing, so that + benchmarks also cover walking over the elements and attributes that a real + page puts between links. + """ + + def item(index: int, url: str) -> str: + href = escape(url) + return ( + f'
  • ' + f'Product {index}' + f'

    Product {index}

    ' + f'

    A description of product {index}.

    ' + f"
    • " + ) + + def nav(urls: list[str]) -> str: + links = "".join(f'{escape(url)}' for url in urls) + return f'' + + items = "".join(item(index, url) for index, url in enumerate(urls)) + return ( + "Catalogue" + f'' + f'{nav(navigation)}
      {items}
    {nav(navigation)}' + "" + ).encode() + + +URLS, NAVIGATION = _read_corpus() +BODY = _build_page(URLS, NAVIGATION) + + +def _response() -> HtmlResponse: + return HtmlResponse(RESPONSE_URL, body=BODY, encoding="utf-8") + + +@pytest.mark.parametrize( + ("kwargs", "links"), + [ + pytest.param({}, LINKS, id="default"), + pytest.param({"unique": False}, DUPLICATE_LINKS, id="duplicates"), + pytest.param({"canonicalize": True}, CANONICAL_LINKS, id="canonicalize"), + pytest.param( + { + "allow": r"/catalogue/", + "deny": r"/legal/", + "allow_domains": ["example.com", "www.example.com"], + }, + FILTERED_LINKS, + id="filtered", + ), + ], +) +def test_extract_links( + benchmark: BenchmarkFixture, kwargs: dict[str, Any], links: int +) -> None: + """Extraction of every link of a page. + + The scenarios cover the choices that change which work dominates: + deduplication and canonicalization both build a key for every link, and the + filters of a configured extractor reject links before the later checks, + which the default extractor reaches for every link. + """ + link_extractor = LinkExtractor(**kwargs) + + def run() -> None: + assert len(link_extractor.extract_links(_response())) == links + + benchmark(run) + + +EXTRACTED_URLS = [link.url for link in LinkExtractor().extract_links(_response())] + + +def test_requests(benchmark: BenchmarkFixture) -> None: + """Building a request for every link of a page.""" + + def run() -> None: + assert len([Request(url) for url in EXTRACTED_URLS]) == LINKS + + benchmark(run) + + +def test_fingerprints(benchmark: BenchmarkFixture) -> None: + """Fingerprinting the request of every link of a page. + + Requests are built here as well, and not once for all rounds, because + fingerprints are cached per request object. + """ + + def run() -> None: + assert ( + len({fingerprint(Request(url)) for url in EXTRACTED_URLS}) == FINGERPRINTS + ) + + benchmark(run) diff --git a/tests/benchmarks/urls.txt b/tests/benchmarks/urls.txt new file mode 100644 index 000000000..6ef6939f1 --- /dev/null +++ b/tests/benchmarks/urls.txt @@ -0,0 +1,130 @@ +# Link targets for the URL benchmarks, as they would appear in the href +# attribute of a page at https://www.example.com/catalogue/page-1.html. +# +# Cost per URL varies by shape: the number of query parameters drives the +# parsing and re-encoding of the query string, non-ASCII characters and +# unescaped characters drive percent-encoding, and non-default ports, dot +# segments and uppercase host names drive normalization. A corpus of uniform +# URLs would therefore measure one shape and miss the others, so this one +# covers each of them, in roughly the proportion of a real listing page. +# +# Blank lines and lines starting with "#" are ignored. + +# Site navigation. These also appear in a second copy of the navigation at the +# end of the page, so that deduplication has duplicates to collapse. +/ +/index.html +/about-us +/contact +/catalogue/ +/catalogue/page-2.html +/catalogue/page-3.html +/help/faq +/help/shipping-and-returns +/legal/terms +/legal/privacy + +# Relative paths of increasing depth. +detail.html +./detail.html +../catalogue/page-4.html +../../index.html +/catalogue/category/books/fiction/index.html +/catalogue/category/books/travel/mystery/historical/index.html +/a/b/c/d/e/f/g/h/i/j/k/index.html + +# One query parameter. +/catalogue/search?q=book +/catalogue/page-1.html?page=2 +/catalogue/detail?id=1042 + +# Several query parameters, in an order that canonicalization changes. +/catalogue/search?q=book&sort=price +/catalogue/search?sort=price&q=book +/catalogue/search?q=book&sort=price&page=3&per_page=20&in_stock=1 +/catalogue/search?zone=eu&q=book&min=10&max=90&sort=rating&page=2&view=grid&lang=en¤cy=EUR&ref=nav + +# Repeated keys, blank values and a bare key. +/catalogue/search?tag=fiction&tag=travel&tag=history +/catalogue/search?q=&sort= +/catalogue/search?featured + +# Characters that need percent-encoding. +/catalogue/search?q=cheap books +/catalogue/detail/a book about books.html +/catalogue/search?q=100%+cotton +/catalogue/search?price=%3E10&title=A%20%26%20B + +# Percent-escapes that are already valid, in both cases. +/catalogue/detail/%C3%A9dition-limit%C3%A9e.html +/catalogue/detail/%c3%a9dition-limit%c3%a9e.html +/catalogue/detail/%7Especial.html + +# Non-ASCII in the path and in the query. +/catalogue/detail/édition-limitée.html +/catalogue/search?q=édition +/catalogue/búsqueda?q=libro&categoría=ficción +/カタログ/詳細.html + +# Internationalized host names, encoded and decoded. +https://例え.テスト/catalogue/page-1.html +https://xn--r8jz45g.xn--zckzah/catalogue/page-2.html + +# Absolute URLs on the same host, on other hosts, and protocol-relative. +https://www.example.com/catalogue/page-5.html +https://www.example.com/catalogue/detail?id=1043 +http://www.example.com/catalogue/page-6.html +https://shop.example.com/catalogue/page-1.html +https://www.example.org/reviews/1042 +https://books.toscrape.com/catalogue/page-1.html +//cdn.example.com/catalogue/page-7.html +//www.example.com/catalogue/page-8.html + +# Ports, including the default one for the scheme. +https://www.example.com:443/catalogue/page-9.html +http://www.example.com:80/catalogue/page-10.html +https://staging.example.com:8443/catalogue/page-1.html + +# Host name case, which normalization lowercases. +https://WWW.EXAMPLE.COM/catalogue/Page-11.html +HTTPS://www.example.com/catalogue/page-12.html + +# Dot segments, empty segments and trailing slashes, which WHATWG +# normalization resolves and the standard library keeps. +/catalogue/../catalogue/page-13.html +/catalogue/./page-14.html +/catalogue//page-15.html +/catalogue/category/ +/catalogue/category + +# Fragments, which canonicalization drops and the deduplication key keeps. +/catalogue/page-16.html#reviews +/catalogue/page-16.html#description +/catalogue/page-17.html# +#top + +# Path parameters, where the semicolon is not the last segment. +/catalogue;sessionid=abc123/page-18.html +/catalogue/page-19.html;sessionid=abc123 + +# User information in the authority. +https://user:password@files.example.com/catalogue/page-1.html + +# A long URL, of the length that tracking parameters reach. +/catalogue/search?q=book&utm_source=newsletter&utm_medium=email&utm_campaign=spring-sale-2026&utm_term=fiction%20paperback&utm_content=hero-banner-variant-b&session=6f1c9a2e4b7d8f0a1c3e5d7b9f2a4c6e&ref=https%3A%2F%2Fwww.example.org%2Freviews%2F1042&page=2&sort=relevance + +# Extensions that the default deny_extensions rejects, and one compound +# extension, which only matches as a whole. +/media/cover-1042.jpg +/media/cover-1042.PNG +/media/catalogue.pdf +/static/style.css +/static/app.js +/downloads/catalogue.tar.gz +/downloads/catalogue.zip + +# Schemes that are not crawlable, which are rejected before any parsing. +mailto:orders@example.com +javascript:void(0) +tel:+441234567890 +data:text/plain,hello diff --git a/tests/ignores.txt b/tests/ignores.txt index 3717bbc95..5be8576f3 100644 --- a/tests/ignores.txt +++ b/tests/ignores.txt @@ -1,3 +1,4 @@ scrapy/core/downloader/handlers/http.py scrapy/extensions/statsmailer.py +scrapy/interfaces.py scrapy/mail.py diff --git a/tests/keys/__init__.py b/tests/keys/__init__.py index 9b73ca4f0..804c9b6a8 100644 --- a/tests/keys/__init__.py +++ b/tests/keys/__init__.py @@ -1,4 +1,5 @@ from datetime import datetime, timedelta, timezone +from ipaddress import IPv4Address from pathlib import Path from cryptography.hazmat.backends import default_backend @@ -12,6 +13,7 @@ from cryptography.hazmat.primitives.serialization import ( from cryptography.x509 import ( CertificateBuilder, DNSName, + IPAddress, Name, NameAttribute, SubjectAlternativeName, @@ -53,7 +55,9 @@ def generate_keys(): .not_valid_before(datetime.now(tz=timezone.utc)) .not_valid_after(datetime.now(tz=timezone.utc) + timedelta(days=10)) .add_extension( - SubjectAlternativeName([DNSName("localhost")]), + SubjectAlternativeName( + [DNSName("localhost"), IPAddress(IPv4Address("127.0.0.1"))] + ), critical=False, ) .sign(key, SHA256(), default_backend()) diff --git a/tests/mockserver/ftp.py b/tests/mockserver/ftp.py index 1edd64dda..d88e953a7 100644 --- a/tests/mockserver/ftp.py +++ b/tests/mockserver/ftp.py @@ -10,7 +10,7 @@ from tempfile import mkdtemp from typing import TYPE_CHECKING from pyftpdlib.authorizers import DummyAuthorizer -from pyftpdlib.handlers import FTPHandler +from pyftpdlib.handlers import FTPHandler, TLS_FTPHandler from pyftpdlib.servers import FTPServer from tests.utils import get_script_run_env @@ -25,27 +25,32 @@ if TYPE_CHECKING: class MockFTPServer: """Creates an FTP server on a random port with a default passwordless user (anonymous) and a temporary root path that you can read from the - :attr:`path` attribute.""" + :attr:`path` attribute. - def __init__(self) -> None: - self.proc: Popen[str] | None = None + If *tls* is ``True``, the server requires FTPS, using the test certificate + from :file:`tests/keys`. + """ + + proc: Popen[str] + port: int + path: Path + + def __init__(self, tls: bool = False) -> None: self.host: str = "127.0.0.1" - self.port: int | None = None - self.path: Path | None = None + self.tls: bool = tls def __enter__(self) -> Self: self.path = Path(mkdtemp()) self.proc = Popen( - [sys.executable, "-u", "-m", "tests.mockserver.ftp", "-d", str(self.path)], + [sys.executable, "-u", "-m", "tests.mockserver.ftp", "-d", str(self.path)] + + (["--tls"] if self.tls else []), stderr=PIPE, env=get_script_run_env(), text=True, ) assert self.proc.stderr is not None for line in self.proc.stderr: - if "starting FTP server" in line and ( - m := re.search(r"starting FTP server on ([^ :]+):(\d+),", line) - ): + if m := re.search(r"starting FTPS? .*on ([^ :]+):(\d+),", line): self.port = int(m.group(2)) break else: @@ -63,23 +68,32 @@ class MockFTPServer: traceback: TracebackType | None, ) -> None: rmtree(str(self.path)) - assert self.proc is not None self.proc.kill() self.proc.communicate() def url(self, path: str) -> str: - return f"ftp://{self.host}:{self.port}/{path}" + scheme = "ftps" if self.tls else "ftp" + return f"{scheme}://{self.host}:{self.port}/{path}" def main() -> None: parser = ArgumentParser() parser.add_argument("-d", "--directory", required=True) + parser.add_argument("--tls", action="store_true") args = parser.parse_args() authorizer = DummyAuthorizer() full_permissions = "elradfmwMT" authorizer.add_anonymous(args.directory, perm=full_permissions) - handler = FTPHandler + if args.tls: + keys = Path(__file__).parent.parent / "keys" + handler = TLS_FTPHandler + handler.certfile = str(keys / "localhost.crt") + handler.keyfile = str(keys / "localhost.key") + handler.tls_control_required = True + handler.tls_data_required = True + else: + handler = FTPHandler handler.authorizer = authorizer address = ("127.0.0.1", 0) server = FTPServer(address, handler) diff --git a/tests/mockserver/http.py b/tests/mockserver/http.py index e5f3e758a..5226b8f5b 100644 --- a/tests/mockserver/http.py +++ b/tests/mockserver/http.py @@ -11,6 +11,7 @@ from tests import tests_datadir from .http_base import BaseMockServer, main_factory from .http_resources import ( ArbitraryLengthPayloadResource, + BadHeader, BaseResource, BrokenChunkedResource, BrokenDownloadResource, @@ -52,6 +53,7 @@ class Root(BaseResource): put_child(self, b"partial", Partial()) put_child(self, b"drop", Drop()) put_child(self, b"raw", Raw()) + put_child(self, b"bad-header", BadHeader()) put_child(self, b"echo", Echo()) put_child(self, b"payload", PayloadResource()) put_child(self, b"alpayload", ArbitraryLengthPayloadResource()) diff --git a/tests/mockserver/http_resources.py b/tests/mockserver/http_resources.py index cb028bc10..969e29a8c 100644 --- a/tests/mockserver/http_resources.py +++ b/tests/mockserver/http_resources.py @@ -210,6 +210,39 @@ class Raw(LeafResource): request.finish() +class BadHeader(LeafResource): + """Sends a response with a bad header line, one with no colon in it, like + some servers do, between two good ones. + + One of the good header lines is split into two lines, so that handling of + such headers is also covered. + """ + + response = ( + b"HTTP/1.1 200 OK\r\n" + b"Content-Length: 5\r\n" + b"Content-Type: text/html\r\n" + b"X-Folded-Header: one\r\n" + b"\ttwo\r\n" + b'\r\n' + b"X-After-Bad-Header: works\r\n" + b"\r\n" + b"Works" + ) + + def render_GET(self, request: Request) -> int: + request.startedWriting = 1 + self.deferRequest(request, 0, self._delayedRender, request) + return NOT_DONE_YET + + def _delayedRender(self, request: Request) -> None: + request.write(self.response) + # Clients that stop parsing headers at the bad one don't get + # Content-Length, so they need the connection to be closed to know that + # the response body is over. + close_connection(request) + + class Echo(LeafResource): def render_GET(self, request: Request) -> bytes: assert request.content diff --git a/tests/test_cmdline/__init__.py b/tests/test_cmdline/__init__.py index 98a85bc17..f6ebe5865 100644 --- a/tests/test_cmdline/__init__.py +++ b/tests/test_cmdline/__init__.py @@ -1,3 +1,4 @@ +import ast import json import os import pstats @@ -60,11 +61,8 @@ class TestCmdline: "-s", "EXTENSIONS=" + json.dumps(EXTENSIONS), ) - # XXX: There's gotta be a smarter way to do this... assert "..." not in settingsstr - for char in ("'", "<", ">"): - settingsstr = settingsstr.replace(char, '"') - settingsdict = json.loads(settingsstr) + settingsdict = ast.literal_eval(settingsstr) assert set(settingsdict.keys()) == set(EXTENSIONS.keys()) assert settingsdict[EXT_PATH] == 200 diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 29667a1ae..6bc1ebbbb 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -201,7 +201,7 @@ class TestInteractiveShell: env = os.environ.copy() env["SCRAPY_PYTHON_SHELL"] = "python" logfile = BytesIO() - p = PopenSpawn(args, env=env, timeout=5) + p = PopenSpawn(args, env=env, timeout=60) p.logfile_read = logfile p.expect_exact("Available Scrapy objects") p.sendline(f"fetch('{mockserver.url('/')}')") @@ -235,7 +235,7 @@ class TestInteractiveShell: def _run_interactive_shell(self, env: dict[str, str]) -> str: args = (sys.executable, "-m", "scrapy.cmdline", "shell") logfile = BytesIO() - p = PopenSpawn(args, env=env, timeout=5) + p = PopenSpawn(args, env=env, timeout=60) p.logfile_read = logfile p.expect_exact("Available Scrapy objects") p.sendeof() @@ -256,7 +256,7 @@ class TestInteractiveShell: self._isolate_config(env, config_home) args = (sys.executable, "-m", "scrapy.cmdline", "shell") logfile = BytesIO() - p = PopenSpawn(args, env=env, timeout=10) + p = PopenSpawn(args, env=env, timeout=60) p.logfile_read = logfile p.expect_exact("Available Scrapy objects") # The standard Python shell never imports IPython, whereas the IPython diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 358f20ed7..17ca02dea 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -74,6 +74,31 @@ class TestCrawler: assert not settings.frozen assert crawler.settings.frozen + @pytest.mark.parametrize( + "attr", + ["extensions", "logformatter", "request_fingerprinter", "stats"], + ) + def test_late_attr_before_apply_settings(self, attr: str) -> None: + crawler = get_raw_crawler(DefaultSpider) + with pytest.raises(RuntimeError, match=rf"Crawler\.{attr} is not set yet"): + getattr(crawler, attr) + crawler._apply_settings() + assert getattr(crawler, attr) is not None + + @pytest.mark.parametrize( + "attr", + ["engine", "extensions", "logformatter", "request_fingerprinter", "stats"], + ) + def test_late_attr_on_class(self, attr: str) -> None: + # Introspection tools such as help() read these off the class. + assert getattr(Crawler, attr) is getattr(Crawler, attr) + + def test_late_attr_engine_before_crawl(self) -> None: + crawler = get_raw_crawler(DefaultSpider) + crawler._apply_settings() + with pytest.raises(RuntimeError, match=r"Crawler\.engine is not set yet"): + _ = crawler.engine + @pytest.mark.parametrize( ("attr", "setting"), [ diff --git a/tests/test_crawler_subprocess.py b/tests/test_crawler_subprocess.py index 733b6797d..defee1041 100644 --- a/tests/test_crawler_subprocess.py +++ b/tests/test_crawler_subprocess.py @@ -10,9 +10,7 @@ from pathlib import Path from typing import TYPE_CHECKING import pytest -from packaging.version import parse as parse_version from pexpect.popen_spawn import PopenSpawn -from w3lib import __version__ as w3lib_version from scrapy.utils.asyncio import sleep from tests.utils import get_script_run_env @@ -97,10 +95,6 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): ) assert "RuntimeError" not in log - @pytest.mark.skipif( - parse_version(w3lib_version) >= parse_version("2.0.0"), - reason="w3lib 2.0.0 and later do not allow invalid domains.", - ) def test_ipv6_default_name_resolver(self) -> None: log = self.run_script("default_name_resolver.py") assert "Spider closed (finished)" in log @@ -116,6 +110,7 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): def test_caching_hostname_resolver_ipv6(self) -> None: log = self.run_script("caching_hostname_resolver_ipv6.py") assert "Spider closed (finished)" in log + assert "http://::1" not in log assert "scrapy.exceptions.CannotResolveHostError" not in log def test_caching_hostname_resolver_finite_execution( @@ -405,7 +400,7 @@ class TestAsyncCrawlerProcessSubprocess(TestCrawlerProcessSubprocessBase): def test_reactorless_import_hook(self) -> None: log = self.run_script("reactorless_import_hook.py") assert "Not using a Twisted reactor" in log - assert "Spider closed (finished)" in log + assert "Spider closed (start_error)" in log assert "ImportError: Import of twisted.internet.reactor is forbidden" in log def test_reactorless_import_hook_uninstall(self) -> None: diff --git a/tests/test_downloader_handler_httpx.py b/tests/test_downloader_handler_httpx.py index 976daacaf..27a44227d 100644 --- a/tests/test_downloader_handler_httpx.py +++ b/tests/test_downloader_handler_httpx.py @@ -61,6 +61,7 @@ class HttpxDownloadHandlerMixin: class TestHttp(HttpxDownloadHandlerMixin, TestHttpBase): handler_supports_bindaddress_meta = False + handler_bad_header_handling = "fail" @pytest.mark.skipif( sys.platform == "darwin", @@ -82,6 +83,7 @@ class TestHttp(HttpxDownloadHandlerMixin, TestHttpBase): class TestHttps(HttpxDownloadHandlerMixin, TestHttpsBase): handler_supports_bindaddress_meta = False + handler_bad_header_handling = "fail" tls_log_message = "SSL connection to 127.0.0.1 using protocol TLSv1.3, cipher" @pytest.mark.skip(reason="The check is Twisted-specific") diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 449f2d635..9d4e161f3 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -186,18 +186,6 @@ class TestHttp2TLSVersion(H2DownloadHandlerMixin, TestHttpsTLSVersionBase): class TestHttp2WithCrawler(H2DownloadHandlerMixin, TestHttpWithCrawlerBase): is_secure = True - def test_bytes_received_stop_download_callback(self) -> None: # type: ignore[override] - pytest.skip("bytes_received support is not implemented") - - def test_bytes_received_stop_download_errback(self) -> None: # type: ignore[override] - pytest.skip("bytes_received support is not implemented") - - def test_headers_received_stop_download_callback(self) -> None: # type: ignore[override] - pytest.skip("headers_received support is not implemented") - - def test_headers_received_stop_download_errback(self) -> None: # type: ignore[override] - pytest.skip("headers_received support is not implemented") - @pytest.mark.skip(reason="Proxy support is not implemented yet") class TestHttp2Proxy(H2DownloadHandlerMixin, TestHttpProxyBase): diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 8d999d952..e4b66fe10 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -345,6 +345,30 @@ class TestCookiesMiddleware: assert "Cookie" in request.headers assert request.headers["Cookie"] == b"currencyCookie=USD" + @pytest.mark.parametrize( + ("url", "domain"), + [ + ("http://example-host/", "example-host.local"), + ("http://127.0.0.1/", "127.0.0.1"), + pytest.param( + "http://example-host/", + "example-host", + marks=pytest.mark.xfail( + reason=( + "http.cookiejar accepts a dotless domain for a dotless " + "host but never returns the resulting cookie" + ) + ), + ), + ], + ) + def test_explicit_local_domain(self, url: str, domain: str) -> None: + request = Request( + url, cookies=[{"name": "currencyCookie", "value": "USD", "domain": domain}] + ) + assert self.mw.process_request(request) is None + assert request.headers.get("Cookie") == b"currencyCookie=USD" + @pytest.mark.xfail(reason="Cookie header is not currently being processed") def test_keep_cookie_from_default_request_headers_middleware(self): DEFAULT_REQUEST_HEADERS = {"Cookie": "default=value; asdf=qwerty"} diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index fa0707491..a43bb51ba 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -52,20 +52,6 @@ FORMAT = { } -def _skip_if_no_br() -> None: - try: - try: - import brotli # noqa: PLC0415 - - brotli.Decompressor.can_accept_more_data - except (ImportError, AttributeError): - import brotlicffi # noqa: PLC0415 - - brotlicffi.Decompressor.can_accept_more_data - except (ImportError, AttributeError): - pytest.skip("no brotli support") - - def _skip_if_no_zstd() -> None: pytest.importorskip("zstandard") @@ -161,8 +147,6 @@ class TestHttpCompression: self.assertStatsEqual("httpcompression/response_bytes", 74837) def test_process_response_br(self): - _skip_if_no_br() - response = self._getresponse("br") assert response.request request = response.request @@ -174,32 +158,6 @@ class TestHttpCompression: self.assertStatsEqual("httpcompression/response_count", 1) self.assertStatsEqual("httpcompression/response_bytes", 74837) - def test_process_response_br_unsupported(self, caplog: pytest.LogCaptureFixture): - if find_spec("brotli") is not None or find_spec("brotlicffi") is not None: - pytest.skip("Requires not having brotli support") - response = self._getresponse("br") - assert response.request - request = response.request - assert response.headers["Content-Encoding"] == b"br" - caplog.clear() - with caplog.at_level( - WARNING, logger="scrapy.downloadermiddlewares.httpcompression" - ): - newresponse = self.mw.process_response(request, response) - assert caplog.record_tuples == [ - ( - "scrapy.downloadermiddlewares.httpcompression", - WARNING, - ( - "HttpCompressionMiddleware cannot decode the response for " - "http://scrapytest.org/ from unsupported encoding(s) 'br'. " - "You need to install brotli or brotlicffi >= 1.2.0 to decode 'br'." - ), - ), - ] - assert newresponse is not response - assert newresponse.headers.getlist("Content-Encoding") == [b"br"] - def test_process_response_zstd(self): _skip_if_no_zstd() @@ -550,8 +508,6 @@ class TestHttpCompression: assert cause.decompressed_size < 1_100_000 def test_compression_bomb_setting_br(self): - _skip_if_no_br() - self._test_compression_bomb_setting("br") def test_compression_bomb_setting_deflate(self): @@ -609,8 +565,6 @@ class TestHttpCompression: @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_compression_bomb_spider_attr_br(self): - _skip_if_no_br() - self._test_compression_bomb_spider_attr("br") @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") @@ -643,8 +597,6 @@ class TestHttpCompression: assert cause.decompressed_size < 1_100_000 def test_compression_bomb_request_meta_br(self): - _skip_if_no_br() - self._test_compression_bomb_request_meta("br") def test_compression_bomb_request_meta_deflate(self): @@ -689,8 +641,6 @@ class TestHttpCompression: def test_download_warnsize_setting_br( self, caplog: pytest.LogCaptureFixture ) -> None: - _skip_if_no_br() - self._test_download_warnsize_setting(caplog, "br") def test_download_warnsize_setting_deflate( @@ -744,8 +694,6 @@ class TestHttpCompression: def test_download_warnsize_spider_attr_br( self, caplog: pytest.LogCaptureFixture ) -> None: - _skip_if_no_br() - self._test_download_warnsize_spider_attr(caplog, "br") @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") @@ -799,8 +747,6 @@ class TestHttpCompression: def test_download_warnsize_request_meta_br( self, caplog: pytest.LogCaptureFixture ) -> None: - _skip_if_no_br() - self._test_download_warnsize_request_meta(caplog, "br") def test_download_warnsize_request_meta_deflate( @@ -834,7 +780,6 @@ class TestHttpCompression: return new_response def test_process_truncated_response_br(self): - _skip_if_no_br() resp = self._get_truncated_response("br") assert resp.body.startswith(b" bool: + allowed_domains: list[str] = getattr(spider, "allowed_domains", []) + return urlparse_cached(request).hostname in allowed_domains + + crawler = get_crawler(Spider) + crawler.spider = crawler._create_spider(name="a", allowed_domains=["example.com"]) + mw = RootOnlyOffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(crawler.spider) + assert mw.process_request(Request("https://example.com/1")) is None + with pytest.raises(IgnoreRequest): + mw.process_request(Request("https://www.example.com/1")) + + def test_ignore_request_reason(): crawler = get_crawler(Spider) crawler.spider = crawler._create_spider(name="a", allowed_domains=["example.com"]) @@ -247,3 +263,50 @@ def test_ignore_request_reason(): IgnoreRequest, match=re.escape("Filtered offsite request to 'other.org'") ): mw.process_request(request) + + +class DomainSpider(Spider): + name = "a" + allowed_domains: list[str] + + +def test_dynamic_allowed_domains(): + crawler = get_crawler(DomainSpider) + spider = DomainSpider.from_crawler(crawler, allowed_domains=["a.example"]) + crawler.spider = spider + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + + with pytest.raises(IgnoreRequest): + mw.process_request(Request("https://b.example")) + + spider.allowed_domains.append("b.example") + assert mw.process_request(Request("https://b.example")) is None + + spider.allowed_domains.remove("a.example") + with pytest.raises(IgnoreRequest): + mw.process_request(Request("https://a.example")) + + +def test_dynamic_allowed_domains_caching(): + calls = 0 + + class TrackingMiddleware(OffsiteMiddleware): + def get_host_regex(self, spider: Spider) -> re.Pattern[str]: + nonlocal calls + calls += 1 + return super().get_host_regex(spider) + + crawler = get_crawler(DomainSpider) + spider = DomainSpider.from_crawler(crawler, allowed_domains=["a.example"]) + crawler.spider = spider + mw = TrackingMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + + for _ in range(3): + mw.process_request(Request("https://a.example")) + assert calls == 1 + + spider.allowed_domains.append("b.example") + assert mw.process_request(Request("https://b.example")) is None + assert calls == 2 diff --git a/tests/test_engine.py b/tests/test_engine.py index 8a3bceccb..53eb4a1f6 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -29,7 +29,9 @@ from tests.utils.engine import ( ) if TYPE_CHECKING: - from collections.abc import AsyncIterator + from collections.abc import AsyncIterator, Generator + + from twisted.internet.defer import Deferred from tests.mockserver.http import MockServer @@ -230,3 +232,51 @@ async def test_request_scheduled_signal(): f"{scheduler.enqueued!r} != [{keep_request!r}]" ) crawler.signals.disconnect(signal_handler, signals.request_scheduled) + + +class ClosingPipeline: + def open_spider(self): + raise CloseSpider("pipeline_reason") + + +class TestCloseSpiderOnStartup: + @coroutine_test + async def test_pipeline(self, caplog: pytest.LogCaptureFixture) -> None: + closed: list[str] = [] + + def spider_closed(reason: str) -> None: + closed.append(reason) + + crawler = get_crawler(DefaultSpider, {"ITEM_PIPELINES": {ClosingPipeline: 1}}) + crawler.signals.connect(spider_closed, signals.spider_closed) + with caplog.at_level(logging.INFO): + await crawler.crawl_async() + assert crawler.stats.get_value("finish_reason") == "pipeline_reason" + assert closed == ["pipeline_reason"] + assert "Traceback" not in caplog.text + + @coroutine_test + async def test_spider_opened(self) -> None: + def spider_opened(spider: Spider) -> None: + raise CloseSpider("signal_reason") + + crawler = get_crawler(DefaultSpider) + crawler.signals.connect(spider_opened, signals.spider_opened) + await crawler.crawl_async() + assert crawler.stats.get_value("finish_reason") == "signal_reason" + + @coroutine_test + async def test_startup_wins_over_spider_opened(self) -> None: + def spider_opened(spider: Spider) -> None: + raise CloseSpider("signal_reason") + + crawler = get_crawler(DefaultSpider, {"ITEM_PIPELINES": {ClosingPipeline: 1}}) + crawler.signals.connect(spider_opened, signals.spider_opened) + await crawler.crawl_async() + assert crawler.stats.get_value("finish_reason") == "pipeline_reason" + + @inline_callbacks_test + def test_deferred_crawl(self) -> Generator[Deferred[Any], Any, None]: + crawler = get_crawler(DefaultSpider, {"ITEM_PIPELINES": {ClosingPipeline: 1}}) + yield crawler.crawl() + assert crawler.stats.get_value("finish_reason") == "pipeline_reason" diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index 14ec3d184..1ecf8b8de 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING, Any from scrapy import Request, Spider, signals from scrapy.core.scheduler import BaseScheduler +from scrapy.exceptions import CloseSpider from scrapy.utils.asyncio import call_later, sleep from scrapy.utils.test import get_crawler from tests.mockserver.http import MockServer @@ -140,6 +141,74 @@ class TestMain: assert crawler.stats.get_value("finish_reason") == "shutdown" assert not actual_urls + @coroutine_test + async def test_start_error(self, caplog: pytest.LogCaptureFixture) -> None: + class TestSpider(Spider): + name = "test" + + async def start(self): + yield Request("data:,a") + raise ValueError + + def parse(self, response): + pass + + actual_urls = [] + errors = [] + + def track_url(request, spider): + actual_urls.append(request.url) + + def track_error(failure, response, spider): + errors.append((failure, response)) + + settings = {"SCHEDULER": MemoryScheduler} + crawler = get_crawler(TestSpider, settings_dict=settings) + crawler.signals.connect(track_url, signals.request_reached_downloader) + crawler.signals.connect(track_error, signals.spider_error) + + caplog.clear() + with caplog.at_level(ERROR): + await crawler.crawl_async() + + # The requests yielded before the exception are still crawled. + assert actual_urls == ["data:,a"] + assert len(caplog.records) == 1 + assert len(errors) == 1 + failure, response = errors[0] + assert isinstance(failure.value, ValueError) + assert response is None + assert crawler.stats + assert crawler.stats.get_value("finish_reason") == "start_error" + assert crawler.stats.get_value("spider_exceptions/count") == 1 + assert crawler.stats.get_value("spider_exceptions/ValueError") == 1 + + @coroutine_test + async def test_close_spider_from_start( + self, caplog: pytest.LogCaptureFixture + ) -> None: + class TestSpider(Spider): + name = "test" + + async def start(self): + yield Request("data:,a") + raise CloseSpider("my_reason") + + def parse(self, response): + pass + + settings = {"SCHEDULER": MemoryScheduler} + crawler = get_crawler(TestSpider, settings_dict=settings) + + caplog.clear() + with caplog.at_level(ERROR): + await crawler.crawl_async() + + assert not caplog.records + assert crawler.stats + assert crawler.stats.get_value("finish_reason") == "my_reason" + assert crawler.stats.get_value("spider_exceptions/count") is None + class TestRequestSendOrder: seconds = 0.1 # increase if flaky diff --git a/tests/test_exporters.py b/tests/test_exporters.py index b857728ba..4359f4ff4 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -4,6 +4,7 @@ import marshal import pickle import re from abc import ABC, abstractmethod +from collections.abc import Mapping from datetime import datetime from io import BytesIO from typing import Any @@ -63,18 +64,18 @@ class TestBaseItemExporter(ABC): self.ie = self._get_exporter() @abstractmethod - def _get_exporter(self, **kwargs) -> BaseItemExporter: + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: raise NotImplementedError - def _check_output(self): # noqa: B027 + def _check_output(self) -> None: # noqa: B027 pass - def _assert_expected_item(self, exported_dict): + def _assert_expected_item(self, exported_dict: dict[str, Any]) -> None: for k, v in exported_dict.items(): exported_dict[k] = to_unicode(v) assert self.i == self.item_class(**exported_dict) - def _get_nonstring_types_item(self): + def _get_nonstring_types_item(self) -> dict[str, Any]: return { "boolean": False, "number": 22, @@ -82,7 +83,7 @@ class TestBaseItemExporter(ABC): "float": 3.14, } - def assertItemExportWorks(self, item): + def assertItemExportWorks(self, item: Any) -> None: self.ie.start_exporting() self.ie.export_item(item) self.ie.finish_exporting() @@ -92,7 +93,7 @@ class TestBaseItemExporter(ABC): del self.ie self._check_output() - def test_export_item(self): + def test_export_item(self) -> None: self.assertItemExportWorks(self.i) def test_export_dict_item(self): @@ -108,26 +109,26 @@ class TestBaseItemExporter(ABC): def test_fields_to_export(self): ie = self._get_exporter(fields_to_export=["name"]) - assert list(ie._get_serialized_fields(self.i)) == [("name", "John\xa3")] + assert list(ie.get_serialized_fields(self.i)) == [("name", "John\xa3")] ie = self._get_exporter(fields_to_export=["name"], encoding="latin-1") - _, name = next(iter(ie._get_serialized_fields(self.i))) + _, name = next(iter(ie.get_serialized_fields(self.i))) assert isinstance(name, str) assert name == "John\xa3" ie = self._get_exporter(fields_to_export={"name": "名稱"}) - assert list(ie._get_serialized_fields(self.i)) == [("名稱", "John\xa3")] + assert list(ie.get_serialized_fields(self.i)) == [("名稱", "John\xa3")] def test_field_order(self): item = self.item_class(age="22", name="John\xa3") ie = self._get_exporter() - assert [name for name, _ in ie._get_serialized_fields(item)] == ["name", "age"] + assert [name for name, _ in ie.get_serialized_fields(item)] == ["name", "age"] def test_field_order_dict_item(self): ie = self._get_exporter() - assert [name for name, _ in ie._get_serialized_fields({"age": "22"})] == ["age"] + assert [name for name, _ in ie.get_serialized_fields({"age": "22"})] == ["age"] assert [ - name for name, _ in ie._get_serialized_fields({"age": "22", "name": "John"}) + name for name, _ in ie.get_serialized_fields({"age": "22", "name": "John"}) ] == ["age", "name"] def test_field_custom_serializer(self): @@ -142,7 +143,7 @@ class TestBaseItemExporter(ABC): class TestPythonItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return PythonItemExporter(**kwargs) def test_invalid_option(self): @@ -173,6 +174,7 @@ class TestPythonItemExporter(TestBaseItemExporter): "age": [{"age": [{"age": "22", "name": "Joseph"}], "name": "Maria"}], "name": "Jesus", } + assert exported is not None assert isinstance(exported["age"][0], dict) assert isinstance(exported["age"][0]["age"][0], dict) @@ -186,6 +188,7 @@ class TestPythonItemExporter(TestBaseItemExporter): "age": [{"age": [{"age": "22", "name": "Joseph"}], "name": "Maria"}], "name": "Jesus", } + assert exported is not None assert isinstance(exported["age"][0], dict) assert isinstance(exported["age"][0]["age"][0], dict) @@ -202,10 +205,10 @@ class TestPythonItemExporterDataclass(TestPythonItemExporter): class TestPprintItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return PprintItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: self._assert_expected_item(eval(self.output.getvalue())) @@ -215,10 +218,10 @@ class TestPprintItemExporterDataclass(TestPprintItemExporter): class TestPickleItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return PickleItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: self._assert_expected_item(pickle.loads(self.output.getvalue())) def test_export_multiple_items(self): @@ -252,10 +255,10 @@ class TestPickleItemExporterDataclass(TestPickleItemExporter): class TestMarshalItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return MarshalItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: self.output.seek(0) self._assert_expected_item(marshal.load(self.output)) @@ -279,7 +282,7 @@ class TestMarshalItemExporterDataclass(TestMarshalItemExporter): class TestCsvItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: # We need a fresh instance for each exporter, because # CsvItemExporter.stream.__del__() closes the underlying file # (CsvItemExporter.finish_exporting() calls detach() but not all tests @@ -287,8 +290,10 @@ class TestCsvItemExporter(TestBaseItemExporter): self.output = BytesIO() return CsvItemExporter(self.output, **kwargs) - def assertCsvEqual(self, first, second, msg=None): - def split_csv(csv): + def assertCsvEqual( + self, first: bytes | str, second: bytes | str, msg: str | None = None + ) -> None: + def split_csv(csv: bytes | str) -> list[list[str]]: return [ sorted(re.split(r"(,|\s+)", line)) for line in to_unicode(csv).splitlines(True) @@ -296,13 +301,15 @@ class TestCsvItemExporter(TestBaseItemExporter): assert split_csv(first) == split_csv(second), msg - def _check_output(self): + def _check_output(self) -> None: self.output.seek(0) self.assertCsvEqual( to_unicode(self.output.read()), "age,name\r\n22,John\xa3\r\n" ) - def assertExportResult(self, item, expected, **kwargs): + def assertExportResult( + self, item: Any, expected: bytes | str = b"", **kwargs: Any + ) -> None: fp = BytesIO() ie = CsvItemExporter(fp, **kwargs) ie.start_exporting() @@ -383,7 +390,6 @@ class TestCsvItemExporter(TestBaseItemExporter): with pytest.raises(UnicodeEncodeError): self.assertExportResult( item={"text": "W\u0275\u200brd"}, - expected=None, encoding="windows-1251", ) @@ -417,7 +423,7 @@ class TestCsvItemExporterDataclass(TestCsvItemExporter): class TestXmlItemExporter(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: # We need a fresh instance for each exporter, because # XmlItemExporter.stream.__del__() closes the underlying file # (XmlItemExporter.finish_exporting() calls detach() but not all tests @@ -425,20 +431,22 @@ class TestXmlItemExporter(TestBaseItemExporter): self.output = BytesIO() return XmlItemExporter(self.output, **kwargs) - def assertXmlEquivalent(self, first, second, msg=None): - def xmltuple(elem): + def assertXmlEquivalent( + self, first: bytes, second: bytes, msg: str | None = None + ) -> None: + def xmltuple(elem: Any) -> list[Any]: children = list(elem.iterchildren()) if children: return [(child.tag, sorted(xmltuple(child))) for child in children] return [(elem.tag, [(elem.text, ())])] - def xmlsplit(xmlcontent): + def xmlsplit(xmlcontent: bytes) -> list[Any]: doc = lxml.etree.fromstring(xmlcontent) return xmltuple(doc) assert xmlsplit(first) == xmlsplit(second), msg - def assertExportResult(self, item, expected_value): + def assertExportResult(self, item: Any, expected_value: bytes) -> None: fp = BytesIO() ie = XmlItemExporter(fp) ie.start_exporting() @@ -447,7 +455,7 @@ class TestXmlItemExporter(TestBaseItemExporter): del ie # See the first “del self.ie” in this file for context. self.assertXmlEquivalent(fp.getvalue(), expected_value) - def _check_output(self): + def _check_output(self) -> None: expected_value = ( b'\n' b"22John\xc2\xa3" @@ -538,10 +546,10 @@ class TestJsonLinesItemExporter(TestBaseItemExporter): "age": {"name": "Maria", "age": {"name": "Joseph", "age": "22"}}, } - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return JsonLinesItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: exported = json.loads(to_unicode(self.output.getvalue().strip())) assert exported == ItemAdapter(self.i).asdict() @@ -570,7 +578,7 @@ class TestJsonLinesItemExporter(TestBaseItemExporter): self.ie.finish_exporting() del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) - item["time"] = str(item["time"]) + item["time"] = item["time"].isoformat() assert exported == item @@ -582,14 +590,14 @@ class TestJsonLinesItemExporterDataclass(TestJsonLinesItemExporter): class TestJsonItemExporter(TestJsonLinesItemExporter): _expected_nested = [TestJsonLinesItemExporter._expected_nested] - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: return JsonItemExporter(self.output, **kwargs) - def _check_output(self): + def _check_output(self) -> None: exported = json.loads(to_unicode(self.output.getvalue().strip())) assert exported == [ItemAdapter(self.i).asdict()] - def assertTwoItemsExported(self, item): + def assertTwoItemsExported(self, item: Any) -> None: self.ie.start_exporting() self.ie.export_item(item) self.ie.export_item(item) @@ -653,12 +661,12 @@ class TestJsonItemExporter(TestJsonLinesItemExporter): self.ie.finish_exporting() del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) - item["time"] = str(item["time"]) + item["time"] = item["time"].isoformat() assert exported == [item] class TestJsonItemExporterToBytes(TestBaseItemExporter): - def _get_exporter(self, **kwargs): + def _get_exporter(self, **kwargs: Any) -> BaseItemExporter: kwargs["encoding"] = "latin" return JsonItemExporter(self.output, **kwargs) @@ -690,7 +698,9 @@ class TestCustomExporterItem: def test_exporter_custom_serializer(self): class CustomItemExporter(BaseItemExporter): - def serialize_field(self, field, name, value): + def serialize_field( + self, field: Mapping[str, Any] | Field, name: str, value: Any + ) -> Any: if name == "age": return str(int(value) + 1) return super().serialize_field(field, name, value) diff --git a/tests/test_extension_telnet.py b/tests/test_extension_telnet.py index fca0e3153..cf858e4ea 100644 --- a/tests/test_extension_telnet.py +++ b/tests/test_extension_telnet.py @@ -1,5 +1,6 @@ from __future__ import annotations +import socket from contextlib import contextmanager from typing import TYPE_CHECKING, Any @@ -91,6 +92,18 @@ def test_invalid_reversed_portrange() -> None: console.start_listening() +@coroutine_test +async def test_unavailable_port(caplog: pytest.LogCaptureFixture) -> None: + """Run a crawl where the console cannot bind any port.""" + with socket.create_server(("127.0.0.1", 0)) as sock: + port = sock.getsockname()[1] + crawler = _get_crawler(settings_dict={"TELNETCONSOLE_PORT": [port]}) + await crawler.crawl_async() + + assert "CannotListenError" in caplog.text + assert "AttributeError" not in caplog.text + + @coroutine_test async def test_telnet_vars() -> None: """Log into the console of a running crawl, which is when the telnet diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 40a763efd..d0f6a297c 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -100,6 +100,8 @@ class InstrumentedFeedSlot(FeedSlot): """Instrumented FeedSlot subclass for keeping track of calls to start_exporting and finish_exporting.""" + update_listener: Callable[[str], None] + def start_exporting(self): self.update_listener("start") super().start_exporting() @@ -109,7 +111,7 @@ class InstrumentedFeedSlot(FeedSlot): super().finish_exporting() @classmethod - def subscribe__listener(cls, listener): + def subscribe__listener(cls, listener: IsExportingListener) -> None: cls.update_listener = listener.update @@ -119,7 +121,7 @@ class IsExportingListener: finish_exporting and when a call to finish_exporting has been made before a call to start_exporting.""" - def __init__(self): + def __init__(self) -> None: self.start_without_finish = False self.finish_without_start = False @@ -307,6 +309,7 @@ class TestFeedExport(TestFeedExportBase): } crawler = get_crawler(ItemSpider, settings) yield crawler.crawl(mockserver=self.mockserver) + assert crawler.stats is not None assert "feedexport/success_count/FileFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/success_count/FileFeedStorage") == 1 @@ -330,6 +333,7 @@ class TestFeedExport(TestFeedExportBase): side_effect=store, ): yield crawler.crawl(mockserver=self.mockserver) + assert crawler.stats is not None assert "feedexport/failed_count/FileFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/failed_count/FileFeedStorage") == 1 @@ -347,6 +351,7 @@ class TestFeedExport(TestFeedExportBase): } crawler = get_crawler(ItemSpider, settings) yield crawler.crawl(mockserver=self.mockserver) + assert crawler.stats is not None assert "feedexport/success_count/FileFeedStorage" in crawler.stats.get_stats() assert "feedexport/success_count/StdoutFeedStorage" in crawler.stats.get_stats() assert crawler.stats.get_value("feedexport/success_count/FileFeedStorage") == 1 @@ -487,7 +492,7 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_start_finish_exporting_no_items(self): - items = [] + items: list[Any] = [] settings = { "FEEDS": { self._random_temp_filename(): {"format": "json"}, @@ -526,7 +531,7 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_start_finish_exporting_no_items_exception(self): - items = [] + items: list[Any] = [] settings = { "FEEDS": { self._random_temp_filename(): {"format": "json"}, @@ -611,7 +616,7 @@ class TestFeedExport(TestFeedExportBase): items = [{"foo": "bar"}] header = ["foo"] rows = [{"foo": "bar"}] - settings = {"FEED_EXPORT_FIELDS": []} + settings: dict[str, Any] = {"FEED_EXPORT_FIELDS": []} await self.assertExportedCsv(items, header, rows) await self.assertExportedJsonLines(items, rows, settings) @@ -727,14 +732,14 @@ class TestFeedExport(TestFeedExportBase): def accepts(self, item): return isinstance(item, MyItem) - class CustomFilter2(scrapy.extensions.feedexport.ItemFilter): + class CustomFilter2(ItemFilter): def accepts(self, item): return "foo" in item.fields - class CustomFilter3(scrapy.extensions.feedexport.ItemFilter): + class CustomFilter3(ItemFilter): def accepts(self, item): return ( - isinstance(item, tuple(self.item_classes)) and item["foo"] == "bar1" + isinstance(item, tuple(self.item_classes)) and item["foo"] == "bar1" # type: ignore[index] ) formats = { @@ -834,7 +839,7 @@ class TestFeedExport(TestFeedExportBase): } for fmt, expected in formats.items(): - settings = { + settings: dict[str, Any] = { "FEEDS": { self._random_temp_filename(): {"format": fmt}, }, @@ -911,7 +916,7 @@ class TestFeedExport(TestFeedExportBase): {"key": "value"}, ] - test_cases = [ + test_cases: list[dict[str, Any]] = [ # JSON { "format": "json", @@ -1132,7 +1137,7 @@ class TestFeedExport(TestFeedExportBase): expected_with_title_csv = b"foo,bar\r\nFOO,BAR\r\n" expected_without_title_csv = b"FOO,BAR\r\n" - test_cases = [ + test_cases: list[dict[str, Any]] = [ # with title { "options": { @@ -1166,6 +1171,9 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_storage_file_no_postprocessing(self): class Storage: + open_file: IO[bytes] + store_file: IO[bytes] + def __init__(self, uri, *, feed_options=None): pass @@ -1187,6 +1195,10 @@ class TestFeedExport(TestFeedExportBase): @coroutine_test async def test_storage_file_postprocessing(self): class Storage: + open_file: IO[bytes] + store_file: IO[bytes] + file_was_closed: bool + def __init__(self, uri, *, feed_options=None): pass @@ -1299,7 +1311,7 @@ class TestItemFilter: class TestFeedExportInit: def test_unsupported_storage(self): - settings = { + settings: dict[str, Any] = { "FEEDS": { "unsupported://uri": {}, }, diff --git a/tests/test_feedexport_postprocess.py b/tests/test_feedexport_postprocess.py index 36d8586ce..69696e65e 100644 --- a/tests/test_feedexport_postprocess.py +++ b/tests/test_feedexport_postprocess.py @@ -74,7 +74,13 @@ class TestFeedPostProcessedExports(TestFeedExportBase): return content - def get_gzip_compressed(self, data, compresslevel=9, mtime=0, filename=""): + def get_gzip_compressed( + self, + data: bytes, + compresslevel: int = 9, + mtime: int = 0, + filename: str = "", + ) -> bytes: data_stream = BytesIO() gzipf = gzip.GzipFile( fileobj=data_stream, @@ -539,11 +545,13 @@ class TestFeedPostProcessedExports(TestFeedExportBase): data = await self.exported_data(self.items, settings) - for filename, result in data.items(): + for filename, data_bytes in data.items(): + expected: Any + result: Any if "pickle" in filename: - expected, result = self.items[0], pickle.loads(result) + expected, result = self.items[0], pickle.loads(data_bytes) elif "marshal" in filename: - expected, result = self.items[0], marshal.loads(result) + expected, result = self.items[0], marshal.loads(data_bytes) else: - expected = filename_to_expected[filename] + expected, result = filename_to_expected[filename], data_bytes assert result == expected diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index 4d28872b7..23b39431e 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -7,6 +7,7 @@ import sys import tempfile from io import BytesIO from pathlib import Path +from ssl import SSLCertVerificationError from typing import IO, Any from unittest import mock from urllib.parse import quote @@ -95,27 +96,34 @@ class TestFileFeedStorage: assert storage.path == path +def get_test_spider(settings: dict[str, Any] | None = None) -> scrapy.Spider: + class TestSpider(scrapy.Spider): + name = "test_spider" + + crawler = get_crawler(settings_dict=settings) + return TestSpider.from_crawler(crawler) + + class TestFTPFeedStorage: - def get_test_spider(self, settings=None): - class TestSpider(scrapy.Spider): - name = "test_spider" - - crawler = get_crawler(settings_dict=settings) - return TestSpider.from_crawler(crawler) - - async def _store(self, uri, content, feed_options=None, settings=None): + async def _store( + self, + uri: str, + content: bytes, + feed_options: dict[str, Any] | None = None, + settings: dict[str, Any] | None = None, + ) -> None: crawler = get_crawler(settings_dict=settings or {}) storage = FTPFeedStorage.from_crawler( crawler, uri, feed_options=feed_options, ) - spider = self.get_test_spider() + spider = get_test_spider() file = storage.open(spider) file.write(content) await maybe_deferred_to_future(storage.store(file)) - def _assert_stored(self, path: Path, content): + def _assert_stored(self, path: Path, content: bytes) -> None: assert path.exists() try: assert path.read_bytes() == content @@ -162,10 +170,28 @@ class TestFTPFeedStorage: await self._store(url, b"bar", settings=settings) self._assert_stored(ftp_server.path / filename, b"bar") + @coroutine_test + async def test_tls(self, monkeypatch): + monkeypatch.setenv( + "SSL_CERT_FILE", str(Path(__file__).parent / "keys" / "localhost.crt") + ) + with MockFTPServer(tls=True) as ftp_server: + filename = "file" + await self._store(ftp_server.url(filename), b"foo") + self._assert_stored(ftp_server.path / filename, b"foo") + + @coroutine_test + async def test_tls_untrusted_certificate(self): + with ( + MockFTPServer(tls=True) as ftp_server, + pytest.raises(SSLCertVerificationError), + ): + await self._store(ftp_server.url("file"), b"foo") + def test_uri_auth_quote(self): # RFC3986: 3.2.1. User Information pw_quoted = quote(string.punctuation, safe="") - st = FTPFeedStorage(f"ftp://foo:{pw_quoted}@example.com/some_path", {}) + st = FTPFeedStorage(f"ftp://foo:{pw_quoted}@example.com/some_path") assert st.password == string.punctuation def test_uri_without_hostname(self): @@ -181,24 +207,17 @@ class MyBlockingFeedStorage(BlockingFeedStorage): class TestBlockingFeedStorage: - def get_test_spider(self, settings=None): - class TestSpider(scrapy.Spider): - name = "test_spider" - - crawler = get_crawler(settings_dict=settings) - return TestSpider.from_crawler(crawler) - def test_default_temp_dir(self): b = MyBlockingFeedStorage() - storage_file = b.open(self.get_test_spider()) + storage_file = b.open(get_test_spider()) storage_dir = Path(storage_file.name).parent assert str(storage_dir) == tempfile.gettempdir() def test_temp_file(self, tmp_path): b = MyBlockingFeedStorage() - spider = self.get_test_spider({"FEED_TEMPDIR": str(tmp_path)}) + spider = get_test_spider({"FEED_TEMPDIR": str(tmp_path)}) storage_file = b.open(spider) storage_dir = Path(storage_file.name).parent assert storage_dir == tmp_path @@ -207,7 +226,7 @@ class TestBlockingFeedStorage: b = MyBlockingFeedStorage() invalid_path = tmp_path / "invalid_path" - spider = self.get_test_spider({"FEED_TEMPDIR": str(invalid_path)}) + spider = get_test_spider({"FEED_TEMPDIR": str(invalid_path)}) with pytest.raises(OSError, match="Not a Directory:"): b.open(spider=spider) @@ -311,7 +330,7 @@ class TestS3FeedStorage: assert storage.access_key == "access_key" assert storage.secret_key == "secret_key" assert storage.region_name == region_name - assert storage.s3_client._client_config.region_name == region_name + assert storage.s3_client._client_config.region_name == region_name # type: ignore[attr-defined] def test_from_crawler_without_acl(self): settings = { @@ -353,7 +372,7 @@ class TestS3FeedStorage: ) assert storage.access_key == "access_key" assert storage.secret_key == "secret_key" - assert storage.s3_client._client_config.region_name == "us-east-1" + assert storage.s3_client._client_config.region_name == "us-east-1" # type: ignore[attr-defined] def test_from_crawler_with_acl(self): settings = { @@ -394,7 +413,7 @@ class TestS3FeedStorage: assert storage.access_key == "access_key" assert storage.secret_key == "secret_key" assert storage.region_name == region_name - assert storage.s3_client._client_config.region_name == region_name + assert storage.s3_client._client_config.region_name == region_name # type: ignore[attr-defined] def test_init_without_max_pool_connections(self) -> None: storage = S3FeedStorage("s3://mybucket/export.csv", "access_key", "secret_key") @@ -497,7 +516,7 @@ class TestGCSFeedStorage: def test_parse_empty_acl(self): pytest.importorskip("google.cloud.storage") - settings = {"GCS_PROJECT_ID": "123", "FEED_STORAGE_GCS_ACL": ""} + settings: dict[str, Any] = {"GCS_PROJECT_ID": "123", "FEED_STORAGE_GCS_ACL": ""} crawler = get_crawler(settings_dict=settings) storage = GCSFeedStorage.from_crawler(crawler, "gs://mybucket/export.csv") assert storage.acl is None @@ -524,7 +543,7 @@ class TestGCSFeedStorage: f.seek.assert_called_once_with(0) m.assert_called_once_with(project=project_id) - client_mock.get_bucket.assert_called_once_with("mybucket") + client_mock.bucket.assert_called_once_with("mybucket") bucket_mock.blob.assert_called_once_with("export.csv") blob_mock.upload_from_file.assert_called_once_with(f, predefined_acl=acl) f.close.assert_called_once_with() @@ -548,7 +567,7 @@ class TestGCSFeedStorage: f.seek.assert_called_once_with(0) m.assert_called_once_with(project=project_id) - client_mock.get_bucket.assert_called_once_with("mybucket") + client_mock.bucket.assert_called_once_with("mybucket") bucket_mock.blob.assert_called_once_with("export.csv") blob_mock.upload_from_file.assert_called_once_with(f, predefined_acl=acl) f.close.assert_called_once_with() diff --git a/tests/test_feedexport_uri_params.py b/tests/test_feedexport_uri_params.py index 150d8449f..21c291732 100644 --- a/tests/test_feedexport_uri_params.py +++ b/tests/test_feedexport_uri_params.py @@ -2,6 +2,7 @@ from __future__ import annotations import warnings from abc import ABC, abstractmethod +from typing import TYPE_CHECKING, Any import pytest @@ -10,16 +11,27 @@ from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.extensions.feedexport import FeedExporter from scrapy.utils.test import get_crawler +if TYPE_CHECKING: + from collections.abc import Callable + + from scrapy.crawler import Crawler + class TestURIParams(ABC): spider_name = "uri_params_spider" deprecated_options = False @abstractmethod - def build_settings(self, uri="file:///tmp/foobar", uri_params=None): + def build_settings( + self, + uri: str = "file:///tmp/foobar", + uri_params: Callable[..., dict[str, Any] | None] | None = None, + ) -> dict[str, Any]: raise NotImplementedError - def _crawler_feed_exporter(self, settings): + def _crawler_feed_exporter( + self, settings: dict[str, Any] + ) -> tuple[Crawler, FeedExporter]: if self.deprecated_options: with pytest.warns( ScrapyDeprecationWarning, @@ -29,6 +41,7 @@ class TestURIParams(ABC): else: crawler = get_crawler(settings_dict=settings) feed_exporter = crawler.get_extension(FeedExporter) + assert feed_exporter is not None return crawler, feed_exporter def test_default(self): @@ -116,8 +129,12 @@ class TestURIParams(ABC): class TestURIParamsSetting(TestURIParams): deprecated_options = True - def build_settings(self, uri="file:///tmp/foobar", uri_params=None): - extra_settings = {} + def build_settings( + self, + uri: str = "file:///tmp/foobar", + uri_params: Callable[..., dict[str, Any] | None] | None = None, + ) -> dict[str, Any]: + extra_settings: dict[str, Any] = {} if uri_params: extra_settings["FEED_URI_PARAMS"] = uri_params return { @@ -129,8 +146,12 @@ class TestURIParamsSetting(TestURIParams): class TestURIParamsFeedOption(TestURIParams): deprecated_options = False - def build_settings(self, uri="file:///tmp/foobar", uri_params=None): - options = { + def build_settings( + self, + uri: str = "file:///tmp/foobar", + uri_params: Callable[..., dict[str, Any] | None] | None = None, + ) -> dict[str, Any]: + options: dict[str, Any] = { "format": "jl", } if uri_params: diff --git a/tests/test_http2_client_protocol.py b/tests/test_http2_client_protocol.py index b8586d1ca..431b7a458 100644 --- a/tests/test_http2_client_protocol.py +++ b/tests/test_http2_client_protocol.py @@ -23,13 +23,13 @@ from twisted.web.static import File from scrapy.exceptions import DownloadCancelledError, DownloadTimeoutError from scrapy.http import JsonRequest, Request, Response -from scrapy.settings import Settings from scrapy.spiders import Spider from scrapy.utils.defer import ( deferred_f_from_coro_f, deferred_from_coro, maybe_deferred_to_future, ) +from scrapy.utils.test import get_crawler from tests.mockserver.http_resources import LeafResource, Status, put_child from tests.mockserver.utils import ssl_context_factory @@ -250,7 +250,7 @@ class TestHttps2ClientProtocol: acceptableProtocols=[b"h2"], ) uri = URI.fromBytes(bytes(self.get_url(server_port, "/"), "utf-8")) - h2_client_factory = H2ClientFactory(uri, Settings(), Deferred()) + h2_client_factory = H2ClientFactory(uri, get_crawler(), Deferred()) client_endpoint = SSL4ClientEndpoint( reactor, self.host, server_port, client_options ) diff --git a/tests/test_http_response_text.py b/tests/test_http_response_text.py index efa63e049..f705dbcee 100644 --- a/tests/test_http_response_text.py +++ b/tests/test_http_response_text.py @@ -481,6 +481,22 @@ class TestTextResponse(TestResponseBase): ): text_response.json() + def test_json_response_non_utf8(self): + response = self.response_class( + "http://www.example.com", + body='{"message": "café"}'.encode("cp1252"), + headers={"Content-Type": "application/json"}, + ) + assert response.json() == {"message": "café"} + + def test_json_response_wrong_charset(self): + response = self.response_class( + "http://www.example.com", + body='{"message": "café"}'.encode(), + headers={"Content-Type": "application/json; charset=iso-8859-1"}, + ) + assert response.json() == {"message": "café"} + def test_cache_json_response(self): json_valid_bodies = [b"""{"ip": "109.187.217.200"}""", b"""null"""] for json_body in json_valid_bodies: diff --git a/tests/test_item.py b/tests/test_item.py index 7b4c2e918..45732f157 100644 --- a/tests/test_item.py +++ b/tests/test_item.py @@ -1,4 +1,5 @@ from abc import ABCMeta +from typing import Any from unittest import mock import pytest @@ -7,9 +8,6 @@ from scrapy.item import Field, Item, ItemMeta class TestItem: - def assertSortedEqual(self, first, second, msg=None): - assert sorted(first) == sorted(second), msg - def test_simple(self): class TestItem(Item): name = Field() @@ -98,16 +96,16 @@ class TestItem: i = TestItem() with pytest.raises(AttributeError): - i.name = "john" + i.name = "john" # type: ignore[assignment] def test_custom_methods(self): class TestItem(Item): name = Field() - def get_name(self): + def get_name(self) -> Any: return self["name"] - def change_name(self, name): + def change_name(self, name: str) -> None: self["name"] = name i = TestItem() @@ -121,40 +119,40 @@ class TestItem: def test_metaclass(self): class TestItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] i = TestItem() i["name"] = "John" - assert list(i.keys()) == ["name"] - assert list(i.values()) == ["John"] + assert list(i.keys()) == ["name"] # type: ignore[operator] + assert list(i.values()) == ["John"] # type: ignore[operator] i["keys"] = "Keys" i["values"] = "Values" - self.assertSortedEqual(list(i.keys()), ["keys", "values", "name"]) - self.assertSortedEqual(list(i.values()), ["Keys", "Values", "John"]) + assert sorted(i.keys()) == ["keys", "name", "values"] # type: ignore[operator] + assert sorted(i.values()) == ["John", "Keys", "Values"] # type: ignore[operator] def test_metaclass_with_fields_attribute(self): class TestItem(Item): fields = {"new": Field(default="X")} item = TestItem(new="New") - self.assertSortedEqual(list(item.keys()), ["new"]) - self.assertSortedEqual(list(item.values()), ["New"]) + assert list(item.keys()) == ["new"] + assert list(item.values()) == ["New"] def test_fields_order(self): class TestItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] assert list(TestItem.fields) == ["name", "keys", "values"] def test_fields_order_inheritance(self): class ParentItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] class TestItem(ParentItem): extra = Field() @@ -169,16 +167,16 @@ class TestItem: def test_metaclass_inheritance(self): class ParentItem(Item): name = Field() - keys = Field() - values = Field() + keys = Field() # type: ignore[assignment] + values = Field() # type: ignore[assignment] class TestItem(ParentItem): keys = Field() i = TestItem() i["keys"] = 3 - assert list(i.keys()) == ["keys"] - assert list(i.values()) == [3] + assert list(i.keys()) == ["keys"] # type: ignore[operator] + assert list(i.values()) == [3] # type: ignore[operator] def test_metaclass_multiple_inheritance_simple(self): class A(Item): @@ -314,7 +312,7 @@ class TestItemMeta: def f(self): # For rationale of this see: # https://github.com/python/cpython/blob/ee1a81b77444c6715cbe610e951c655b6adab88b/Lib/test/test_super.py#L222 - return __class__ + return __class__ # type: ignore[name-defined] MyItem() diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 95a6aee54..7b73a133f 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -9,6 +9,7 @@ from w3lib import __version__ as w3lib_version from scrapy.http import HtmlResponse, XmlResponse from scrapy.link import Link +from scrapy.linkextractors import lxmlhtml from scrapy.linkextractors.lxmlhtml import LxmlLinkExtractor, LxmlParserLinkExtractor from tests import get_testdata @@ -798,6 +799,25 @@ class Base: class TestLxmlLinkExtractor(Base.TestLinkExtractorBase): extractor_cls = LxmlLinkExtractor + def test_canonicalize_once_per_link(self, monkeypatch): + canonicalize_url = lxmlhtml.canonicalize_url + calls = [] + + def counting_canonicalize_url(url, *args, **kwargs): + calls.append(url) + return canonicalize_url(url, *args, **kwargs) + + monkeypatch.setattr(lxmlhtml, "canonicalize_url", counting_canonicalize_url) + response = HtmlResponse( + "https://example.com", + body=b"".join(b'x' % i for i in range(10)), + ) + lx = self.extractor_cls(canonicalize=True) + assert lx.extract_links(response) == [ + Link(url="https://example.com/p?a=1&b=2", text="x") + ] + assert len(calls) == 10 + def test_link_restrict_text(self): html = b""" Pic of a cat diff --git a/tests/test_loader.py b/tests/test_loader.py index c094d25d8..969bf5b98 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -79,7 +79,7 @@ class TestBasicItemLoader: class InitializationTestMixin: - item_class: type | None = None + item_class: type def test_keep_single_value(self): """Loaded item should contain values from the initial item""" @@ -311,7 +311,7 @@ class TestSelectortemLoader: def test_init_method_with_base_response(self): """Selector should be None after initialization""" response = Response("https://scrapy.org") - l = ProcessorItemLoader(response=response) + l = ProcessorItemLoader(response=response) # type: ignore[arg-type] assert l.selector is None def test_init_method_with_response(self): @@ -461,6 +461,7 @@ class TestSubselectorLoader: l = NestedItemLoader(response=self.response) nl = l.nested_xpath("//header") + assert nl.selector is not None nl.add_xpath("name", "div/text()") nl.add_css("name_div", "#id") nl.add_value("name_value", nl.selector.xpath('div[@id = "id"]/text()').getall()) @@ -476,6 +477,7 @@ class TestSubselectorLoader: def test_nested_css(self): l = NestedItemLoader(response=self.response) nl = l.nested_css("header") + assert nl.selector is not None nl.add_xpath("name", "div/text()") nl.add_css("name_div", "#id") nl.add_value("name_value", nl.selector.xpath('div[@id = "id"]/text()').getall()) diff --git a/tests/test_pipeline_crawl.py b/tests/test_pipeline_crawl.py index 0681371ef..8b522255a 100644 --- a/tests/test_pipeline_crawl.py +++ b/tests/test_pipeline_crawl.py @@ -23,7 +23,10 @@ if TYPE_CHECKING: class MediaDownloadSpider(SimpleSpider): name = "mediadownload" - def _process_url(self, url): + media_key: str + media_urls_key: str + + def _process_url(self, url: str) -> str: return url def parse(self, response): @@ -44,14 +47,15 @@ class MediaDownloadSpider(SimpleSpider): class BrokenLinksMediaDownloadSpider(MediaDownloadSpider): name = "brokenmedia" - def _process_url(self, url): + def _process_url(self, url: str) -> str: return url + ".foo" class RedirectedMediaDownloadSpider(MediaDownloadSpider): name = "redirectedmedia" - def _process_url(self, url): + def _process_url(self, url: str) -> str: + assert self.mockserver return add_or_replace_parameter( self.mockserver.url("/redirect-to"), "goto", url ) diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index 4e7fb118b..f40619933 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -1,6 +1,7 @@ import base64 import dataclasses import logging +import mimetypes import random import re import time @@ -22,6 +23,7 @@ from itemadapter import ItemAdapter from twisted.internet.defer import Deferred from twisted.python.failure import Failure +from scrapy.crawler import Crawler from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Request, Response from scrapy.item import Field, Item @@ -75,7 +77,7 @@ class DeferredFSFilesStore(FSFilesStore): """A simple store with persist_file() returning a deferred.""" def persist_file(self, path, buf, info, meta=None, headers=None): - deferred = Deferred() + deferred: Deferred[None] = Deferred() # short-hand super() doesn't work in nested functions parent_persist_file = super().persist_file @@ -95,8 +97,14 @@ class TestFilesPipeline: def teardown_method(self): rmtree(self.tempdir) - def _create_pipeline(self, pipeline_cls: type[FilesPipeline]) -> FilesPipeline: - crawler = get_crawler(DefaultSpider, {"FILES_STORE": self.tempdir}) + def _create_pipeline( + self, + pipeline_cls: type[FilesPipeline], + settings: dict[str, Any] | None = None, + ) -> FilesPipeline: + crawler = get_crawler( + DefaultSpider, {"FILES_STORE": self.tempdir, **(settings or {})} + ) crawler.spider = crawler._create_spider() crawler.engine = MagicMock(download_async=mocked_download_func) pipeline = pipeline_cls.from_crawler(crawler) @@ -152,7 +160,7 @@ class TestFilesPipeline: file_path( Request("http://www.dorma.co.uk/images/product_details/2532"), response=Response("http://www.dorma.co.uk/images/product_details/2532"), - info=object(), + info=object(), # type: ignore[arg-type] ) == "full/244e0dd7d96a3b7b01f54eded250c9e272577aa1" ) @@ -383,7 +391,7 @@ class TestFilesPipeline: """ class CustomFilesPipeline(FilesPipeline): - def file_path(self, request, response=None, info=None, item=None): + def file_path(self, request, response=None, info=None, item=None) -> str: return f"full/{item.get('path')}" file_path = CustomFilesPipeline.from_crawler( @@ -393,6 +401,47 @@ class TestFilesPipeline: request = Request("http://example.com") assert file_path(request, item=item) == "full/path-to-store-file" + @coroutine_test + async def test_file_path_from_response(self) -> None: + """file_path() may build the path out of the response, e.g. to get the + file extension from a response header, as long as FILES_EXPIRES is 0 to + disable the up-to-date check, which runs before the download and hence + cannot reach the same path.""" + + class ContentTypeFilesPipeline(FilesPipeline): + def file_path(self, request, response=None, info=None, *, item=None): + path = super().file_path(request, response, info, item=item) + if response is None: + return path + content_type = response.headers["Content-Type"].decode() + return path + (mimetypes.guess_extension(content_type) or "") + + item_url = "http://example.com/download?id=1" + item = _create_item_with_files(item_url) + pipeline = self._create_pipeline(ContentTypeFilesPipeline, {"FILES_EXPIRES": 0}) + request = _prepare_request_object( + item_url, headers={"Content-Type": "application/pdf"} + ) + with ( + mock.patch.object(FilesPipeline, "inc_stats", return_value=True), + # A fresh file at the response-less path is ignored thanks to + # FILES_EXPIRES being 0. + mock.patch.object( + FSFilesStore, + "stat_file", + return_value={"checksum": "abc", "last_modified": time.time()}, + ), + mock.patch.object( + FilesPipeline, "get_media_requests", return_value=[request] + ), + ): + result = await pipeline.process_item(item) + + file_info = result["files"][0] + assert file_info["status"] == "downloaded" + assert file_info["path"].endswith(".pdf") + assert (Path(self.tempdir) / file_info["path"]).read_bytes() == b"data" + def test_media_failed_filtered_request( self, caplog: pytest.LogCaptureFixture ) -> None: @@ -476,7 +525,7 @@ class TestFilesPipeline: item["file_urls"] = bad_type with pytest.raises(TypeError, match="file_urls must be a list of URLs"): - list(pipeline.get_media_requests(item, None)) + list(pipeline.get_media_requests(item, None)) # type: ignore[arg-type] class TestFilesPipelineFieldsMixin(ABC): @@ -491,10 +540,10 @@ class TestFilesPipelineFieldsMixin(ABC): pipeline = FilesPipeline.from_crawler( get_crawler(None, {"FILES_STORE": tmp_path}) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, None)) # type: ignore[arg-type] assert requests[0].url == url results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + item = pipeline.item_completed(results, item, None) # type: ignore[arg-type] files = ItemAdapter(item).get("files") assert files == [results[0][1]] assert isinstance(item, self.item_class) @@ -512,10 +561,10 @@ class TestFilesPipelineFieldsMixin(ABC): }, ) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, None)) # type: ignore[arg-type] assert requests[0].url == url results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + item = pipeline.item_completed(results, item, None) # type: ignore[arg-type] custom_files = ItemAdapter(item).get("custom_files") assert custom_files == [results[0][1]] assert isinstance(item, self.item_class) @@ -581,8 +630,10 @@ class TestFilesPipelineCustomSettings: ("FILES_RESULT_FIELD", "FILES_RESULT_FIELD", "files_result_field"), } - def _generate_fake_settings(self, tmp_path, prefix=None): - def random_string(): + def _generate_fake_settings( + self, tmp_path: Path, prefix: str | None = None + ) -> dict[str, Any]: + def random_string() -> str: return "".join([chr(random.randint(97, 123)) for _ in range(10)]) settings = { @@ -599,7 +650,7 @@ class TestFilesPipelineCustomSettings: for k, v in settings.items() } - def _generate_fake_pipeline(self): + def _generate_fake_pipeline(self) -> type[FilesPipeline]: class UserDefinedFilePipeline(FilesPipeline): EXPIRES = 1001 FILES_URLS_FIELD = "alfa" @@ -739,14 +790,14 @@ class TestFilesPipelineCustomSettings: def test_file_pipeline_using_pathlike_objects(self, tmp_path): class CustomFilesPipelineWithPathLikeDir(FilesPipeline): - def file_path(self, request, response=None, info=None, *, item=None): - return Path("subdir") / Path(request.url).name + def file_path(self, request, response=None, info=None, *, item=None) -> str: + return str(Path("subdir") / Path(request.url).name) pipeline = CustomFilesPipelineWithPathLikeDir.from_crawler( get_crawler(None, {"FILES_STORE": tmp_path}) ) request = Request("http://example.com/image01.jpg") - assert pipeline.file_path(request) == Path("subdir/image01.jpg") + assert pipeline.file_path(request) == str(Path("subdir/image01.jpg")) class TestFSFilesStore: @@ -1092,7 +1143,7 @@ class TestFTPFileStore: store.port, store.username, store.password, - store.USE_ACTIVE_MODE, + bool(store.USE_ACTIVE_MODE), ) assert data == content @@ -1130,10 +1181,18 @@ def _create_item_with_files(*files: str) -> ItemWithFiles: return item -def _prepare_request_object(item_url: str, flags: list[str] | None = None) -> Request: +def _prepare_request_object( + item_url: str, + flags: list[str] | None = None, + headers: dict[str, str] | None = None, +) -> Request: return Request( item_url, - meta={"response": Response(item_url, status=200, body=b"data", flags=flags)}, + meta={ + "response": Response( + item_url, status=200, body=b"data", flags=flags, headers=headers + ) + }, ) @@ -1160,7 +1219,7 @@ class TestBuildFromCrawler: _from_crawler_called = False @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> "Pipeline": settings = crawler.settings store_uri = settings["FILES_STORE"] o = cls(store_uri, crawler=crawler) @@ -1179,7 +1238,7 @@ def test_files_pipeline_raises_notconfigured_when_files_store_invalid(store): settings = Settings() settings.clear() settings.set("FILES_STORE", store, priority="cmdline") - crawler = get_crawler(settings_dict=settings) + crawler = get_crawler(settings_dict=dict(settings)) with pytest.raises(NotConfigured): FilesPipeline.from_crawler(crawler) diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py index 19e61579f..43316f125 100644 --- a/tests/test_pipeline_images.py +++ b/tests/test_pipeline_images.py @@ -91,7 +91,7 @@ class TestImagesPipeline: file_path( Request("http://www.dorma.co.uk/images/product_details/2532"), response=Response("http://www.dorma.co.uk/images/product_details/2532"), - info=object(), + info=DUMMY_SPIDER_INFO, ) == "full/244e0dd7d96a3b7b01f54eded250c9e272577aa1.jpg" ) @@ -120,7 +120,7 @@ class TestImagesPipeline: Request("file:///tmp/some.name/foo"), name, response=Response("file:///tmp/some.name/foo"), - info=object(), + info=DUMMY_SPIDER_INFO, ) == "thumbs/50/850233df65a5b83361798f532f1fc549cd13cbe9.jpg" ) @@ -133,7 +133,7 @@ class TestImagesPipeline: class CustomImagesPipeline(ImagesPipeline): def thumb_path( self, request, thumb_id, response=None, info=None, item=None - ): + ) -> str: return f"thumb/{thumb_id}/{item.get('path')}" thumb_path = CustomImagesPipeline.from_crawler( @@ -159,11 +159,23 @@ class TestImagesPipeline: req = Request(url="https://dev.mydeco.com/mydeco.gif") with pytest.raises(ImageException): - next(self.pipeline.get_images(response=resp1, request=req, info=object())) + next( + self.pipeline.get_images( + response=resp1, request=req, info=DUMMY_SPIDER_INFO + ) + ) with pytest.raises(ImageException): - next(self.pipeline.get_images(response=resp2, request=req, info=object())) + next( + self.pipeline.get_images( + response=resp2, request=req, info=DUMMY_SPIDER_INFO + ) + ) with pytest.raises(ImageException): - next(self.pipeline.get_images(response=resp3, request=req, info=object())) + next( + self.pipeline.get_images( + response=resp3, request=req, info=DUMMY_SPIDER_INFO + ) + ) def test_get_images(self): self.pipeline.min_width = 0 @@ -176,7 +188,7 @@ class TestImagesPipeline: req = Request(url="https://dev.mydeco.com/mydeco.gif") get_images_gen = self.pipeline.get_images( - response=resp, request=req, info=object() + response=resp, request=req, info=DUMMY_SPIDER_INFO ) path, new_im, new_buf = next(get_images_gen) @@ -201,7 +213,7 @@ class TestImagesPipeline: req = Request(url="https://dev.mydeco.com/mydeco.gif") get_images_gen = self.pipeline.get_images( - response=resp, request=req, info=object() + response=resp, request=req, info=DUMMY_SPIDER_INFO ) path, new_im, _ = next(get_images_gen) @@ -230,7 +242,7 @@ class TestImagesPipeline: def test_convert_image(self): SIZE = (100, 100) # straight forward case: RGB and JPEG - COLOUR = (0, 127, 255) + COLOUR: tuple[int, ...] = (0, 127, 255) im, buf = _create_image("JPEG", "RGB", SIZE, COLOUR) converted, converted_buf = self.pipeline.convert_image(im, response_body=buf) assert converted.mode == "RGB" @@ -296,7 +308,7 @@ class TestImagesPipeline: item["image_urls"] = bad_type with pytest.raises(TypeError, match="image_urls must be a list of URLs"): - list(pipeline.get_media_requests(item, None)) + list(pipeline.get_media_requests(item, DUMMY_SPIDER_INFO)) class TestImagesPipelineFieldsMixin(ABC): @@ -311,10 +323,10 @@ class TestImagesPipelineFieldsMixin(ABC): pipeline = ImagesPipeline.from_crawler( get_crawler(None, {"IMAGES_STORE": "s3://example/images/"}) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, DUMMY_SPIDER_INFO)) assert requests[0].url == url - results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + results: Any = [(True, {"url": url})] + item = pipeline.item_completed(results, item, DUMMY_SPIDER_INFO) images = ItemAdapter(item).get("images") assert images == [results[0][1]] assert isinstance(item, self.item_class) @@ -332,10 +344,10 @@ class TestImagesPipelineFieldsMixin(ABC): }, ) ) - requests = list(pipeline.get_media_requests(item, None)) + requests = list(pipeline.get_media_requests(item, DUMMY_SPIDER_INFO)) assert requests[0].url == url - results = [(True, {"url": url})] - item = pipeline.item_completed(results, item, None) + results: Any = [(True, {"url": url})] + item = pipeline.item_completed(results, item, DUMMY_SPIDER_INFO) custom_images = ItemAdapter(item).get("custom_images") assert custom_images == [results[0][1]] assert isinstance(item, self.item_class) @@ -410,13 +422,15 @@ class TestImagesPipelineCustomSettings: "IMAGES_RESULT_FIELD": "images", } - def _generate_fake_settings(self, tmp_path, prefix=None): + def _generate_fake_settings( + self, tmp_path: Path, prefix: str | None = None + ) -> dict[str, Any]: """ :param prefix: string for setting keys :return: dictionary of image pipeline settings """ - def random_string(): + def random_string() -> str: return "".join([chr(random.randint(97, 123)) for _ in range(10)]) settings = { @@ -439,7 +453,7 @@ class TestImagesPipelineCustomSettings: for k, v in settings.items() } - def _generate_fake_pipeline_subclass(self): + def _generate_fake_pipeline_subclass(self) -> type[ImagesPipeline]: """ :return: ImagePipeline class will all uppercase attributes set. """ diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index ba1c18006..50787d7b7 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from typing import TYPE_CHECKING, Any, cast from unittest.mock import MagicMock import pytest @@ -10,7 +11,12 @@ from scrapy import signals from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.pipelines.files import FileException -from scrapy.pipelines.media import MediaPipeline, _MediaRequestFiltered +from scrapy.pipelines.media import ( + FileInfo, + FileInfoOrError, + MediaPipeline, + _MediaRequestFiltered, +) from scrapy.utils.defer import _defer_sleep_async from scrapy.utils.log import failure_to_exc_info from scrapy.utils.signal import disconnect_all @@ -19,21 +25,48 @@ from scrapy.utils.test import get_crawler from tests.utils.decorators import coroutine_test from tests.utils.media_pipelines import mocked_download_func +if TYPE_CHECKING: + from collections.abc import Awaitable + + from twisted.internet.defer import Deferred + + from scrapy.crawler import Crawler + class UserDefinedPipeline(MediaPipeline): - def media_to_download(self, request, info, *, item=None): - pass + def media_to_download( + self, request: Request, info: MediaPipeline.SpiderInfo, *, item: Any = None + ) -> Deferred[FileInfo | None] | None: + return None - def get_media_requests(self, item, info): - pass + def get_media_requests( + self, item: Any, info: MediaPipeline.SpiderInfo + ) -> list[Request]: + return [] - def media_downloaded(self, response, request, info, *, item=None): - return {} + def media_downloaded( + self, + response: Response, + request: Request, + info: MediaPipeline.SpiderInfo, + *, + item: Any = None, + ) -> FileInfo | Awaitable[FileInfo]: + return cast("FileInfo", {}) - def media_failed(self, failure, request, info): + def media_failed( + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> Failure: failure.raiseException() - def file_path(self, request, response=None, info=None, *, item=None): + def file_path( + self, + request: Request, + response: Response | None = None, + info: MediaPipeline.SpiderInfo | None = None, + *, + item: Any = None, + ) -> str: return "" @@ -48,8 +81,14 @@ class TestBaseMediaPipeline: self.pipe = self.pipeline_class.from_crawler(crawler) self.pipe.open_spider() self.info = self.pipe.spiderinfo + assert crawler.request_fingerprinter is not None self.fingerprint = crawler.request_fingerprinter.fingerprint + @property + def mocked_pipe(self) -> MockedMediaPipeline: + assert isinstance(self.pipe, MockedMediaPipeline) + return self.pipe + def teardown_method(self): for name, signal in vars(signals).items(): if not name.startswith("_"): @@ -121,11 +160,13 @@ class TestBaseMediaPipeline: # When calling the method that caches the Request's result ... self.pipe._cache_result_and_execute_waiters(failure, fp, info) # ... it should store the Twisted Failure ... - assert info.downloaded[fp] == failure + downloaded = info.downloaded[fp] + assert downloaded == failure # ... encapsulating the original FileException ... - assert info.downloaded[fp].value == file_exc + assert isinstance(downloaded, Failure) + assert downloaded.value == file_exc # ... but it should not store the StopIteration exception on its context - context = getattr(info.downloaded[fp].value, "__context__", None) + context = getattr(downloaded.value, "__context__", None) assert context is None def test_default_item_completed(self, caplog: pytest.LogCaptureFixture) -> None: @@ -134,7 +175,7 @@ class TestBaseMediaPipeline: # Check that failures are logged by default fail = Failure(Exception()) - results = [(True, 1), (False, fail)] + results: Any = [(True, 1), (False, fail)] caplog.clear() new_item = self.pipe.item_completed(results, item, self.info) @@ -158,7 +199,7 @@ class TestBaseMediaPipeline: by item_completed(), as they are not download errors.""" item = {"name": "name"} fail = Failure(_MediaRequestFiltered("Filtered offsite request")) - results = [(True, 1), (False, fail)] + results: Any = [(True, 1), (False, fail)] with caplog.at_level(logging.DEBUG): new_item = self.pipe.item_completed(results, item, self.info) @@ -174,29 +215,44 @@ class TestBaseMediaPipeline: class MockedMediaPipeline(UserDefinedPipeline): - def __init__(self, *args, crawler=None, **kwargs): + def __init__(self, *args: Any, crawler: Crawler, **kwargs: Any): super().__init__(*args, crawler=crawler, **kwargs) - self._mockcalled = [] + self._mockcalled: list[str] = [] - def media_to_download(self, request, info, *, item=None): + def media_to_download( + self, request: Request, info: MediaPipeline.SpiderInfo, *, item: Any = None + ) -> Deferred[FileInfo | None] | None: self._mockcalled.append("media_to_download") if "result" in request.meta: return request.meta.get("result") return super().media_to_download(request, info) - def get_media_requests(self, item, info): + def get_media_requests( + self, item: Any, info: MediaPipeline.SpiderInfo + ) -> list[Request]: self._mockcalled.append("get_media_requests") - return item.get("requests") + return item.get("requests") # type: ignore[no-any-return] - def media_downloaded(self, response, request, info, *, item=None): + def media_downloaded( + self, + response: Response, + request: Request, + info: MediaPipeline.SpiderInfo, + *, + item: Any = None, + ) -> FileInfo | Awaitable[FileInfo]: self._mockcalled.append("media_downloaded") return super().media_downloaded(response, request, info) - def media_failed(self, failure, request, info): + def media_failed( + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> Failure: self._mockcalled.append("media_failed") return super().media_failed(failure, request, info) - def item_completed(self, results, item, info): + def item_completed( + self, results: list[FileInfoOrError], item: Any, info: MediaPipeline.SpiderInfo + ) -> Any: self._mockcalled.append("item_completed") item = super().item_completed(results, item, info) item["results"] = results @@ -204,7 +260,14 @@ class MockedMediaPipeline(UserDefinedPipeline): class AsyncMediaDownloadedPipeline(MockedMediaPipeline): - async def media_downloaded(self, response, request, info, *, item=None): + async def media_downloaded( # type: ignore[override] + self, + response: Response, + request: Request, + info: MediaPipeline.SpiderInfo, + *, + item: Any = None, + ) -> FileInfo | Awaitable[FileInfo]: return super().media_downloaded(response, request, info) @@ -212,7 +275,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): pipeline_class = MockedMediaPipeline def _errback(self, result): - self.pipe._mockcalled.append("request_errback") + self.mocked_pipe._mockcalled.append("request_errback") return result @coroutine_test @@ -226,7 +289,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): item = {"requests": req} new_item = await self.pipe.process_item(item) assert new_item["results"] == [(True, {})] - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_downloaded", @@ -248,7 +311,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): assert new_item["results"][0][0] is False assert isinstance(new_item["results"][0][1], Failure) assert new_item["results"][0][1].value == exc - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_failed", @@ -270,7 +333,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): assert new_item["results"][1][0] is False assert isinstance(new_item["results"][1][1], Failure) assert new_item["results"][1][1].value == exc - m = self.pipe._mockcalled + m = self.mocked_pipe._mockcalled # only once assert m[0] == "get_media_requests" # first hook called assert m.count("get_media_requests") == 1 @@ -294,7 +357,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): # returns iterable of Requests req1 = Request("http://url1") req2 = Request("http://url2") - item = {"requests": iter([req1, req2])} + item = {"requests": iter([req1, req2])} # type: ignore[dict-item] new_item = await self.pipe.process_item(item) assert new_item is item assert self.fingerprint(req1) in self.info.downloaded @@ -304,7 +367,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): async def test_results_are_cached_across_multiple_items(self): rsp1 = Response("http://url1") req1 = Request("http://url1", meta={"response": rsp1}) - item = {"requests": req1} + item: dict[str, Any] = {"requests": req1} new_item = await self.pipe.process_item(item) assert new_item is item assert new_item["results"] == [(True, {})] @@ -335,7 +398,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): new_item = await self.pipe.process_item({"requests": req2}) assert new_item["results"][0][0] is False assert new_item["results"][0][1].value is exc - assert self.pipe._mockcalled.count("media_to_download") == 1 + assert self.mocked_pipe._mockcalled.count("media_to_download") == 1 @coroutine_test async def test_cached_failure_calls_errback(self): @@ -347,13 +410,13 @@ class TestMediaPipeline(TestBaseMediaPipeline): ) def errback(failure): - self.pipe._mockcalled.append("request_errback") + self.mocked_pipe._mockcalled.append("request_errback") return {"recovered": failure.value} req = Request("http://url1", errback=errback) new_item = await self.pipe.process_item({"requests": req}) assert new_item["results"] == [(True, {"recovered": exc})] - assert self.pipe._mockcalled.count("request_errback") == 1 + assert self.mocked_pipe._mockcalled.count("request_errback") == 1 @coroutine_test async def test_results_are_cached_for_requests_of_single_item(self): @@ -362,14 +425,14 @@ class TestMediaPipeline(TestBaseMediaPipeline): req2 = Request( req1.url, meta={"response": Response("http://donot.download.me")} ) - item = {"requests": [req1, req2]} + item: dict[str, Any] = {"requests": [req1, req2]} new_item = await self.pipe.process_item(item) assert new_item is item assert new_item["results"] == [(True, {}), (True, {})] @coroutine_test async def test_wait_if_request_is_downloading(self): - def _check_downloading(response): + def _check_downloading(response: Response) -> Response: fp = self.fingerprint(req1) assert fp in self.info.downloading assert fp in self.info.waiting @@ -398,7 +461,7 @@ class TestMediaPipeline(TestBaseMediaPipeline): item = {"requests": req} new_item = await self.pipe.process_item(item) assert new_item["results"] == [(True, "ITSME")] - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "item_completed", @@ -422,7 +485,9 @@ class TestAsyncMediaDownloaded(TestMediaPipeline): class TestMediaPipelineAllowRedirectSettings: - def _assert_request_no3xx(self, pipeline_class, settings): + def _assert_request_no3xx( + self, pipeline_class: type[MediaPipeline], settings: dict[str, Any] + ) -> None: pipe = pipeline_class(crawler=get_crawler(None, settings)) request = Request("http://url") pipe._modify_media_request(request) @@ -477,7 +542,7 @@ class TestBuildFromCrawler: self._init_called = True @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> Pipeline: settings = crawler.settings store_uri = settings["FILES_STORE"] o = cls(store_uri, settings=settings, crawler=crawler) @@ -493,9 +558,10 @@ class TestBuildFromCrawler: def test_has_from_crawler(self): class Pipeline(UserDefinedPipeline): _from_crawler_called = False + store_uri: str @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> Pipeline: settings = crawler.settings o = super().from_crawler(crawler) o._from_crawler_called = True @@ -509,7 +575,9 @@ class TestBuildFromCrawler: class MediaFailedNonePipeline(MockedMediaPipeline): - def media_failed(self, failure, request, info): + def media_failed( # type: ignore[override] + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> None: self._mockcalled.append("media_failed") @@ -524,7 +592,7 @@ class TestMediaFailedNone(TestBaseMediaPipeline): req = Request("http://url1", meta={"response": Exception("foo")}) new_item = await self.pipe.process_item({"requests": req}) assert new_item["results"] == [(True, None)] - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_failed", @@ -533,7 +601,9 @@ class TestMediaFailedNone(TestBaseMediaPipeline): class MediaFailedFailurePipeline(MockedMediaPipeline): - def media_failed(self, failure, request, info): + def media_failed( + self, failure: Failure, request: Request, info: MediaPipeline.SpiderInfo + ) -> Failure: self._mockcalled.append("media_failed") return failure # deprecated @@ -544,7 +614,7 @@ class TestMediaFailedFailure(TestBaseMediaPipeline): pipeline_class = MediaFailedFailurePipeline def _errback(self, result): - self.pipe._mockcalled.append("request_errback") + self.mocked_pipe._mockcalled.append("request_errback") return result @coroutine_test @@ -565,7 +635,7 @@ class TestMediaFailedFailure(TestBaseMediaPipeline): assert new_item["results"][0][0] is False assert isinstance(new_item["results"][0][1], Failure) assert new_item["results"][0][1].value == exc - assert self.pipe._mockcalled == [ + assert self.mocked_pipe._mockcalled == [ "get_media_requests", "media_to_download", "media_failed", diff --git a/tests/test_pipelines.py b/tests/test_pipelines.py index e65b5bc32..4b34946ae 100644 --- a/tests/test_pipelines.py +++ b/tests/test_pipelines.py @@ -47,7 +47,7 @@ class DeferredPipeline: return succeed(None) def process_item(self, item): - d = Deferred() + d: Deferred[Any] = Deferred() d.addCallback(self.cb) d.callback(item) return d @@ -55,7 +55,7 @@ class DeferredPipeline: class AsyncDefPipeline: async def process_item(self, item): - d = Deferred() + d: Deferred[Any] = Deferred() call_later(0, d.callback, None) await maybe_deferred_to_future(d) item["pipeline_passed"] = True @@ -64,7 +64,7 @@ class AsyncDefPipeline: class AsyncDefAsyncioPipeline: async def process_item(self, item): - d = Deferred() + d: Deferred[Any] = Deferred() loop = asyncio.get_event_loop() loop.call_later(0, d.callback, None) await deferred_to_future(d) @@ -75,12 +75,12 @@ class AsyncDefAsyncioPipeline: class AsyncDefNotAsyncioPipeline: async def process_item(self, item): - d1 = Deferred() + d1: Deferred[Any] = Deferred() from twisted.internet import reactor reactor.callLater(0, d1.callback, None) await d1 - d2 = Deferred() + d2: Deferred[Any] = Deferred() reactor.callLater(0, d2.callback, None) await maybe_deferred_to_future(d2) item["pipeline_passed"] = True @@ -120,6 +120,8 @@ class OpenSpiderExceptionAsyncPipeline: class ItemSpider(Spider): name = "itemspider" + mockserver: MockServer + async def start(self): yield Request(self.mockserver.url("/status?n=200")) diff --git a/tests/test_pqueues.py b/tests/test_pqueues.py index 85fefd172..6ecbb0721 100644 --- a/tests/test_pqueues.py +++ b/tests/test_pqueues.py @@ -6,7 +6,7 @@ import queuelib from scrapy.core.downloader import Downloader from scrapy.http.request import Request -from scrapy.pqueues import DownloaderAwarePriorityQueue, ScrapyPriorityQueue +from scrapy.pqueues import DownloaderAwarePriorityQueue, ScrapyPriorityQueue, _path_safe from scrapy.spiders import Spider from scrapy.squeues import FifoMemoryQueue, PickleFifoDiskQueue from scrapy.utils.misc import build_from_crawler, load_object @@ -258,6 +258,29 @@ class TestDownloaderAwarePriorityQueue: assert "other-slot" not in self.queue +def test_slot_directory_removed_when_slot_drains(tmp_path): + crawler = get_crawler(Spider) + crawler.spider = crawler._create_spider("foo") + crawler.engine = Mock(downloader=MockDownloader()) + queue = DownloaderAwarePriorityQueue.from_crawler( + crawler=crawler, + downstream_queue_cls=PickleFifoDiskQueue, + key=str(tmp_path), + ) + request = Request("https://example.org/1") + slot_dir = tmp_path / _path_safe("example.org") + + queue.push(request) + assert slot_dir.is_dir() + + assert queue.pop().url == request.url + assert not slot_dir.exists() + + queue.push(request) + assert slot_dir.is_dir() + queue.close() + + @pytest.mark.parametrize( ("input_", "output"), [ diff --git a/tests/test_spidermiddleware_depth.py b/tests/test_spidermiddleware_depth.py index 32a2ea8f2..bdacd7287 100644 --- a/tests/test_spidermiddleware_depth.py +++ b/tests/test_spidermiddleware_depth.py @@ -60,6 +60,19 @@ def test_process_spider_output(mw: DepthMiddleware, stats: StatsCollector) -> No assert rdm == 1 +def test_depth_reset(mw: DepthMiddleware, stats: StatsCollector) -> None: + resp = Response("https://example.com") + resp.request = Request("https://example.com", meta={"depth": 5}) + result = [Request("https://example.com", meta={"depth_reset": True})] + + out = list(mw.process_spider_output(resp, result)) + + assert out == result + assert out[0].meta["depth"] == 0 + assert "depth_reset" not in out[0].meta + assert stats.get_value("request_depth_count/0") == 1 + + def test_process_spider_output_no_response( mw: DepthMiddleware, stats: StatsCollector ) -> None: @@ -85,6 +98,23 @@ async def test_process_spider_output_async_no_response( assert stats.get_value("request_depth_count/0") is None +def test_ignored_logged_once( + mw: DepthMiddleware, stats: StatsCollector, caplog: pytest.LogCaptureFixture +) -> None: + resp = Response("http://example.com") + resp.request = Request("http://example.com") + resp.meta["depth"] = 1 + result = [Request(f"http://example.com/{i}") for i in range(3)] + + with caplog.at_level("DEBUG", logger="scrapy.spidermiddlewares.depth"): + assert not list(mw.process_spider_output(resp, result)) + + messages = [r.getMessage() for r in caplog.records] + assert len(messages) == 1 + assert "http://example.com/0" in messages[0] + assert stats.get_value("depth/request_ignored_count") == 3 + + def test_priority_and_non_verbose_stats() -> None: crawler = get_crawler( Spider, diff --git a/tests/test_utils_console.py b/tests/test_utils_console.py index 0dea8af6d..fe6a7a97a 100644 --- a/tests/test_utils_console.py +++ b/tests/test_utils_console.py @@ -122,6 +122,11 @@ class TestIPythonShell: @pytest.mark.skipif( sys.platform == "win32", reason="requires a POSIX pseudo-terminal" ) + # The child of the pseudo-terminal fork execs right away, so the deadlocks + # that Python warns about cannot happen. + @pytest.mark.filterwarnings( + "ignore:.*is multi-threaded, use of forkpty:DeprecationWarning" + ) @pytest.mark.parametrize( "script", [CONSOLE, CONSOLE_IN_RUNNING_LOOP], diff --git a/tests/test_utils_log.py b/tests/test_utils_log.py index 7f5301387..42b2b95fd 100644 --- a/tests/test_utils_log.py +++ b/tests/test_utils_log.py @@ -5,7 +5,7 @@ import logging import re import sys from io import StringIO -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast import pytest from twisted.python.failure import Failure @@ -16,6 +16,7 @@ from scrapy.utils.log import ( StreamLogger, TopLevelFormatter, failure_to_exc_info, + logformatter_adapter, ) from scrapy.utils.test import get_crawler from tests.spiders import LogSpider @@ -24,6 +25,7 @@ if TYPE_CHECKING: from collections.abc import Generator, Mapping, MutableMapping from scrapy.crawler import Crawler + from scrapy.logformatter import LogFormatterResult class TestFailureToExcInfo: @@ -311,3 +313,36 @@ class TestLoggingWithExtra: assert log_contents["message"] == log_message assert self.regex_pattern.match(log_contents["spider"]) assert log_contents["important_info"] == extra["important_info"] + + +class TestLogformatterAdapter: + @staticmethod + def _log(caplog: pytest.LogCaptureFixture, logkws: LogFormatterResult) -> str: + with caplog.at_level(logging.INFO): + logging.getLogger(__name__).log(*logformatter_adapter(logkws)) + return caplog.records[-1].getMessage() + + @pytest.mark.parametrize("args", [None, {}, ()]) + def test_empty_args( + self, + caplog: pytest.LogCaptureFixture, + args: dict[str, Any] | tuple[Any, ...] | None, + ) -> None: + logkws = cast( + "LogFormatterResult", + {"level": logging.INFO, "msg": "90% done", "args": args}, + ) + assert self._log(caplog, logkws) == "90% done" + + @pytest.mark.parametrize( + ("msg", "args"), + [("%(pct)d%% done", {"pct": 90}), ("%d%% done", (90,))], + ) + def test_args( + self, + caplog: pytest.LogCaptureFixture, + msg: str, + args: dict[str, Any] | tuple[Any, ...], + ) -> None: + logkws: LogFormatterResult = {"level": logging.INFO, "msg": msg, "args": args} + assert self._log(caplog, logkws) == "90% done" diff --git a/tests/test_utils_serialize.py b/tests/test_utils_serialize.py index 2702c2cce..6becad13e 100644 --- a/tests/test_utils_serialize.py +++ b/tests/test_utils_serialize.py @@ -20,11 +20,17 @@ class TestJsonEncoder: def test_encode_decode(self, encoder: ScrapyJSONEncoder) -> None: dt = datetime.datetime(2010, 1, 2, 10, 11, 12) - dts = "2010-01-02 10:11:12" + dts = "2010-01-02T10:11:12" + dt_aware = datetime.datetime( + 2010, 1, 2, 10, 11, 12, 133700, tzinfo=datetime.timezone.utc + ) + dt_awares = "2010-01-02T10:11:12.133700+00:00" d = datetime.date(2010, 1, 2) ds = "2010-01-02" t = datetime.time(10, 11, 12) ts = "10:11:12" + t_us = datetime.time(10, 11, 12, 133700) + t_uss = "10:11:12.133700" dec = Decimal("1000.12") decs = "1000.12" s = {"foo"} @@ -36,7 +42,9 @@ class TestJsonEncoder: ("foo", "foo"), (d, ds), (t, ts), + (t_us, t_uss), (dt, dts), + (dt_aware, dt_awares), (dec, decs), (["foo", d], ["foo", ds]), (s, ss), diff --git a/tests/test_utils_trackref.py b/tests/test_utils_trackref.py index 5458aa603..9585c6d83 100644 --- a/tests/test_utils_trackref.py +++ b/tests/test_utils_trackref.py @@ -124,3 +124,13 @@ def test_iter_all(): o2 = Bar() # noqa: F841 o3 = Foo() assert set(trackref.iter_all("Foo")) == {o1, o3} + + +def test_run_time_classes() -> None: + for _ in range(10): + base = type("Baz", (trackref.object_ref,), {}) + base() + del base + garbage_collect() + assert not list(trackref.iter_all("Baz")) + assert sum(1 for cls in trackref.live_refs if cls.__name__ == "Baz") == 0 diff --git a/tests/utils/bases/download_handlers_http.py b/tests/utils/bases/download_handlers_http.py index e44f9bcb8..9b4a38724 100644 --- a/tests/utils/bases/download_handlers_http.py +++ b/tests/utils/bases/download_handlers_http.py @@ -11,7 +11,7 @@ from contextlib import asynccontextmanager from http import HTTPStatus from ipaddress import IPv4Address from socket import gethostbyname -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Literal from urllib.parse import urlparse import pytest @@ -60,6 +60,9 @@ if TYPE_CHECKING: from tests.mockserver.http import MockServer +BadHeaderHandling = Literal["skip-bad", "skip-rest", "fail"] + + class TestHttpBase(ABC): is_secure: bool = False http2: bool = False @@ -72,6 +75,14 @@ class TestHttpBase(ABC): # h2.connection.H2Connection.receive_data()), thus closing all streams that # were using it, and we handle this as a normal exception. handler_supports_http2_dataloss: bool = True + # What the handler does with a bad response header line, e.g. one with no + # colon in it: + # "skip-bad": the bad line is skipped and the header lines that follow it + # are still parsed, which is what web browsers do; + # "skip-rest": the bad line is skipped along with the header lines that + # follow it; + # "fail": the response cannot be downloaded at all. + handler_bad_header_handling: BadHeaderHandling = "skip-bad" # default headers added by the underlying library that cannot be suppressed always_present_req_headers: ClassVar[frozenset[str]] = frozenset() default_handler_settings: ClassVar[dict[str, Any]] = {} @@ -628,6 +639,7 @@ class TestHttpBase(ABC): "Expected to receive 5 bytes which is larger than download warn size (4)" in caplog.text ) + assert caplog.text.count("download warn size (4)") == 1 @coroutine_test async def test_download_with_warnsize_no_content_length( @@ -644,6 +656,32 @@ class TestHttpBase(ABC): in caplog.text ) + @coroutine_test + async def test_download_bad_header(self, mockserver: MockServer) -> None: + if self.http2: + pytest.skip("Header lines are specific to HTTP/1.x") + request = Request(mockserver.url("/bad-header", is_secure=self.is_secure)) + async with self.get_dh() as download_handler: + if self.handler_bad_header_handling == "fail": + with pytest.raises(DownloadFailedError): + await download_handler.download_request(request) + return + response = await download_handler.download_request(request) + assert response.status == 200 + assert response.body == b"Works" + # the header line that precedes the bad one + assert response.headers.get(b"Content-Type") == b"text/html" + # the header split into two lines, also before the bad one + folded_header = response.headers.get(b"X-Folded-Header") + assert folded_header is not None + # the separator between both parts depends on the handler + assert folded_header.split() == [b"one", b"two"] + # the header line that follows the bad one + expected_value = ( + b"works" if self.handler_bad_header_handling == "skip-bad" else None + ) + assert response.headers.get(b"X-After-Bad-Header") == expected_value + @coroutine_test async def test_download_chunked_content(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/chunked", is_secure=self.is_secure)) diff --git a/tests/utils/cloud.py b/tests/utils/cloud.py index 662e0b3ee..4e253fbdc 100644 --- a/tests/utils/cloud.py +++ b/tests/utils/cloud.py @@ -14,7 +14,6 @@ def mock_google_cloud_storage() -> tuple[Any, Any, Any]: bucket_mock = mock.create_autospec(Bucket) client_mock.bucket.return_value = bucket_mock - client_mock.get_bucket.return_value = bucket_mock blob_mock = mock.create_autospec(Blob) bucket_mock.blob.return_value = blob_mock diff --git a/tox.ini b/tox.ini index ac32064a6..097562289 100644 --- a/tox.ini +++ b/tox.ini @@ -44,6 +44,7 @@ deps = pygments pytest pytest-cov >= 7.0.0 + pytest-timeout pytest-xdist sybil >= 1.3.0 # https://github.com/cjw296/sybil/issues/20#issuecomment-605433422 pytest-twisted >= 1.14.3 @@ -136,6 +137,8 @@ deps = pytest==8.4.0 Protego==0.1.15 Twisted==21.7.0 + brotli==1.2.0; implementation_name != "pypy" + brotlicffi==1.2.0.0; implementation_name == "pypy" cryptography==37.0.0 cssselect==0.9.1 httpx2==2.0.0 @@ -143,7 +146,7 @@ deps = lxml==4.6.4 parsel==1.5.0 pyOpenSSL==22.0.0 - queuelib==1.4.2 + queuelib==1.6.1 service_identity==23.1.0 w3lib==1.17.0 zope.interface==5.1.0 @@ -170,8 +173,6 @@ deps = Twisted[http2] boto3 bpython # optional for shell wrapper tests - brotli >= 1.2.0; implementation_name != "pypy" # optional for HTTP compress downloader middleware tests - brotlicffi >= 1.2.0.0; implementation_name == "pypy" # optional for HTTP compress downloader middleware tests google-cloud-storage httpx2[http2,socks] ipython @@ -188,8 +189,6 @@ deps = Twisted[http2]==21.7.0 boto3==1.20.0 bpython==0.7.1 - brotli==1.2.0; implementation_name != "pypy" - brotlicffi==1.2.0.0; implementation_name == "pypy" google-cloud-storage==1.29.0 httpx2[http2,socks]==2.0.0 ipython==8.15.0 @@ -201,6 +200,51 @@ setenv = {[min]setenv} commands = {[min]commands} +[testenv:vcs-deps] +basepython = python3 +deps = + {[testenv:extra-deps]deps} + uv +# Dependencies cap each other at their latest release, so their development +# branches usually cannot be resolved together: pyOpenSSL, for one, requires a +# cryptography older than the one cryptography itself is heading towards. +# --no-deps skips resolution entirely, replacing only these distributions and +# leaving the rest of the environment as the install above resolved it. +# +# Pillow and uvloop build from source, and need the libjpeg headers and +# autotools respectively. robotexclusionrulesparser has no public repository, +# and PyDispatcher has seen no commit since 2023-10-23, so they stay at their +# latest release. +commands_pre = + uv pip install --python {envpython} --no-deps --reinstall \ + git+https://github.com/twisted/twisted \ + git+https://github.com/python-pillow/Pillow \ + git+https://github.com/MagicStack/uvloop \ + git+https://github.com/pyca/cryptography \ + git+https://github.com/scrapy/cssselect \ + git+https://github.com/tiran/defusedxml \ + git+https://github.com/scrapy/itemadapter \ + git+https://github.com/scrapy/itemloaders \ + git+https://github.com/lxml/lxml \ + git+https://github.com/pypa/packaging \ + git+https://github.com/scrapy/parsel \ + git+https://github.com/scrapy/protego \ + git+https://github.com/pyca/pyopenssl \ + git+https://github.com/scrapy/queuelib \ + git+https://github.com/pyca/service-identity \ + git+https://github.com/john-kurkowski/tldextract \ + git+https://github.com/scrapy/w3lib \ + git+https://github.com/zopefoundation/zope.interface \ + git+https://github.com/boto/boto3 \ + git+https://github.com/bpython/bpython \ + git+https://github.com/google/brotli \ + git+https://github.com/python-hyper/brotlicffi \ + git+https://github.com/googleapis/python-storage \ + git+https://github.com/pydantic/httpx2\#subdirectory=src/httpx2 \ + git+https://github.com/ipython/ipython \ + git+https://github.com/prompt-toolkit/ptpython \ + git+https://github.com/indygreg/python-zstandard + [testenv:default-reactor] commands = {[testenv]commands} --reactor=default @@ -245,7 +289,6 @@ commands = basepython = pypy3 deps = {[testenv:extra-deps]deps} -commands = {[testenv:pypy3]commands} [testenv:min-pypy3] basepython = pypy3.11 @@ -255,13 +298,14 @@ deps = pytest==8.4.0 Protego==0.1.15 Twisted==21.7.0 + brotlicffi==1.2.0.0 cryptography==44.0.2 cssselect==0.9.1 itemadapter==0.1.0 lxml==5.3.2 parsel==1.5.0 pyOpenSSL==24.3.0 - queuelib==1.4.2 + queuelib==1.6.1 service_identity==23.1.0 # w3lib 1.17 fails to import on PyPy 3.11 because its encoding regex uses # an inline flag placement that Python 3.11 treats as an error: global