Error reporting to the client has been improved

castor/tape/tapeserver/daemon/DiskReadTask.cpp

@@ -89,8 +89,9 @@ void DiskReadTask::execute(log::LogContext& lc) {
       //we either read at full capacity (ie size=capacity) or if there different,
       //it should be the end => migratingFileSize should be 0. If it not, error
       if(mb->m_payload.size() != mb->m_payload.totalCapacity() && migratingFileSize>0){
-        mb->markAsFailed();
-        throw castor::tape::Exception("Error while reading a file. Did not read at full capacity but the file is not fully read");
+        std::string erroMsg = "Error while reading a file. Did not read at full capacity but the file is not fully read";
+        mb->markAsFailed(erroMsg,SEINTERNAL);
+        throw castor::tape::Exception(erroMsg);
       }
     } //end of while(migratingFileSize>0)
   }
@@ -127,7 +128,7 @@ void DiskReadTask::circulateAllBlocks(size_t fromBlockId){
   while(blockId<m_numberOfBlock) {
     MemBlock * mb = m_nextTask.getFreeBlock();
     mb->m_fileid = m_migratedFile->fileid();
-    mb->markAsFailed();
+//    mb->markAsFailed();
     m_nextTask.pushDataBlock(mb);
     ++blockId;
   } //end of while

castor/tape/tapeserver/daemon/DiskWriteTask.cpp

@@ -50,25 +50,34 @@ bool DiskWriteTask::execute(RecallReportPacker& reporter,log::LogContext& lc) {
     while(1) {
       if(MemBlock* const mb = m_fifo.pop()) {
         AutoReleaseBlock<RecallMemoryManager> releaser(mb,m_memManager);
-        if(mb->m_cancelled) {
-          // If the tape side got cancelled, we report nothing and count
+        if(mb->isCanceled()) {
+          // If the tape side got canceled, we report nothing and count
           // it as a success.
-          lc.log(LOG_DEBUG, "File transfer cancelled");
+          lc.log(LOG_DEBUG, "File transfer canceled");
           return true;
         }
+        //----------------------------deal with errors--------------------------
         if(m_recallingFile->fileid() != static_cast<unsigned int>(mb->m_fileid)
-                || blockId != mb->m_fileBlock  || mb->m_failed ){
+                || blockId != mb->m_fileBlock  || mb->isFailed() ){
           LogContext::ScopedParam sp[]={
             LogContext::ScopedParam(lc, Param("expected_NSFILEID",m_recallingFile->fileid())),
             LogContext::ScopedParam(lc, Param("received_NSFILEID", mb->m_fileid)),
             LogContext::ScopedParam(lc, Param("expected_NSFBLOCKId", blockId)),
             LogContext::ScopedParam(lc, Param("received_NSFBLOCKId", mb->m_fileBlock)),
-            LogContext::ScopedParam(lc, Param("failed_Status", mb->m_failed))
+            LogContext::ScopedParam(lc, Param("failed_Status", mb->isFailed()))
           };
           tape::utils::suppresUnusedVariable(sp);
-          lc.log(LOG_ERR,mb->m_errorMsg);
-          throw castor::tape::Exception(mb->m_errorMsg);
+          std::string errorMsg;
+          if(mb->isFailed()){
+            errorMsg=mb->errorMsg();
+          }
+          else{
+            errorMsg="Mistmatch between expected and received filed or blockid";
+          }
+          lc.log(LOG_ERR,errorMsg);
+          throw castor::tape::Exception(errorMsg);
         }
+        //----------------------------end deal with errors-----------------------
         mb->m_payload.write(ourFile);
         checksum = mb->m_payload.adler32(checksum);
         blockId++;

castor/tape/tapeserver/daemon/DiskWriteTaskTest.cpp

@@ -65,7 +65,9 @@ namespace unitTests{
       MemBlock* mb=mm.getFreeBlock();
       mb->m_fileid=0;
       mb->m_fileBlock=i;
-      mb->m_failed = (i==5) ? true : false;
+      if(5==i){
+        mb->markAsFailed("Test error",SEINTERNAL);
+      }
       t.pushDataBlock(mb);
     }    
     MemBlock* mb=mm.getFreeBlock();

castor/tape/tapeserver/daemon/MemBlock.hpp

@@ -37,6 +37,38 @@ namespace daemon {
  * Individual memory block with metadata
  */
 class MemBlock {
+  
+  struct AlterationContext{
+    //provide an enumation of type, thus we can 
+    //overload the constructor on those types
+    struct Failed_t{};
+    static Failed_t Failed ;
+    
+    struct Cancelled_t{}; 
+    static Cancelled_t Cancelled;
+    
+    /** Flag indicating to the receiver that the file read failed */
+    bool m_failed;
+    
+    /** Flag indicating that the transfer was cancelled, usually due to a 
+     previous failure. */
+    bool m_cancelled;
+    
+    /**
+     * in case of error, the error message 
+     */
+    std::string m_errorMsg;
+    
+    int m_errorCode;
+    
+    AlterationContext(const std::string& msg,int errorCode,Failed_t):
+    m_failed(true),m_cancelled(false),m_errorMsg(msg),m_errorCode(errorCode){}
+    
+    AlterationContext(Cancelled_t):
+    m_failed(false),m_cancelled(true),m_errorMsg(""),m_errorCode(0){}
+  };
+  
+  std::auto_ptr<AlterationContext> m_context;
 public:
   /**
    * COnstrucor 
@@ -48,25 +80,42 @@ public:
     reset();
   }
   
+  std::string errorMsg() const {
+    if(m_context.get()) {
+      return m_context->m_errorMsg;
+    }
+
+    throw castor::exception::Exception("Error Context is not set ="
+            " no error message to give");
+  }
+    
+  bool isFailed() const {
+    return m_context.get() && m_context->m_failed;
+  }
+  
+  bool isCanceled() const {
+    return m_context.get() && m_context->m_cancelled;
+  }
+    
   /**
    * Mark this block as failed ie 
    * m_failed is true, m_fileBlock and m_tapeFileBlock are set at -1
    * Other members do not change
    */
-  void markAsFailed(){
-    m_failed = true;
+  void markAsFailed(const std::string msg,int errCode){
+    m_context.reset(new AlterationContext(msg,errCode,AlterationContext::Failed));
     m_fileBlock = -1;
     m_tapeFileBlock = -1;
   }
   /**
-   * Mark the block as cancelled: this indicates the writer thread that
+   * Mark the block as canceled: this indicates the writer thread that
    * the read was skipped due to previous, unrelated errors, and that this
    * file will not be processed at all (and hence should not be reported about).
    * This is mainly used for the tape read case, when positioning is confused
    * (when positioning by fSeq, there's nothing we can do).
    */
   void markAsCancelled(){
-    m_cancelled = true;
+    m_context.reset(new AlterationContext(AlterationContext::Cancelled));
     m_fileBlock = -1;
     m_tapeFileBlock = -1;
   }
@@ -79,10 +128,11 @@ public:
     m_fileBlock = -1;
     m_fSeq = -1;
     m_tapeFileBlock = -1;
-    m_failed=false;
-    m_cancelled=false;
     m_payload.reset();
-    m_errorMsg="";
+    
+    //delete the previous m_context (if allocated) 
+    //and set the new one to NULL
+    m_context.reset();
   }
   /** Unique memory block id */
   const int m_memoryBlockId;
@@ -105,14 +155,6 @@ public:
   /** Size of the tape blocks, allowing sanity checks on the disk write side in recalls */
   int m_tapeBlockSize;
   
-  /** Flag indicating to the receiver that the file read failed */
-  bool m_failed;
-  
-  /** Flag indicating that the transfer was cancelled, usually due to a 
-   previous failure. */
-  bool m_cancelled;
- 
-  std::string m_errorMsg;
 };
 
 }

castor/tape/tapeserver/daemon/TapeReadTask.hpp

@@ -163,7 +163,7 @@ public:
       
       // mb might or might not be allocated at this point, but 
       // reportErrorToDiskTask will deal with the allocation if required.
-      reportErrorToDiskTask(ex.getMessageValue(),mb);
+      reportErrorToDiskTask(ex.getMessageValue(),ex.code(),mb);
     } //end of catch
     watchdog.fileFinished();
   }
@@ -181,9 +181,10 @@ public:
 private:
   /**
    * Do the actual report to the disk write task
-   * @param mb We assume that mb is a valid mem block
+   * @param errorMsg The error message we will give to the client
+   * @param mb The mem block we will use
    */
-  void reportErrorToDiskTask(const std::string& msg,MemBlock* mb = NULL){
+  void reportErrorToDiskTask(const std::string& msg,int code,MemBlock* mb = NULL){
     //If we are not provided with a block, allocate it and
     // fill it up
     if (!mb) {
@@ -192,8 +193,7 @@ private:
       mb->m_fileid = m_fileToRecall->fileid();
     }
     //mark the block failed and push it (plus signal the end)
-     mb->markAsFailed();
-     mb->m_errorMsg=msg;
+     mb->markAsFailed(msg,code);
      m_fifo.pushDataBlock(mb);
      m_fifo.pushDataBlock(NULL);
    }

castor/tape/tapeserver/daemon/TapeWriteTask.cpp

@@ -68,7 +68,9 @@ namespace daemon {
     // stats to the session's.
     SessionStats localStats;
     utils::Timer localTime;
+    
     unsigned long ckSum = Payload::zeroAdler32();
+    
     int blockId  = 0;
     try {
       //we first check here to not even try to move the tape  if a previous task has failed
@@ -88,20 +90,27 @@ namespace daemon {
         MemBlock* const mb = m_fifo.popDataBlock();
         localStats.waitDataTime += timer.secs(utils::Timer::resetCounter);
         AutoReleaseBlock<MigrationMemoryManager> releaser(mb,m_memManager);
-        
+        //----------------------------deal with errors--------------------------
         if(m_fileToMigrate->fileid() != static_cast<unsigned int>(mb->m_fileid)
-                         || blockId != mb->m_fileBlock  || mb->m_failed ){
+                         || blockId != mb->m_fileBlock  || mb->isFailed() ){
           LogContext::ScopedParam sp[]={
             LogContext::ScopedParam(lc, Param("received_NSFILEID", mb->m_fileid)),
             LogContext::ScopedParam(lc, Param("expected_NSFBLOCKId", blockId)),
             LogContext::ScopedParam(lc, Param("received_NSFBLOCKId", mb->m_fileBlock)),
-            LogContext::ScopedParam(lc, Param("failed_Status", mb->m_failed))
+            LogContext::ScopedParam(lc, Param("failed_Status", mb->isFailed()))
           };
           tape::utils::suppresUnusedVariable(sp);
-          lc.log(LOG_ERR,"Failed to read from disk");
-          throw castor::tape::Exception("Failed to read from disk");
+          std::string errorMsg;
+          if(mb->isFailed()){
+            errorMsg=mb->errorMsg();
+          }
+          else{
+            errorMsg="Mistmatch between expected and received filed or blockid";
+          }
+          lc.log(LOG_ERR,errorMsg);
+          throw castor::tape::Exception(errorMsg);
         }
-
+        //----------------------------end deal with errors-----------------------
         ckSum =  mb->m_payload.adler32(ckSum);
         localStats.checksumingTime += timer.secs(utils::Timer::resetCounter);
         mb->m_payload.write(*output);