mirror of https://github.com/scrapy/scrapy.git
Fix omitting repeated dataloss warnings in HTTP11DownloadHandler. (#7222)
This commit is contained in:
parent
2347138ba4
commit
3ec6ae05c1
|
|
@ -93,6 +93,7 @@ class HTTP11DownloadHandler(BaseDownloadHandler):
|
||||||
"DOWNLOAD_FAIL_ON_DATALOSS"
|
"DOWNLOAD_FAIL_ON_DATALOSS"
|
||||||
)
|
)
|
||||||
self._disconnect_timeout: int = 1
|
self._disconnect_timeout: int = 1
|
||||||
|
self._fail_on_dataloss_warned: bool = False
|
||||||
|
|
||||||
async def download_request(self, request: Request) -> Response:
|
async def download_request(self, request: Request) -> Response:
|
||||||
"""Return a deferred for the HTTP download"""
|
"""Return a deferred for the HTTP download"""
|
||||||
|
|
@ -115,8 +116,19 @@ class HTTP11DownloadHandler(BaseDownloadHandler):
|
||||||
fail_on_dataloss=self._fail_on_dataloss,
|
fail_on_dataloss=self._fail_on_dataloss,
|
||||||
crawler=self._crawler,
|
crawler=self._crawler,
|
||||||
)
|
)
|
||||||
with wrap_twisted_exceptions():
|
try:
|
||||||
return await maybe_deferred_to_future(agent.download_request(request))
|
with wrap_twisted_exceptions():
|
||||||
|
return await maybe_deferred_to_future(agent.download_request(request))
|
||||||
|
except ResponseDataLossError:
|
||||||
|
if not self._fail_on_dataloss_warned:
|
||||||
|
logger.warning(
|
||||||
|
"Got data loss in %s. If you want to process broken "
|
||||||
|
"responses set the setting DOWNLOAD_FAIL_ON_DATALOSS = False"
|
||||||
|
" -- This message won't be shown in further requests",
|
||||||
|
request.url,
|
||||||
|
)
|
||||||
|
self._fail_on_dataloss_warned = True
|
||||||
|
raise
|
||||||
|
|
||||||
async def close(self) -> None:
|
async def close(self) -> None:
|
||||||
from twisted.internet import reactor
|
from twisted.internet import reactor
|
||||||
|
|
@ -633,7 +645,6 @@ class _ResponseReader(Protocol):
|
||||||
self._maxsize: int = maxsize
|
self._maxsize: int = maxsize
|
||||||
self._warnsize: int = warnsize
|
self._warnsize: int = warnsize
|
||||||
self._fail_on_dataloss: bool = fail_on_dataloss
|
self._fail_on_dataloss: bool = fail_on_dataloss
|
||||||
self._fail_on_dataloss_warned: bool = False
|
|
||||||
self._reached_warnsize: bool = False
|
self._reached_warnsize: bool = False
|
||||||
self._bytes_received: int = 0
|
self._bytes_received: int = 0
|
||||||
self._certificate: ssl.Certificate | None = None
|
self._certificate: ssl.Certificate | None = None
|
||||||
|
|
@ -738,15 +749,6 @@ class _ResponseReader(Protocol):
|
||||||
self._finish_response(flags=["dataloss"])
|
self._finish_response(flags=["dataloss"])
|
||||||
return
|
return
|
||||||
|
|
||||||
if not self._fail_on_dataloss_warned:
|
|
||||||
logger.warning(
|
|
||||||
"Got data loss in %s. If you want to process broken "
|
|
||||||
"responses set the setting DOWNLOAD_FAIL_ON_DATALOSS = False"
|
|
||||||
" -- This message won't be shown in further requests",
|
|
||||||
self._txresponse.request.absoluteURI.decode(),
|
|
||||||
)
|
|
||||||
self._fail_on_dataloss_warned = True
|
|
||||||
|
|
||||||
exc = ResponseDataLossError()
|
exc = ResponseDataLossError()
|
||||||
exc.__cause__ = reason.value
|
exc.__cause__ = reason.value
|
||||||
reason = Failure(exc)
|
reason = Failure(exc)
|
||||||
|
|
|
||||||
|
|
@ -86,6 +86,9 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base):
|
||||||
def test_download_cause_data_loss(self) -> None: # type: ignore[override]
|
def test_download_cause_data_loss(self) -> None: # type: ignore[override]
|
||||||
pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON)
|
pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON)
|
||||||
|
|
||||||
|
def test_download_cause_data_loss_double_warning(self) -> None: # type: ignore[override]
|
||||||
|
pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON)
|
||||||
|
|
||||||
def test_download_allow_data_loss(self) -> None: # type: ignore[override]
|
def test_download_allow_data_loss(self) -> None: # type: ignore[override]
|
||||||
pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON)
|
pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -516,6 +516,21 @@ class TestHttp11Base(TestHttpBase):
|
||||||
with pytest.raises(ResponseDataLossError):
|
with pytest.raises(ResponseDataLossError):
|
||||||
await download_handler.download_request(request)
|
await download_handler.download_request(request)
|
||||||
|
|
||||||
|
@deferred_f_from_coro_f
|
||||||
|
async def test_download_cause_data_loss_double_warning(
|
||||||
|
self, caplog: pytest.LogCaptureFixture, mockserver: MockServer
|
||||||
|
) -> None:
|
||||||
|
request = Request(mockserver.url("/broken", is_secure=self.is_secure))
|
||||||
|
async with self.get_dh() as download_handler:
|
||||||
|
with pytest.raises(ResponseDataLossError):
|
||||||
|
await download_handler.download_request(request)
|
||||||
|
assert "Got data loss" in caplog.text
|
||||||
|
caplog.clear()
|
||||||
|
with pytest.raises(ResponseDataLossError):
|
||||||
|
await download_handler.download_request(request)
|
||||||
|
# no repeated warning
|
||||||
|
assert "Got data loss" not in caplog.text
|
||||||
|
|
||||||
@pytest.mark.parametrize("url", ["broken", "broken-chunked"])
|
@pytest.mark.parametrize("url", ["broken", "broken-chunked"])
|
||||||
@deferred_f_from_coro_f
|
@deferred_f_from_coro_f
|
||||||
async def test_download_allow_data_loss(
|
async def test_download_allow_data_loss(
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue