Error reporting cleanup

git-svn-id: http://svn.savannah.nongnu.org/svn/rdiff-backup/trunk@684 2b77aa54-bcbc-44c9-a7ec-4f6cf2b41109

7 files changed, 81 insertions, 26 deletions

diff --git a/rdiff-backup/CHANGELOG b/rdiff-backup/CHANGELOG
index 17c6b07..2008e1c 100644
--- a/rdiff-backup/CHANGELOG
+++ b/rdiff-backup/CHANGELOG
@@ -1,3 +1,13 @@
+New in v1.1.3 (????/??/??)
+--------------------------
+
+Regression metadata bug introduced with 1.1.1/1.1.2 fixed.
+
+rdiff-backup should now give a clean error message (no stack traces!)
+when aborted with control-C, killed with a signal, or when the
+connection is lost.
+
+
 New in v1.1.2 (2005/11/06)
 --------------------------
 
diff --git a/rdiff-backup/rdiff-backup b/rdiff-backup/rdiff-backup
index 54f5a18..b27be9e 100755
--- a/rdiff-backup/rdiff-backup
+++ b/rdiff-backup/rdiff-backup
@@ -20,6 +20,6 @@ import sys
 import rdiff_backup.Main
 
 if __name__ == "__main__" and not globals().has_key('__no_execute__'):
-	rdiff_backup.Main.Main(sys.argv[1:])
+	rdiff_backup.Main.error_check_Main(sys.argv[1:])
 
 
diff --git a/rdiff-backup/rdiff_backup/Main.py b/rdiff-backup/rdiff_backup/Main.py
index 210a246..ccda5b4 100644
--- a/rdiff-backup/rdiff_backup/Main.py
+++ b/rdiff-backup/rdiff_backup/Main.py
@@ -87,7 +87,7 @@ def parse_cmdlineoptions(arglist):
 		  "user-mapping-file=", "verbosity=", "verify",
 		  "verify-at-time=", "version"])
 	except getopt.error, e:
-		commandline_error("Bad commandline options: %s" % str(e))
+		commandline_error("Bad commandline options: " + str(e))
 
 	for opt, arg in optlist:
 		if opt == "-b" or opt == "--backup-mode": action = "backup"
@@ -231,9 +231,8 @@ def final_set_action(rps):
 	else: action = "backup"
 
 def commandline_error(message):
-	sys.stderr.write("Error: %s\n" % message)
-	sys.stderr.write("See the rdiff-backup manual page for instructions\n")
-	sys.exit(2)
+	Log.FatalError(message + "\nSee the rdiff-backup manual page for "
+				   "more information.")
 
 def misc_setup(rps):
 	"""Set default change ownership flag, umask, relay regexps"""
@@ -291,6 +290,19 @@ def cleanup():
 	Log.close_logfile()
 	if not Globals.server: SetConnections.CloseConnections()
 
+def error_check_Main(arglist):
+	"""Run Main on arglist, suppressing stack trace for routine errors"""
+	try: Main(arglist)
+	except SystemExit: raise
+	except Exception, exc:
+		errmsg = robust.is_routine_fatal(exc)
+		if errmsg:
+			Log.exception(2, 6)
+			Log.FatalError(errmsg)
+		else:
+			Log.exception(2, 2)
+			raise
+
 def Main(arglist):
 	"""Start everything up!"""
 	parse_cmdlineoptions(arglist)
@@ -304,7 +316,6 @@ def Main(arglist):
 	cleanup()
 	if return_val is not None: sys.exit(return_val)
 
-
 def Backup(rpin, rpout):
 	"""Backup, possibly incrementally, src_path to dest_path."""
 	global incdir
diff --git a/rdiff-backup/rdiff_backup/connection.py b/rdiff-backup/rdiff_backup/connection.py
index 99b8f72..dadbcd5 100644
--- a/rdiff-backup/rdiff_backup/connection.py
+++ b/rdiff-backup/rdiff_backup/connection.py
@@ -31,6 +31,7 @@ except ImportError: pass
 
 class ConnectionError(Exception): pass
 class ConnectionReadError(ConnectionError): pass
+class ConnectionWriteError(ConnectionError): pass
 class ConnectionQuit(Exception): pass
 
 
@@ -197,14 +198,17 @@ class LowLevelPipeConnection(Connection):
 
 	def _write(self, headerchar, data, req_num):
 		"""Write header and then data to the pipe"""
-		self.outpipe.write(headerchar + chr(req_num) +
-						   C.long2str(long(len(data))))
-		self.outpipe.write(data)
-		self.outpipe.flush()
+		try:
+			self.outpipe.write(headerchar + chr(req_num) +
+							   C.long2str(long(len(data))))
+			self.outpipe.write(data)
+			self.outpipe.flush()
+		except IOError: raise ConnectionWriteError()
 
 	def _read(self, length):
 		"""Read length bytes from inpipe, returning result"""
-		return self.inpipe.read(length)
+		try: return self.inpipe.read(length)
+		except IOError: raise ConnectionReadError()
 
 	def _s2l_old(self, s):
 		"""Convert string to long int"""
@@ -228,11 +232,9 @@ class LowLevelPipeConnection(Connection):
 		if not len(header_string) == 9:
 			raise ConnectionReadError("Truncated header string (problem "
 									  "probably originated remotely)")
-		try:
-			format_string, req_num, length = (header_string[0],
-											  ord(header_string[1]),
-											  C.str2long(header_string[2:]))
-		except IndexError: raise ConnectionError()
+		format_string, req_num, length = (header_string[0],
+										  ord(header_string[1]),
+										  C.str2long(header_string[2:]))
 		if format_string == "q": raise ConnectionQuit("Received quit signal")
 
 		data = self._read(length)
@@ -337,6 +339,8 @@ class PipeConnection(LowLevelPipeConnection):
 
 	def extract_exception(self):
 		"""Return active exception"""
+		if robust.is_routine_fatal(sys.exc_info()[1]):
+			raise # Fatal error--No logging necessary, but connection down
 		if log.Log.verbosity >= 5 or log.Log.term_verbosity >= 5:
 			log.Log("Sending back exception %s of type %s: \n%s" %
 					(sys.exc_info()[1], sys.exc_info()[0],
diff --git a/rdiff-backup/rdiff_backup/log.py b/rdiff-backup/rdiff_backup/log.py
index f977420..93933e6 100644
--- a/rdiff-backup/rdiff_backup/log.py
+++ b/rdiff-backup/rdiff_backup/log.py
@@ -156,9 +156,9 @@ class Logger:
 		assert no_fatal_message == 0 or no_fatal_message == 1
 		if no_fatal_message: prefix_string = ""
 		else: prefix_string = "Fatal Error: "
-		self(prefix_string + message, 1)
-		import Main
-		Main.cleanup()
+		self.log_to_term(prefix_string + message, 1)
+		#import Main
+		#Main.cleanup()
 		sys.exit(errlevel)
 
 	def exception_to_string(self, arglist = []):
@@ -182,10 +182,13 @@ class Logger:
 		if (only_terminal == 0 or
 			(only_terminal == 1 and self.log_file_open)):
 			logging_func = self.__call__
-		else: logging_func = self.log_to_term
+		else:
+			logging_func = self.log_to_term
+			if verbosity >= self.term_verbosity: return
 
 		logging_func(self.exception_to_string(), verbosity)
 
+
 Log = Logger()
 
 
diff --git a/rdiff-backup/rdiff_backup/robust.py b/rdiff-backup/rdiff_backup/robust.py
index 67221e3..4fa8e6c 100644
--- a/rdiff-backup/rdiff_backup/robust.py
+++ b/rdiff-backup/rdiff_backup/robust.py
@@ -19,8 +19,8 @@
 
 """Catch various exceptions given system call"""
 
-import errno, signal
-import librsync, C, static, rpath, Globals, log, statistics
+import errno, signal, exceptions
+import librsync, C, static, rpath, Globals, log, statistics, connection
 
 def check_common_error(error_handler, function, args = []):
 	"""Apply function to args, if error, run error_handler on exception
@@ -38,7 +38,8 @@ def check_common_error(error_handler, function, args = []):
 			if conn is not None: conn.statistics.record_error()
 			if error_handler: return error_handler(exc, *args)
 			else: return None
-		log.Log.exception(1, 2)
+		if is_routine_fatal(exc): log.Log.exception(1, 6)
+		else: log.Log.exception(1, 2)
 		raise
 
 def catch_error(exc):
@@ -59,6 +60,22 @@ def catch_error(exc):
 		return 1
 	return 0
 
+def is_routine_fatal(exc):
+	"""Return string if exception is non-error unrecoverable, None otherwise
+
+	Used to suppress a stack trace for exceptions like keyboard
+	interrupts or connection drops.  Return value is string to use as
+	an exit message.
+
+	"""
+	if isinstance(exc, exceptions.KeyboardInterrupt):
+		return "User abort"
+	elif isinstance(exc, connection.ConnectionError):
+		return "Lost connection to the remote system"
+	elif isinstance(exc, SignalException):
+		return "Killed with signal %s" % (exc,)
+	return None
+
 def get_error_handler(error_type):
 	"""Return error handler function that can be used above
 
diff --git a/rdiff-backup/rdiff_backup/rpath.py b/rdiff-backup/rdiff_backup/rpath.py
index 6793147..47f3b3e 100644
--- a/rdiff-backup/rdiff_backup/rpath.py
+++ b/rdiff-backup/rdiff_backup/rpath.py
@@ -265,7 +265,7 @@ def make_socket_local(rpath):
 def gzip_open_local_read(rpath):
 	"""Return open GzipFile.  See security note directly above"""
 	assert rpath.conn is Globals.local_connection
-	return gzip.GzipFile(rpath.path, "rb")
+	return GzipFile(rpath.path, "rb")
 
 def open_local_read(rpath):
 	"""Return open file (provided for security reasons)"""
@@ -982,13 +982,13 @@ class RPath(RORPath):
 
 		"""
 		if self.conn is Globals.local_connection:
-			if compress: return gzip.GzipFile(self.path, mode)
+			if compress: return GzipFile(self.path, mode)
 			else: return open(self.path, mode)
 
 		if compress:
 			if mode == "r" or mode == "rb":
 				return self.conn.rpath.gzip_open_local_read(self)
-			else: return self.conn.gzip.GzipFile(self.path, mode)
+			else: return self.conn.rpath.GzipFile(self.path, mode)
 		else:
 			if mode == "r" or mode == "rb":
 				return self.conn.rpath.open_local_read(self)
@@ -1217,6 +1217,16 @@ class RPathFileHook:
 		return result
 
 
+class GzipFile(gzip.GzipFile):
+	"""Like gzip.GzipFile, except remove destructor
+
+	The default GzipFile's destructor prints out some messy error
+	messages.  Use this class instead to clean those up.
+
+	"""
+	def __del__(self): pass
+
+
 def setdata_local(rpath):
 	"""Set eas/acls, uid/gid, resource fork in data dictionary