[Box Backup-commit] COMMIT r3417 - in box/trunk: lib/backupstore lib/server test/basicserver

subversion at boxbackup.org subversion at boxbackup.org
Fri Oct 31 22:09:43 GMT 2014


Author: chris
Date: 2014-10-31 22:09:42 +0000 (Fri, 31 Oct 2014)
New Revision: 3417

Modified:
   box/trunk/lib/backupstore/BackupCommands.cpp
   box/trunk/lib/backupstore/BackupProtocol.h
   box/trunk/lib/server/makeprotocol.pl.in
   box/trunk/test/basicserver/TestCommands.cpp
Log:
Refactor handling of exceptions in protocol server command executors.

Add a standard method to Replyable that will be called if a recoverable
exception (a BoxException) occurs, and can return a protocol Message to be
sent to the client, such as an error code for various standard errors, or
rethrow the exception.

If you want something different, catch exceptions and return the desired
reply yourself, or you'll get the default handling.

Modified: box/trunk/lib/backupstore/BackupCommands.cpp
===================================================================
--- box/trunk/lib/backupstore/BackupCommands.cpp	2014-10-31 22:09:39 UTC (rev 3416)
+++ box/trunk/lib/backupstore/BackupCommands.cpp	2014-10-31 22:09:42 UTC (rev 3417)
@@ -52,11 +52,58 @@
 		return PROTOCOL_ERROR(Err_SessionReadOnly); \
 	}
 
+
 // --------------------------------------------------------------------------
 //
 // Function
-//		Name:    BackupProtocolVersion::DoCommand(Protocol &, BackupStoreContext &)
-//		Purpose: Return the current version, or an error if the requested version isn't allowed
+//		Name:    BackupProtocolMessage::HandleException(BoxException& e)
+//		Purpose: Return an error message appropriate to the passed-in
+//		exception, or rethrow it.
+//		Created: 2014/09/14
+//
+// --------------------------------------------------------------------------
+std::auto_ptr<BackupProtocolMessage> BackupProtocolReplyable::HandleException(BoxException& e) const
+{
+	if(e.GetType() == RaidFileException::ExceptionType &&
+		e.GetSubType() == RaidFileException::RaidFileDoesntExist)
+	{
+		return PROTOCOL_ERROR(Err_DoesNotExist);
+	}
+	else if (e.GetType() == BackupStoreException::ExceptionType)
+	{
+		if(e.GetSubType() == BackupStoreException::AddedFileDoesNotVerify)
+		{
+			return PROTOCOL_ERROR(Err_FileDoesNotVerify);
+		}
+		else if(e.GetSubType() == BackupStoreException::AddedFileExceedsStorageLimit)
+		{
+			return PROTOCOL_ERROR(Err_StorageLimitExceeded);
+		}
+		else if(e.GetSubType() == BackupStoreException::MultiplyReferencedObject)
+		{
+			return PROTOCOL_ERROR(Err_MultiplyReferencedObject);
+		}
+		else if(e.GetSubType() == BackupStoreException::CouldNotFindEntryInDirectory)
+		{
+			return PROTOCOL_ERROR(Err_DoesNotExist);
+		}
+		else if(e.GetSubType() == BackupStoreException::NameAlreadyExistsInDirectory)
+		{
+			return PROTOCOL_ERROR(Err_TargetNameExists);
+		}
+	}
+
+	throw;
+}
+
+
+// --------------------------------------------------------------------------
+//
+// Function
+//		Name:    BackupProtocolVersion::DoCommand(Protocol &,
+//			 BackupStoreContext &)
+//		Purpose: Return the current version, or an error if the
+//			 requested version isn't allowed
 //		Created: 2003/08/20
 //
 // --------------------------------------------------------------------------
@@ -192,23 +239,12 @@
 	// Store the listing to a stream
 	std::auto_ptr<CollectInBufferStream> stream(new CollectInBufferStream);
 
-	try
-	{
-		// Ask the context for a directory
-		const BackupStoreDirectory &rdir(
-			rContext.GetDirectory(mObjectID));
-		rdir.WriteToStream(*stream, mFlagsMustBeSet,
-			mFlagsNotToBeSet, mSendAttributes,
-			false /* never send dependency info to the client */);
-	}
-	catch(RaidFileException &e)
-	{
-		if (e.GetSubType() == RaidFileException::RaidFileDoesntExist)
-		{
-			return PROTOCOL_ERROR(Err_DoesNotExist);
-		}
-		throw;
-	}
+	// Ask the context for a directory
+	const BackupStoreDirectory &rdir(
+		rContext.GetDirectory(mObjectID));
+	rdir.WriteToStream(*stream, mFlagsMustBeSet,
+		mFlagsNotToBeSet, mSendAttributes,
+		false /* never send dependency info to the client */);
 
 	stream->SetForReading();
 
@@ -252,29 +288,10 @@
 	}
 
 	// Ask the context to store it
-	int64_t id = 0;
-	try
-	{
-		id = rContext.AddFile(rDataStream, mDirectoryObjectID,
-			mModificationTime, mAttributesHash, mDiffFromFileID,
-			mFilename,
-			true /* mark files with same name as old versions */);
-	}
-	catch(BackupStoreException &e)
-	{
-		if(e.GetSubType() == BackupStoreException::AddedFileDoesNotVerify)
-		{
-			return PROTOCOL_ERROR(Err_FileDoesNotVerify);
-		}
-		else if(e.GetSubType() == BackupStoreException::AddedFileExceedsStorageLimit)
-		{
-			return PROTOCOL_ERROR(Err_StorageLimitExceeded);
-		}
-		else
-		{
-			throw;
-		}
-	}
+	int64_t id = rContext.AddFile(rDataStream, mDirectoryObjectID,
+		mModificationTime, mAttributesHash, mDiffFromFileID,
+		mFilename,
+		true /* mark files with same name as old versions */);
 
 	// Tell the caller what the file ID was
 	return std::auto_ptr<BackupProtocolMessage>(new BackupProtocolSuccess(id));
@@ -507,26 +524,10 @@
 	}
 
 	bool alreadyExists = false;
-	int64_t id;
+	int64_t id = rContext.AddDirectory(mContainingDirectoryID,
+		mDirectoryName, attr, mAttributesModTime, mModificationTime,
+		alreadyExists);
 
-	try
-	{
-		id = rContext.AddDirectory(mContainingDirectoryID,
-			mDirectoryName, attr, mAttributesModTime, mModificationTime,
-			alreadyExists);
-	}
-	catch(BackupStoreException &e)
-	{
-		if(e.GetSubType() == BackupStoreException::AddedFileExceedsStorageLimit)
-		{
-			return PROTOCOL_ERROR(Err_StorageLimitExceeded);
-		}
-		else
-		{
-			throw;
-		}
-	}
-
 	if(alreadyExists)
 	{
 		return PROTOCOL_ERROR(Err_DirectoryAlreadyExists);
@@ -666,19 +667,7 @@
 	}
 
 	// Context handles this
-	try
-	{
-		rContext.DeleteDirectory(mObjectID);
-	}
-	catch(BackupStoreException &e)
-	{
-		if(e.GetSubType() == BackupStoreException::MultiplyReferencedObject)
-		{
-			return PROTOCOL_ERROR(Err_MultiplyReferencedObject);
-		}
-		
-		throw;
-	}
+	rContext.DeleteDirectory(mObjectID);
 
 	// return the object ID
 	return std::auto_ptr<BackupProtocolMessage>(new BackupProtocolSuccess(mObjectID));
@@ -746,27 +735,9 @@
 	CHECK_WRITEABLE_SESSION
 
 	// Let context do this, but modify error reporting on exceptions...
-	try
-	{
-		rContext.MoveObject(mObjectID, mMoveFromDirectory, mMoveToDirectory,
-			mNewFilename, (mFlags & Flags_MoveAllWithSameName) == Flags_MoveAllWithSameName,
-			(mFlags & Flags_AllowMoveOverDeletedObject) == Flags_AllowMoveOverDeletedObject);
-	}
-	catch(BackupStoreException &e)
-	{
-		if(e.GetSubType() == BackupStoreException::CouldNotFindEntryInDirectory)
-		{
-			return PROTOCOL_ERROR(Err_DoesNotExist);
-		}
-		else if(e.GetSubType() == BackupStoreException::NameAlreadyExistsInDirectory)
-		{
-			return PROTOCOL_ERROR(Err_TargetNameExists);
-		}
-		else
-		{
-			throw;
-		}
-	}
+	rContext.MoveObject(mObjectID, mMoveFromDirectory, mMoveToDirectory,
+		mNewFilename, (mFlags & Flags_MoveAllWithSameName) == Flags_MoveAllWithSameName,
+		(mFlags & Flags_AllowMoveOverDeletedObject) == Flags_AllowMoveOverDeletedObject);
 
 	// Return the object ID
 	return std::auto_ptr<BackupProtocolMessage>(new BackupProtocolSuccess(mObjectID));

Modified: box/trunk/lib/backupstore/BackupProtocol.h
===================================================================
--- box/trunk/lib/backupstore/BackupProtocol.h	2014-10-31 22:09:39 UTC (rev 3416)
+++ box/trunk/lib/backupstore/BackupProtocol.h	2014-10-31 22:09:42 UTC (rev 3417)
@@ -19,7 +19,7 @@
 // Class
 //		Name:    BackupProtocolLocal2
 //		Purpose: BackupProtocolLocal with a few more IQ points
-//		Created: 2003/08/21
+//		Created: 2014/09/20
 //
 // --------------------------------------------------------------------------
 class BackupProtocolLocal2 : public BackupProtocolLocal

Modified: box/trunk/lib/server/makeprotocol.pl.in
===================================================================
--- box/trunk/lib/server/makeprotocol.pl.in	2014-10-31 22:09:39 UTC (rev 3416)
+++ box/trunk/lib/server/makeprotocol.pl.in	2014-10-31 22:09:42 UTC (rev 3417)
@@ -604,10 +604,11 @@
 
 	virtual int GetTimeout() = 0;
 	void SendStreamAfterCommand(std::auto_ptr<IOStream> apStream);
-	
+
 protected:
 	std::list<IOStream*> mStreamsToSend;
 	void DeleteStreamsToSend();
+	virtual std::auto_ptr<$message_base_class> HandleException(BoxException& e) const;
 
 private:
 	$replyable_base_class(const $replyable_base_class &rToCopy); /* do not call */
@@ -1077,24 +1078,31 @@
 		// Run the command
 		try
 		{
-			if(pobj->HasStreamWithCommand())
+			try
 			{
-				std::auto_ptr<IOStream> apDataStream = ReceiveStream();
-				SelfFlushingStream autoflush(*apDataStream);
-				preply = pobj->DoCommand(*this, rContext, *apDataStream);
+				if(pobj->HasStreamWithCommand())
+				{
+					std::auto_ptr<IOStream> apDataStream = ReceiveStream();
+					SelfFlushingStream autoflush(*apDataStream);
+					preply = pobj->DoCommand(*this, rContext, *apDataStream);
+				}
+				else
+				{
+					preply = pobj->DoCommand(*this, rContext);
+				}
 			}
-			else
+			catch(BoxException &e)
 			{
-				preply = pobj->DoCommand(*this, rContext);
+				// First try a the built-in exception handler
+				preply = HandleException(e);
 			}
 		}
-		catch (std::exception &e)
-		{
-			Send($cmd_classes{$error_message}());
-			throw;
-		}
 		catch (...)
 		{
+			// Fallback in case the exception isn't a BoxException
+			// or the exception handler fails as well. This path
+			// throws the exception upwards, killing the process
+			// that handles the current client.
 			Send($cmd_classes{$error_message}());
 			throw;
 		}
@@ -1183,19 +1191,32 @@
 				}
 				elsif($writing_local)
 				{
+					print CPP <<__E;
+	std::auto_ptr<$message_base_class> apReply;
+	try
+	{
+__E
 					if($has_stream)
 					{
 						print CPP <<__E;
-	std::auto_ptr<$message_base_class> apReply = rQuery.DoCommand(*this,
-		mrContext, *apDataStream);
+		apReply = rQuery.DoCommand(*this, mrContext, *apDataStream);
 __E
 					}
 					else
 					{
 						print CPP <<__E;
-	std::auto_ptr<$message_base_class> apReply = rQuery.DoCommand(*this, mrContext);
+		apReply = rQuery.DoCommand(*this, mrContext);
 __E
 					}
+
+					print CPP <<__E;
+	}
+	catch(BoxException &e)
+	{
+		// First try a the built-in exception handler
+		apReply = HandleException(e);
+	}
+__E
 				}
 
 				# Common to both client and local

Modified: box/trunk/test/basicserver/TestCommands.cpp
===================================================================
--- box/trunk/test/basicserver/TestCommands.cpp	2014-10-31 22:09:39 UTC (rev 3416)
+++ box/trunk/test/basicserver/TestCommands.cpp	2014-10-31 22:09:42 UTC (rev 3417)
@@ -11,6 +11,11 @@
 #include "MemLeakFindOn.h"
 
 
+std::auto_ptr<TestProtocolMessage> TestProtocolReplyable::HandleException(BoxException& e) const
+{
+	throw;
+}
+
 std::auto_ptr<TestProtocolMessage> TestProtocolHello::DoCommand(TestProtocolReplyable &rProtocol, TestContext &rContext) const
 {
 	if(mNumber32 != 41 || mNumber16 != 87 || mNumber8 != 11 || mText != "pingu")




More information about the Boxbackup-commit mailing list