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-vcs-deps.yml b/.github/workflows/tests-vcs-deps.yml new file mode 100644 index 000000000..bf867dba7 --- /dev/null +++ b/.github/workflows/tests-vcs-deps.yml @@ -0,0 +1,53 @@ +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 + env: + PYTEST_ADDOPTS: -n auto --no-cov + 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..1b41adaad 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -141,6 +141,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/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/topics/broad-crawls.rst b/docs/topics/broad-crawls.rst index cace1f883..d6b9fd6f9 100644 --- a/docs/topics/broad-crawls.rst +++ b/docs/topics/broad-crawls.rst @@ -182,8 +182,9 @@ 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 +BFO order `, :ref:`lowering concurrency +` and :ref:`delaying start request iteration +` you should :ref:`debug your memory leaks `. diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index f6842f0ff..ead0e6582 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/download-handlers.rst b/docs/topics/download-handlers.rst index 34ab4f105..94e75ab6f 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: @@ -221,9 +203,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 diff --git a/docs/topics/item-pipeline.rst b/docs/topics/item-pipeline.rst index 951c0f485..35891ce8e 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -49,7 +49,8 @@ Additionally, they may also implement the following methods: .. 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 +331,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/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..a177d1ad5 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -770,6 +770,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 +1432,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/settings.rst b/docs/topics/settings.rst index 65ee77258..ec222d999 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -658,6 +658,11 @@ The default headers used for Scrapy HTTP Requests. They're populated in the :class:`Request.cookies ` parameter. This is a known current limitation that is being worked on. +.. 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 DEPTH_LIMIT @@ -749,6 +754,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 @@ -2328,6 +2338,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..f060710a4 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 `. diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index aa14f6801..db85906c1 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 ` diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 8fbf0c52d..e68c208b7 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,10 +411,6 @@ 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`). - .. _builtin-spiders: Generic Spiders diff --git a/docs/topics/stats.rst b/docs/topics/stats.rst index c702cefe7..b558c1cf2 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`` diff --git a/pyproject.toml b/pyproject.toml index 13267e427..0dcbade90 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", 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/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/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/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py index db85b62a1..28c0e09cb 100644 --- a/scrapy/downloadermiddlewares/offsite.py +++ b/scrapy/downloadermiddlewares/offsite.py @@ -22,10 +22,12 @@ logger = logging.getLogger(__name__) class OffsiteMiddleware: 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: @@ -37,7 +39,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,6 +72,7 @@ class OffsiteMiddleware: raise IgnoreRequest(f"Filtered offsite request to {domain!r}") def should_follow(self, request: Request, spider: Spider) -> bool: + 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 "" diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 5564669ea..554134696 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -392,7 +392,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) return bucket.blob(self.blob_name) def _store_in_thread(self, file: IO[bytes]) -> None: diff --git a/scrapy/extensions/telnet.py b/scrapy/extensions/telnet.py index 3be24c53f..1506cb1ea 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: 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/text.py b/scrapy/http/response/text.py index d01e23e47..64251780a 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 diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 3fb741d7a..46ac39a28 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. diff --git a/scrapy/pqueues.py b/scrapy/pqueues.py index 41411ceaf..8e9783f5a 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 @@ -409,6 +411,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/spidermiddlewares/depth.py b/scrapy/spidermiddlewares/depth.py index 054804119..0131b62e7 100644 --- a/scrapy/spidermiddlewares/depth.py +++ b/scrapy/spidermiddlewares/depth.py @@ -41,6 +41,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: @@ -94,11 +95,14 @@ class DepthMiddleware(BaseSpiderMiddleware): 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/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/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_crawler_subprocess.py b/tests/test_crawler_subprocess.py index 733b6797d..240482586 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( 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_offsite.py b/tests/test_downloadermiddleware_offsite.py index cb17c2553..bab89814f 100644 --- a/tests/test_downloadermiddleware_offsite.py +++ b/tests/test_downloadermiddleware_offsite.py @@ -247,3 +247,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_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_storages.py b/tests/test_feedexport_storages.py index 0132d4f98..4a51fc6b6 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -634,7 +634,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() @@ -658,7 +658,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_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_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..2aa76195e 100644 --- a/tests/test_spidermiddleware_depth.py +++ b/tests/test_spidermiddleware_depth.py @@ -85,6 +85,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/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..94c04f75f 100644 --- a/tox.ini +++ b/tox.ini @@ -143,7 +143,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 @@ -201,6 +201,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, +# so it stays at its 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/mcfletch/pydispatcher \ + 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 @@ -261,7 +306,7 @@ deps = 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