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

Improve standalone errors #8919

Merged
merged 2 commits into from Jun 21, 2021
Merged

Conversation

alexzorin
Copy link
Collaborator

There's two changes here:

  1. Add an authenticator hint for the standalone plugin, which we forgot to add. It looks like this: image
  2. Restores this functionality in the standalone plugin which is supposed to provide friendly error messages for EACCES and EADDRINUSE. It seems that this functionality was accidentally lost during a change to acme.standalone a long time ago. We go from seeing a very generic error message in master: image to much more actionable: image and: image

I suggest merging this PR instead of squashing it.

Fixes #8887.

Copy link
Contributor

@ohemorange ohemorange left a comment

Choose a reason for hiding this comment

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

Wow, very nice catches! One minor suggestion to clarify the explanation, because otherwise "server" could mean the webserver or the machine as a whole.

neat_addr = f"{addr}:{port}" if addr else f"port {port}"
return ("The Certificate Authority failed to download the challenge files from "
f"the temporary standalone webserver started by Certbot on {neat_addr}. "
"Ensure the listed domains point to this server and that this server can accept "
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Ensure the listed domains point to this server and that this server can accept "
"Ensure the listed domains point to this server and that this machine can accept "

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the preceding "server" refers to the same thing. Would something like this be more correct:

Ensure that the listed domains point to this machine and that it can accept inbound connections from the internet

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yes, true. That looks great!

@ohemorange ohemorange merged commit 62426ca into certbot:master Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement an auth_hint for the standalone plugin
2 participants