Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle ConnectionError on HTTP HEAD Requests #9309

Merged
merged 17 commits into from Jun 13, 2021
Merged

Conversation

jamathews
Copy link
Contributor

Subject: Resolves #9306

Feature or Bugfix

  • Bugfix

Purpose

Detail

  • The US Patent and Trademark website seems to close connections on HTTP HEAD requests, but will return the requested document for HTTP GET requests. The linkcheck.py code wasn't catching the ConnectionError exception, and as a result the code would skip the requests.get() attempt. With this fix, if requests.head() raises a ConnectionError, it will attempt a requests.get() which, for the UT Patent website, succeeds. If the website truly is rejecting all connections, an exception on the requests.get() call will still send control to the except Exception as err block as before.

Relates

Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the nice report and patch! 👍
I agree Sphinx could be more tolerant here.


Please add a note to the CHANGES.

sphinx/builders/linkcheck.py Outdated Show resolved Hide resolved
Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! A few minor changes and this should be good to go.

CHANGES Outdated Show resolved Hide resolved
sphinx/builders/linkcheck.py Outdated Show resolved Hide resolved
sphinx/builders/linkcheck.py Outdated Show resolved Hide resolved
sphinx/builders/linkcheck.py Outdated Show resolved Hide resolved
tests/test_build_linkcheck.py Outdated Show resolved Hide resolved
tests/test_build_linkcheck.py Outdated Show resolved Hide resolved
tests/test_build_linkcheck.py Outdated Show resolved Hide resolved
tests/test_build_linkcheck.py Show resolved Hide resolved
jamathews and others added 9 commits June 10, 2021 11:39
Co-authored-by: François Freitag <mail@franek.fr>
Co-authored-by: François Freitag <mail@franek.fr>
Co-authored-by: François Freitag <mail@franek.fr>
Co-authored-by: François Freitag <mail@franek.fr>
Co-authored-by: François Freitag <mail@franek.fr>
Co-authored-by: François Freitag <mail@franek.fr>
Co-authored-by: François Freitag <mail@franek.fr>
Co-authored-by: François Freitag <mail@franek.fr>
Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me, thanks!

CHANGES Outdated Show resolved Hide resolved
Co-authored-by: François Freitag <mail@franek.fr>
@tk0miya tk0miya added this to the 4.1.0 milestone Jun 13, 2021
@tk0miya tk0miya changed the base branch from 4.0.x to 4.x June 13, 2021 08:16
@tk0miya
Copy link
Member

tk0miya commented Jun 13, 2021

Thank you for your contribution. +1 for merging this into 4.x branch instead of 4.0.x. Usually, we use 4.0.x branch only for severe or critical bug fixes. So this is better to merge this into 4.x branch.

@tk0miya tk0miya merged commit a5eefc0 into sphinx-doc:4.x Jun 13, 2021
@tk0miya
Copy link
Member

tk0miya commented Jun 13, 2021

Merged. Thank you for your contribution!

@francoisfreitag Thank you for reviewing :-)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linkcheck Reports Broken Link when Remote Server Closes on HEAD Request
3 participants