Merge pull request #8919 from alexzorin/standalone-error-ux

Improve standalone errors

acme/acme/standalone.py

@@ -8,6 +8,7 @@ import socket
 import socketserver
 import threading
 from typing import List
+from typing import Optional
 
 from acme import challenges
 from acme import crypto_util
@@ -66,6 +67,9 @@ class BaseDualNetworkedServers:
         self.threads: List[threading.Thread] = []
         self.servers: List[socketserver.BaseServer] = []
 
+        # Preserve socket error for re-raising, if no servers can be started
+        last_socket_err: Optional[socket.error] = None
+
         # Must try True first.
         # Ubuntu, for example, will fail to bind to IPv4 if we've already bound
         # to IPv6. But that's ok, since it will accept IPv4 connections on the IPv6
@@ -82,7 +86,8 @@ class BaseDualNetworkedServers:
                 logger.debug(
                     "Successfully bound to %s:%s using %s", new_address[0],
                     new_address[1], "IPv6" if ip_version else "IPv4")
-            except socket.error:
+            except socket.error as e:
+                last_socket_err = e
                 if self.servers:
                     # Already bound using IPv6.
                     logger.debug(
@@ -101,7 +106,10 @@ class BaseDualNetworkedServers:
                 # bind to the same port for both servers.
                 port = server.socket.getsockname()[1]
         if not self.servers:
-            raise socket.error("Could not bind to IPv4 or IPv6.")
+            if last_socket_err:
+                raise last_socket_err
+            else: # pragma: no cover
+                raise socket.error("Could not bind to IPv4 or IPv6.")
 
     def serve_forever(self):
         """Wraps socketserver.TCPServer.serve_forever"""

acme/tests/standalone_test.py

@@ -190,12 +190,18 @@ class BaseDualNetworkedServersTest(unittest.TestCase):
 
     @mock.patch("socket.socket.bind")
     def test_fail_to_bind(self, mock_bind):
-        mock_bind.side_effect = socket.error
+        from errno import EADDRINUSE
         from acme.standalone import BaseDualNetworkedServers
-        self.assertRaises(socket.error, BaseDualNetworkedServers,
-                          BaseDualNetworkedServersTest.SingleProtocolServer,
-                          ('', 0),
-                          socketserver.BaseRequestHandler)
+
+        mock_bind.side_effect = socket.error(EADDRINUSE, "Fake addr in use error")
+
+        with self.assertRaises(socket.error) as em:
+            BaseDualNetworkedServers(
+                BaseDualNetworkedServersTest.SingleProtocolServer,
+                ('', 0), socketserver.BaseRequestHandler)
+
+        self.assertEqual(em.exception.errno, EADDRINUSE)
+
 
     def test_ports_equal(self):
         from acme.standalone import BaseDualNetworkedServers

certbot/certbot/_internal/plugins/standalone.py

@@ -5,6 +5,7 @@ import logging
 import socket
 from typing import DefaultDict
 from typing import Dict
+from typing import List
 from typing import Set
 from typing import Tuple
 from typing import TYPE_CHECKING
@@ -184,6 +185,14 @@ class Authenticator(common.Plugin):
             if not self.served[servers]:
                 self.servers.stop(port)
 
+    def auth_hint(self, failed_achalls: List[achallenges.AnnotatedChallenge]) -> str:
+        port, addr = self.config.http01_port, self.config.http01_address
+        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 that the listed domains point to this machine and that it can "
+                "accept inbound connections from the internet.")
+
 
 def _handle_perform_error(error):
     if error.socket_error.errno == errno.EACCES:

certbot/tests/plugins/standalone_test.py

@@ -177,6 +177,13 @@ class AuthenticatorTest(unittest.TestCase):
             "server1": set(), "server2": set()})
         self.auth.servers.stop.assert_called_with(2)
 
+    def test_auth_hint(self):
+        self.config.http01_port = "80"
+        self.config.http01_address = None
+        self.assertIn("on port 80", self.auth.auth_hint([]))
+        self.config.http01_address = "127.0.0.1"
+        self.assertIn("on 127.0.0.1:80", self.auth.auth_hint([]))
+
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover