diff --git a/acme/client.py b/acme/client.py index f5a651652..4979bce8e 100644 --- a/acme/client.py +++ b/acme/client.py @@ -77,6 +77,9 @@ class Client(object): # pylint: disable=too-many-instance-attributes :raises .ClientError: In case of other networking errors. """ + logging.debug('Received response %s (headers: %s): %r', + response, response.headers, response.content) + response_ct = response.headers.get('Content-Type') try: # TODO: response.json() is called twice, once here, and @@ -92,8 +95,6 @@ class Client(object): # pylint: disable=too-many-instance-attributes 'Ignoring wrong Content-Type (%r) for JSON Error', response_ct) try: - logging.error("Error: %s", jobj) - logging.error("Response from server: %s", response.content) raise messages.Error.from_json(jobj) except jose.DeserializationError as error: # Couldn't deserialize JSON object @@ -169,7 +170,6 @@ class Client(object): # pylint: disable=too-many-instance-attributes response = requests.post(uri, data=data, **kwargs) except requests.exceptions.RequestException as error: raise errors.ClientError(error) - logging.debug('Received response %s: %r', response, response.text) self._add_nonce(response) self._check_response(response, content_type=content_type) diff --git a/acme/jose/json_util.py b/acme/jose/json_util.py index a08145459..f38ebc62f 100644 --- a/acme/jose/json_util.py +++ b/acme/jose/json_util.py @@ -218,11 +218,12 @@ class JSONObjectWithFields(util.ImmutableMap, interfaces.JSONDeSerializable): def fields_to_partial_json(self): """Serialize fields to JSON.""" jobj = {} + omitted = set() for slot, field in self._fields.iteritems(): value = getattr(self, slot) if field.omit(value): - logging.debug('Omitting empty field "%s" (%s)', slot, value) + omitted.add((slot, value)) else: try: jobj[field.json_name] = field.encode(value) @@ -230,6 +231,10 @@ class JSONObjectWithFields(util.ImmutableMap, interfaces.JSONDeSerializable): raise errors.SerializationError( 'Could not encode {0} ({1}): {2}'.format( slot, value, error)) + if omitted: + # pylint: disable=star-args + logging.debug('Omitted empty fields: %s', ', '.join( + '{0!s}={1!r}'.format(*field) for field in omitted)) return jobj def to_partial_json(self): diff --git a/letsencrypt/account.py b/letsencrypt/account.py index 9f351387f..a97e07504 100644 --- a/letsencrypt/account.py +++ b/letsencrypt/account.py @@ -227,5 +227,5 @@ class Account(object): if cls.EMAIL_REGEX.match(email): return not email.startswith(".") and ".." not in email else: - logging.warn("Invalid email address.") + logging.warn("Invalid email address: %s.", email) return False diff --git a/letsencrypt/augeas_configurator.py b/letsencrypt/augeas_configurator.py index c59d755c2..a375b2e17 100644 --- a/letsencrypt/augeas_configurator.py +++ b/letsencrypt/augeas_configurator.py @@ -52,10 +52,10 @@ class AugeasConfigurator(common.Plugin): lens_path = self.aug.get(path + "/lens") # As aug.get may return null if lens_path and lens in lens_path: - # Strip off /augeas/files and /error - logging.error("There has been an error in parsing the file: %s", - path[13:len(path) - 6]) - logging.error(self.aug.get(path + "/message")) + logging.error( + "There has been an error in parsing the file (%s): %s", + # Strip off /augeas/files and /error + path[13:len(path) - 6], self.aug.get(path + "/message")) def save(self, title=None, temporary=False): """Saves all changes to the configuration files. @@ -122,13 +122,10 @@ class AugeasConfigurator(common.Plugin): # Check for the root of save problems new_errs = self.aug.match("/augeas//error") # logging.error("During Save - %s", mod_conf) - # Only print new errors caused by recent save - for err in new_errs: - if err not in ex_errs: - logging.error( - "Unable to save file - %s", err[13:len(err) - 6]) - logging.error("Attempted Save Notes") - logging.error(self.save_notes) + logging.error("Unable to save files: %s. Attempted Save Notes: %s", + ", ".join(err[13:len(err) - 6] for err in new_errs + # Only new errors caused by recent save + if err not in ex_errs), self.save_notes) # Wrapper functions for Reverter class def recovery_routine(self): diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index d895c165c..50a66c0d0 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -333,28 +333,22 @@ def challb_to_achall(challb, key, domain): """ chall = challb.chall + logging.info("%s challenge for %s", chall.typ, domain) if isinstance(chall, challenges.DVSNI): - logging.info(" DVSNI challenge for %s.", domain) return achallenges.DVSNI( challb=challb, domain=domain, key=key) elif isinstance(chall, challenges.SimpleHTTP): - logging.info(" SimpleHTTP challenge for %s.", domain) return achallenges.SimpleHTTP( challb=challb, domain=domain, key=key) elif isinstance(chall, challenges.DNS): - logging.info(" DNS challenge for %s.", domain) return achallenges.DNS(challb=challb, domain=domain) - elif isinstance(chall, challenges.RecoveryToken): - logging.info(" Recovery Token Challenge for %s.", domain) return achallenges.RecoveryToken(challb=challb, domain=domain) elif isinstance(chall, challenges.RecoveryContact): - logging.info(" Recovery Contact Challenge for %s.", domain) return achallenges.RecoveryContact( challb=challb, domain=domain) elif isinstance(chall, challenges.ProofOfPossession): - logging.info(" Proof-of-Possession Challenge for %s", domain) return achallenges.ProofOfPossession( challb=challb, domain=domain) diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index 1eb565289..7e1bb58fb 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -40,7 +40,7 @@ def init_save_key(key_size, key_dir, keyname="key-letsencrypt.pem"): try: key_pem = make_key(key_size) except ValueError as err: - logging.fatal(str(err)) + logging.exception(err) raise err # Save file diff --git a/letsencrypt_apache/configurator.py b/letsencrypt_apache/configurator.py index 0cff94bbd..5b0dbdea9 100644 --- a/letsencrypt_apache/configurator.py +++ b/letsencrypt_apache/configurator.py @@ -181,8 +181,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if not path["cert_path"] or not path["cert_key"]: # Throw some can't find all of the directives error" logging.warn( - "Cannot find a cert or key directive in %s", vhost.path) - logging.warn("VirtualHost was not modified") + "Cannot find a cert or key directive in %s. " + "VirtualHost was not modified", vhost.path) # Presumably break here so that the virtualhost is not modified return False @@ -408,8 +408,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Note: This could be made to also look for ip:443 combo # TODO: Need to search only open directives and IfMod mod_ssl.c if len(self.parser.find_dir(parser.case_i("Listen"), "443")) == 0: - logging.debug("No Listen 443 directive found") - logging.debug("Setting the Apache Server to Listen on port 443") + logging.debug("No Listen 443 directive found. Setting the " + "Apache Server to Listen on port 443") path = self.parser.add_dir_to_ifmodssl( parser.get_aug_path(self.parser.loc["listen"]), "Listen", "443") self.save_notes += "Added Listen 443 directive to %s\n" % path @@ -922,9 +922,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if proc.returncode != 0: # Enter recovery routine... - logging.error("Configtest failed") - logging.error(stdout) - logging.error(stderr) + logging.error("Configtest failed\n%s\n%s", stdout, stderr) return False return True @@ -1054,9 +1052,8 @@ def enable_mod(mod_name, apache_init_script, apache_enmod): stdout=open("/dev/null", "w"), stderr=open("/dev/null", "w")) apache_restart(apache_init_script) - except (OSError, subprocess.CalledProcessError) as err: - logging.error("Error enabling mod_%s", mod_name) - logging.error("Exception: %s", err) + except (OSError, subprocess.CalledProcessError): + logging.exception("Error enabling mod_%s", mod_name) sys.exit(1) @@ -1119,9 +1116,7 @@ def apache_restart(apache_init_script): if proc.returncode != 0: # Enter recovery routine... - logging.error("Apache Restart Failed!") - logging.error(stdout) - logging.error(stderr) + logging.error("Apache Restart Failed!\n%s\n%s", stdout, stderr) return False except (OSError, ValueError): diff --git a/letsencrypt_apache/dvsni.py b/letsencrypt_apache/dvsni.py index c25426371..5ff09aa50 100644 --- a/letsencrypt_apache/dvsni.py +++ b/letsencrypt_apache/dvsni.py @@ -59,10 +59,9 @@ class ApacheDvsni(common.Dvsni): vhost = self.configurator.choose_vhost(achall.domain) if vhost is None: logging.error( - "No vhost exists with servername or alias of: %s", - achall.domain) - logging.error("No _default_:443 vhost exists") - logging.error("Please specify servernames in the Apache config") + "No vhost exists with servername or alias of: %s. " + "No _default_:443 vhost exists. Please specify servernames " + "in the Apache config", achall.domain) return None # TODO - @jdkasten review this code to make sure it makes sense diff --git a/letsencrypt_nginx/configurator.py b/letsencrypt_nginx/configurator.py index f7b53f3fa..f74ad0a3a 100644 --- a/letsencrypt_nginx/configurator.py +++ b/letsencrypt_nginx/configurator.py @@ -130,9 +130,8 @@ class NginxConfigurator(common.Plugin): vhost.filep, vhost.names) except errors.LetsEncryptMisconfigurationError: logging.warn( - "Cannot find a cert or key directive in %s for %s", - vhost.filep, vhost.names) - logging.warn("VirtualHost was not modified") + "Cannot find a cert or key directive in %s for %s. " + "VirtualHost was not modified.", vhost.filep, vhost.names) # Presumably break here so that the virtualhost is not modified return False @@ -352,9 +351,7 @@ class NginxConfigurator(common.Plugin): if proc.returncode != 0: # Enter recovery routine... - logging.error("Config test failed") - logging.error(stdout) - logging.error(stderr) + logging.error("Config test failed\n%s\n%s", stdout, stderr) return False return True @@ -570,14 +567,11 @@ def nginx_restart(nginx_ctl): if nginx_proc.returncode != 0: # Enter recovery routine... - logging.error("Nginx Restart Failed!") - logging.error(stdout) - logging.error(stderr) + logging.error("Nginx Restart Failed!\n%s\n%s", stdout, stderr) return False except (OSError, ValueError): - logging.fatal( - "Nginx Restart Failed - Please Check the Configuration") + logging.fatal("Nginx Restart Failed - Please Check the Configuration") sys.exit(1) return True diff --git a/letsencrypt_nginx/dvsni.py b/letsencrypt_nginx/dvsni.py index 0697f6e1e..f6f82c5cb 100644 --- a/letsencrypt_nginx/dvsni.py +++ b/letsencrypt_nginx/dvsni.py @@ -50,9 +50,9 @@ class NginxDvsni(common.Dvsni): vhost = self.configurator.choose_vhost(achall.domain) if vhost is None: logging.error( - "No nginx vhost exists with server_name matching: %s", + "No nginx vhost exists with server_name matching: %s. " + "Please specify server_names in the Nginx config.", achall.domain) - logging.error("Please specify server_names in the Nginx config") return None for addr in vhost.addrs: