Improve error handling

- directory creation
  - zip extraction

UnitTests/Basics/files-test.cpp

@@ -40,13 +40,15 @@ using namespace triagens::basics;
 
 struct CFilesSetup {
   CFilesSetup () : _directory(TRI_UNKNOWN_MEM_ZONE) {
+    long systemError;
+    std::string errorMessage;
     BOOST_TEST_MESSAGE("setup files");
 
     _directory.appendText("/tmp/arangotest-");
     _directory.appendInteger((uint64_t) TRI_microtime());
     _directory.appendInteger((uint32_t) TRI_UInt32Random());
 
-    TRI_CreateDirectory(_directory.c_str());
+    TRI_CreateDirectory(_directory.c_str(), systemError, errorMessage);
   }
 
   ~CFilesSetup () {

arangod/VocBase/collection.cpp

@@ -1053,13 +1053,17 @@ TRI_collection_t* TRI_CreateCollection (TRI_vocbase_t* vocbase,
   }
 
   // create directory
-  int res = TRI_CreateDirectory(filename);
+  std::string errorMessage;
+  long systemError;
+  int res = TRI_CreateDirectory(filename, systemError, errorMessage);
 
   if (res != TRI_ERROR_NO_ERROR) {
-    LOG_ERROR("cannot create collection '%s' in directory '%s': %s",
+    LOG_ERROR("cannot create collection '%s' in directory '%s': %s - %ld - %s",
               parameters->_name,
               path,
-              TRI_errno_string(res));
+              TRI_errno_string(res),
+              systemError,
+              errorMessage.c_str());
 
     TRI_FreeString(TRI_CORE_MEM_ZONE, filename);
 

arangod/VocBase/server.cpp

@@ -440,16 +440,17 @@ static int CreateBaseApplicationDirectory (char const* basePath,
 
   if (path != nullptr) {
     if (! TRI_IsDirectory(path)) {
-      res = TRI_CreateDirectory(path);
+      std::string errorMessage;
+      long systemError;
+      res = TRI_CreateDirectory(path, systemError, errorMessage);
 
       if (res == TRI_ERROR_NO_ERROR) {
         LOG_INFO("created base application directory '%s'",
                  path);
       }
       else {
-        LOG_ERROR("unable to create base application directory '%s': %s",
+        LOG_ERROR("unable to create base application directory %s",
-                  path,
+                  errorMessage.c_str());
-                  TRI_errno_string(res));
       }
     }
 
@@ -474,7 +475,9 @@ static int CreateApplicationDirectory (char const* name,
 
   if (path != nullptr) {
     if (! TRI_IsDirectory(path)) {
-      res = TRI_CreateDirectory(path);
+      long systemError;
+      std::string errorMessage;
+      res = TRI_CreateDirectory(path, systemError, errorMessage);
 
       if (res == TRI_ERROR_NO_ERROR) {
         if (triagens::wal::LogfileManager::instance()->isInRecovery()) {
@@ -492,13 +495,13 @@ static int CreateApplicationDirectory (char const* name,
         LOG_INFO("unable to create application directory '%s' for database '%s': %s",
                  path,
                  name,
-                 TRI_errno_string(res));
+                 errorMessage.c_str());
       }
       else {
         LOG_ERROR("unable to create application directory '%s' for database '%s': %s",
                   path,
                   name,
-                  TRI_errno_string(res));
+                  errorMessage.c_str());
       }
     }
 
@@ -1178,10 +1181,15 @@ static int CreateDatabaseDirectory (TRI_server_t* server,
 
     return TRI_ERROR_OUT_OF_MEMORY;
   }
+  std::string errorMessage;
+  long systemError;
+  
+  res = TRI_CreateDirectory(file, systemError, errorMessage);
 
-  res = TRI_CreateDirectory(file);
+  if (res != TRI_ERROR_NO_ERROR){
-
+    if (res != TRI_ERROR_FILE_EXISTS) {
-  if (res != TRI_ERROR_NO_ERROR) {
+      LOG_ERROR("failed to create database directory: %s", errorMessage.c_str());
+    }
     TRI_FreeString(TRI_CORE_MEM_ZONE, file);
     TRI_FreeString(TRI_CORE_MEM_ZONE, dname);
 
@@ -1800,12 +1808,14 @@ int TRI_StartServer (TRI_server_t* server,
   // .............................................................................
 
   if (! TRI_IsDirectory(server->_databasePath)) {
-    res = TRI_CreateDirectory(server->_databasePath);
+    long systemError;
+    std::string errorMessage;
+    res = TRI_CreateDirectory(server->_databasePath, systemError, errorMessage);
 
     if (res != TRI_ERROR_NO_ERROR) {
       LOG_ERROR("unable to create database directory '%s': %s",
                 server->_databasePath,
-                TRI_errno_string(res));
+                errorMessage.c_str());
 
       return TRI_ERROR_ARANGO_DATADIR_NOT_WRITABLE;
     }
@@ -1851,12 +1861,14 @@ int TRI_StartServer (TRI_server_t* server,
       return TRI_ERROR_BAD_PARAMETER;
     }
 
-    res = TRI_CreateDirectory(server->_appPath);
+    long systemError;
+    std::string errorMessage;
+    res = TRI_CreateDirectory(server->_appPath, systemError, errorMessage);
 
     if (res != TRI_ERROR_NO_ERROR) {
       LOG_ERROR("unable to create --javascript.app-path directory '%s': %s",
                 server->_appPath,
-                TRI_errno_string(res));
+                errorMessage.c_str());
       return res;
     }
   }

arangosh/V8Client/arangodump.cpp

@@ -1385,10 +1385,12 @@ int main (int argc, char* argv[]) {
   }
 
   if (! isDirectory) {
-    int res = TRI_CreateDirectory(OutputDirectory.c_str());
+    long systemError;
+    std::string errorMessage;
+    int res = TRI_CreateDirectory(OutputDirectory.c_str(),systemError, errorMessage);
 
     if (res != TRI_ERROR_NO_ERROR) {
-      cerr << "unable to create output directory '" << OutputDirectory << "': " << string(TRI_errno_string(res)) << endl;
+      cerr << "unable to create output directory '" << OutputDirectory << "': " << errorMessage << endl;
       TRI_EXIT_FUNCTION(EXIT_FAILURE, nullptr);
     }
   }

lib/Basics/files.cpp

@@ -513,14 +513,13 @@ int TRI_MTimeFile (char const* path, int64_t* mtime) {
 ////////////////////////////////////////////////////////////////////////////////
 
 int TRI_CreateRecursiveDirectory (char const* path,
-                                  long *systemError,
+                                  long &systemError,
-                                  std::string *systemErrorStr) {
+                                  std::string &systemErrorStr) {
   TRI_ERRORBUF;
   char* copy;
   char* p;
   char* s;
   int res;
-  int m;
 
   res = TRI_ERROR_NO_ERROR;
   p = s = copy = TRI_DuplicateString(path);
@@ -530,19 +529,13 @@ int TRI_CreateRecursiveDirectory (char const* path,
       if (p - s > 0) {
 
         *p = '\0';
-        m = TRI_MKDIR(copy, 0777);
+        res = TRI_CreateDirectory(copy, systemError, systemErrorStr);
-        TRI_SYSTEM_ERROR();
+        if ((res == TRI_ERROR_FILE_EXISTS) ||
-        *p = TRI_DIR_SEPARATOR_CHAR;
+            (res == TRI_ERROR_NO_ERROR)) {
-        s = p + 1;
+          *p = TRI_DIR_SEPARATOR_CHAR;
-
+          s = p + 1;
-        if (m != 0 && errno != EEXIST) {
+        }
-          res = errno;
+        else {
-          if (systemErrorStr != nullptr) {
-            *systemErrorStr = std::string("Failed to create directory [") + path + "] " + TRI_GET_ERRORBUF;
-          }
-          if (systemError != nullptr) {
-            *systemError = errno;
-          }
           break;
         }
       }
@@ -550,12 +543,10 @@ int TRI_CreateRecursiveDirectory (char const* path,
     p++;
   }
 
-  if (res == TRI_ERROR_NO_ERROR && (p - s > 0)) {
+  if (((res == TRI_ERROR_FILE_EXISTS) ||
-    m = TRI_MKDIR(copy, 0777);
+       (res == TRI_ERROR_NO_ERROR))
-
+      && (p - s > 0)) {
-    if (m != 0 && errno != EEXIST) {
+    res = TRI_CreateDirectory(copy, systemError, systemErrorStr);
-      res = errno;
-    }
   }
 
   TRI_Free(TRI_CORE_MEM_ZONE, copy);
@@ -566,10 +557,9 @@ int TRI_CreateRecursiveDirectory (char const* path,
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief creates a directory
 ////////////////////////////////////////////////////////////////////////////////
-
 int TRI_CreateDirectory (char const* path,
-                         long *systemError,
+                         long &systemError,
-                         std::string *systemErrorStr) {
+                         std::string &systemErrorStr) {
   TRI_ERRORBUF;
   int res;
 
@@ -583,9 +573,7 @@ int TRI_CreateDirectory (char const* path,
     TRI_SYSTEM_ERROR();
     res = errno;
     if (res != TRI_ERROR_NO_ERROR) {
-      if (systemErrorStr != nullptr) {
+      systemErrorStr = std::string("Failed to create directory [") + path + "] " + TRI_GET_ERRORBUF;
-        *systemErrorStr = std::string("Failed to create directory [") + path + "] " + TRI_GET_ERRORBUF;
-      }
 #ifndef _WIN32
       if (res == ENOENT) {
         res = TRI_ERROR_FILE_NOT_FOUND;
@@ -607,13 +595,11 @@ int TRI_CreateDirectory (char const* path,
         res = TRI_ERROR_FORBIDDEN;
       }
 #endif
-      if (systemError != nullptr) {
-        *systemError = errno;
-      }
       else {
-        // an unknown error type. will be translated into system error below
         res = TRI_ERROR_NO_ERROR;
       }
+      systemError = errno;
+        // an unknown error type. will be translated into system error below
     }
 
     // if errno doesn't indicate an error, return a system error
@@ -2034,6 +2020,8 @@ int TRI_GetTempName (char const* directory,
   char* dir;
   char* temp;
   int tries;
+  int long systemError = 0;
+  std::string errorMessage;
 
   temp = TRI_GetUserTempPath();
 
@@ -2049,7 +2037,7 @@ int TRI_GetTempName (char const* directory,
   // remove trailing PATH_SEPARATOR
   RemoveTrailingSeparator(dir);
 
-  TRI_CreateRecursiveDirectory(dir);
+  TRI_CreateRecursiveDirectory(dir, systemError, errorMessage); /// TODO: errormessage!
 
   if (! TRI_IsDirectory(dir)) {
     TRI_Free(TRI_CORE_MEM_ZONE, dir);

lib/Basics/files.h

@@ -97,16 +97,16 @@ int TRI_MTimeFile (char const* path, int64_t* mtime);
 ////////////////////////////////////////////////////////////////////////////////
 
 int TRI_CreateRecursiveDirectory (char const* path,
-                                  long *systemError = nullptr,
+                                  long &systemError,
-                                  std::string *systemErrorStr = nullptr);
+                                  std::string &systemErrorStr);
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief creates a directory
 ////////////////////////////////////////////////////////////////////////////////
 
 int TRI_CreateDirectory (char const* path,
-                         long *systemError = nullptr,
+                         long &systemError,
-                         std::string *systemErrorStr = nullptr);
+                         std::string &systemErrorStr);
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief removes an empty directory

lib/Basics/tri-zip.cpp

@@ -53,16 +53,21 @@ static int ExtractCurrentFile (unzFile uf,
                                const char* outPath,
                                const bool skipPaths,
                                const bool overwrite,
-                               const char* password) {
+                               const char* password,
+                               std::string &errorMessage) {
   char filenameInZip[256];
   char* filenameWithoutPath;
   char* fullPath;
   char* p;
   FILE *fout;
   unz_file_info64 fileInfo;
+  long systemError;
+  int err;
 
   filenameInZip[0] = '\0';
-  if (unzGetCurrentFileInfo64(uf, &fileInfo, filenameInZip, sizeof(filenameInZip), NULL, 0, NULL, 0) != UNZ_OK) {
+  err = unzGetCurrentFileInfo64(uf, &fileInfo, filenameInZip, sizeof(filenameInZip), NULL, 0, NULL, 0);
+  if(err != UNZ_OK) {
+    errorMessage = std::string("Failed to get file info for ") + filenameInZip + ": " + std::to_string(err);
     return TRI_ERROR_INTERNAL;
   }
 
@@ -96,7 +101,12 @@ static int ExtractCurrentFile (unzFile uf,
   if (*filenameWithoutPath == '\0') {
     if (! skipPaths) {
       fullPath = TRI_Concatenate2File(outPath, filenameInZip);
-      TRI_CreateRecursiveDirectory(fullPath);
+      err = TRI_CreateRecursiveDirectory(fullPath, systemError, errorMessage);
+      if ((err != TRI_ERROR_NO_ERROR) && 
+          (err != TRI_ERROR_FILE_EXISTS) ) {
+        TRI_Free(TRI_CORE_MEM_ZONE, fullPath);
+        return err;
+      }
       TRI_Free(TRI_CORE_MEM_ZONE, fullPath);
     }
   }
@@ -112,7 +122,9 @@ static int ExtractCurrentFile (unzFile uf,
       writeFilename = filenameWithoutPath;
     }
 
-    if (unzOpenCurrentFilePassword(uf, password) != UNZ_OK) {
+    err = unzOpenCurrentFilePassword(uf, password);
+    if (err != UNZ_OK) {
+      errorMessage = "failed to authenticate the password in the zip: " + std::to_string(err);
       return TRI_ERROR_INTERNAL;
     }
 
@@ -121,6 +133,7 @@ static int ExtractCurrentFile (unzFile uf,
 
     if (! overwrite && TRI_ExistsFile(fullPath)) {
       TRI_Free(TRI_CORE_MEM_ZONE, fullPath);
+      errorMessage = std::string("not allowed to overwrite file ") + fullPath;
       return TRI_ERROR_CANNOT_OVERWRITE_FILE;
     }
 
@@ -128,13 +141,18 @@ static int ExtractCurrentFile (unzFile uf,
     fout = fopen(fullPath, "wb");
 
     // cannot write to outfile. this may be due to the target directory missing
-    if (fout == NULL && ! skipPaths && filenameWithoutPath != (char*) filenameInZip) {
+    if (fout == nullptr && ! skipPaths && filenameWithoutPath != (char*) filenameInZip) {
       char c = *(filenameWithoutPath - 1);
       *(filenameWithoutPath - 1) = '\0';
 
       // create target directory recursively
       char* d = TRI_Concatenate2File(outPath, filenameInZip);
-      TRI_CreateRecursiveDirectory(d);
+      err = TRI_CreateRecursiveDirectory(d, systemError, errorMessage);
+      if ((err != TRI_ERROR_NO_ERROR) && 
+          (err != TRI_ERROR_FILE_EXISTS) ) {
+        TRI_Free(TRI_CORE_MEM_ZONE, d);
+        return err;
+      }
       TRI_Free(TRI_CORE_MEM_ZONE, d);
 
       *(filenameWithoutPath - 1) = c;
@@ -148,15 +166,22 @@ static int ExtractCurrentFile (unzFile uf,
       // strip filename so we only have the directory name
       char* dir = TRI_Dirname(d);
       TRI_Free(TRI_CORE_MEM_ZONE, d);
-      TRI_CreateRecursiveDirectory(dir);
+      err = TRI_CreateRecursiveDirectory(dir, systemError, errorMessage);
+      if ((err != TRI_ERROR_NO_ERROR) && 
+          (err != TRI_ERROR_FILE_EXISTS) ) {
+        TRI_Free(TRI_CORE_MEM_ZONE, d);
+        TRI_Free(TRI_CORE_MEM_ZONE, dir);
+        return err;
+      }
+      TRI_Free(TRI_CORE_MEM_ZONE, d);
       TRI_Free(TRI_CORE_MEM_ZONE, dir);
       // try again
       fout = fopen(fullPath, "wb");
     }
 
-    TRI_Free(TRI_CORE_MEM_ZONE, fullPath);
-
     if (fout == NULL) {
+      errorMessage = std::string("failed to open file for writing: ") + fullPath + " - " + strerror(errno);
+      TRI_Free(TRI_CORE_MEM_ZONE, fullPath);
       return TRI_ERROR_CANNOT_WRITE_FILE;
     }
 
@@ -164,15 +189,18 @@ static int ExtractCurrentFile (unzFile uf,
       int result = unzReadCurrentFile(uf, buffer, (unsigned int) bufferSize);
 
       if (result < 0) {
+        errorMessage = std::string("failed to write file ") + fullPath + " - " + strerror(errno);
         fclose(fout);
-
+        TRI_Free(TRI_CORE_MEM_ZONE, fullPath);
         return TRI_ERROR_CANNOT_WRITE_FILE;
       }
 
       if (result > 0) {
         if (fwrite(buffer, result, 1, fout) != 1) {
+    
+          errorMessage = std::string("failed to write file ") + fullPath + " - " + strerror(errno);
+          TRI_Free(TRI_CORE_MEM_ZONE, fullPath);
           fclose(fout);
-
           return TRI_set_errno(TRI_ERROR_SYS_ERROR);
         }
       }
@@ -181,6 +209,7 @@ static int ExtractCurrentFile (unzFile uf,
         break;
       }
     }
+    TRI_Free(TRI_CORE_MEM_ZONE, fullPath);
 
     fclose(fout);
   }
@@ -200,24 +229,33 @@ static int UnzipFile (unzFile uf,
                       const char* outPath,
                       const bool skipPaths,
                       const bool overwrite,
-                      const char* password) {
+                      const char* password,
+                      std::string &errorMessage) {
   unz_global_info64 gi;
   uLong i;
   int res = TRI_ERROR_NO_ERROR;
+  int err;
 
-  if (unzGetGlobalInfo64(uf, &gi) != UNZ_OK) {
+  err = unzGetGlobalInfo64(uf, &gi);
+  if(err!= UNZ_OK) {
+    errorMessage = "Failed to get info: " + std::to_string(err);
     return TRI_ERROR_INTERNAL;
   }
 
   for (i = 0; i < gi.number_entry; i++) {
-    res = ExtractCurrentFile(uf, buffer, bufferSize, outPath, skipPaths, overwrite, password);
+    res = ExtractCurrentFile(uf, buffer, bufferSize, outPath, skipPaths, overwrite, password, errorMessage);
 
     if (res != TRI_ERROR_NO_ERROR) {
       break;
     }
 
     if ((i + 1) < gi.number_entry) {
-      if (unzGoToNextFile(uf) != UNZ_OK) {
+      err = unzGoToNextFile(uf);
+      if (err == UNZ_END_OF_LIST_OF_FILE) {
+        break;
+      }
+      else if (err != UNZ_OK) {
+        errorMessage = "Failed to jump to next file: " + std::to_string(err);
         res = TRI_ERROR_INTERNAL;
         break;
       }
@@ -386,7 +424,8 @@ int TRI_UnzipFile (const char* filename,
                    const char* outPath,
                    const bool skipPaths,
                    const bool overwrite,
-                   const char* password) {
+                   const char* password,
+                   std::string &errorMessage) {
   unzFile uf;
 #ifdef USEWIN32IOAPI
   zlib_filefunc64_def ffunc;
@@ -409,8 +448,12 @@ int TRI_UnzipFile (const char* filename,
 #else
   uf = unzOpen64(filename);
 #endif
+  if (uf == nullptr) {
+    errorMessage = std::string("unable to open zip file ") + filename;
+    return TRI_ERROR_INTERNAL;
+  }
 
-  res = UnzipFile(uf, buffer, bufferSize, outPath, skipPaths, overwrite, password);
+  res = UnzipFile(uf, buffer, bufferSize, outPath, skipPaths, overwrite, password, errorMessage);
 
   unzClose(uf);
 

lib/Basics/tri-zip.h

@@ -58,7 +58,8 @@ int TRI_UnzipFile (const char*,
                    const char*,
                    const bool,
                    const bool,
-                   const char*);
+                   const char*,
+                   std::string &errorMessage);
 
 #endif
 

lib/V8/v8-utils.cpp

@@ -1400,9 +1400,9 @@ static void JS_MakeDirectory (const v8::FunctionCallbackInfo<v8::Value>& args) {
   if (*name == nullptr) {
     TRI_V8_THROW_TYPE_ERROR("<path> must be a string");
   }
-
+  long systemError = 0;
   std::string systemErrorStr;
-  int res = TRI_CreateDirectory(*name, nullptr, &systemErrorStr);
+  int res = TRI_CreateDirectory(*name, systemError, systemErrorStr);
 
   if (res != TRI_ERROR_NO_ERROR) {
     TRI_V8_THROW_EXCEPTION_MESSAGE(res, systemErrorStr);
@@ -1452,14 +1452,14 @@ static void JS_UnzipFile (const v8::FunctionCallbackInfo<v8::Value>& args) {
     password = TRI_ObjectToString(args[4]);
     p = password.c_str();
   }
-
+  std::string errMsg;
-  int res = TRI_UnzipFile(filename.c_str(), outPath.c_str(), skipPaths, overwrite, p);
+  int res = TRI_UnzipFile(filename.c_str(), outPath.c_str(), skipPaths, overwrite, p, errMsg);
 
   if (res == TRI_ERROR_NO_ERROR) {
     TRI_V8_RETURN_TRUE();
   }
 
-  TRI_V8_THROW_EXCEPTION(res);
+  TRI_V8_THROW_EXCEPTION_MESSAGE(res, errMsg);
 }
 
 ////////////////////////////////////////////////////////////////////////////////