aws_fopen_safe() raises aws-error on failure (#970)

**Issue:**
`aws_fopen_safe()` and `aws_fopen()` did not raise an aws-error-code on failure. It expected the caller to check `errno`. This is super inconsistent, none of our other functions expect the caller to check `errno`. Also, the windows implementation never set `errno` in any circumstance.

**Description of changes:**
* `aws_fopen_safe()` and `aws_fopen()` always raise an aws-error-code if something goes wrong.
* aws_logging_init_noalloc() checks if file open failed. It wasn't checking before.

include/aws/common/error.h

@@ -129,7 +129,9 @@ AWS_COMMON_API
 void aws_unregister_error_info(const struct aws_error_info_list *error_info);
 
 /**
- * Convert a c library io error into an aws error.
+ * Convert a c library io error into an aws error, and raise it.
+ * If no conversion is found, AWS_ERROR_SYS_CALL_FAILURE is raised.
+ * Always returns AWS_OP_ERR.
  */
 AWS_COMMON_API
 int aws_translate_and_raise_io_error(int error_no);

include/aws/common/file.h

@@ -56,12 +56,15 @@ typedef bool(aws_on_directory_entry)(const struct aws_directory_entry *entry, vo
 AWS_EXTERN_C_BEGIN
 
 /**
- * Don't use this. It never should have been added in the first place. It's now deprecated.
+ * Deprecated - Use aws_fopen_safe() instead, avoid const char * in public APIs.
+ * Opens file at file_path using mode. Returns the FILE pointer if successful.
+ * Otherwise, aws_last_error() will contain the error that occurred
  */
 AWS_COMMON_API FILE *aws_fopen(const char *file_path, const char *mode);
 
 /**
  * Opens file at file_path using mode. Returns the FILE pointer if successful.
+ * Otherwise, aws_last_error() will contain the error that occurred
  */
 AWS_COMMON_API FILE *aws_fopen_safe(const struct aws_string *file_path, const struct aws_string *mode);
 

source/error.c

@@ -193,13 +193,17 @@ int aws_translate_and_raise_io_error(int error_no) {
         case EISDIR:
         case ENAMETOOLONG:
         case ENOENT:
+        case ENOTDIR:
             return aws_raise_error(AWS_ERROR_FILE_INVALID_PATH);
+        case EMFILE:
         case ENFILE:
             return aws_raise_error(AWS_ERROR_MAX_FDS_EXCEEDED);
         case ENOMEM:
             return aws_raise_error(AWS_ERROR_OOM);
         case ENOSPC:
             return aws_raise_error(AWS_ERROR_NO_SPACE);
+        case ENOTEMPTY:
+            return aws_raise_error(AWS_ERROR_DIRECTORY_NOT_EMPTY);
         default:
             return aws_raise_error(AWS_ERROR_SYS_CALL_FAILURE);
     }

source/file.c

@@ -13,7 +13,14 @@
 
 FILE *aws_fopen(const char *file_path, const char *mode) {
     if (!file_path || strlen(file_path) == 0) {
-        AWS_LOGF_ERROR(AWS_LS_COMMON_IO, "static: Failed to open file %s", file_path);
+        AWS_LOGF_ERROR(AWS_LS_COMMON_IO, "static: Failed to open file. path is empty");
+        aws_raise_error(AWS_ERROR_FILE_INVALID_PATH);
+        return NULL;
+    }
+
+    if (!mode || strlen(mode) == 0) {
+        AWS_LOGF_ERROR(AWS_LS_COMMON_IO, "static: Failed to open file. mode is empty");
+        aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
         return NULL;
     }
 
@@ -70,12 +77,7 @@ int aws_byte_buf_init_from_file(struct aws_byte_buf *out_buf, struct aws_allocat
         return AWS_OP_SUCCESS;
     }
 
-    AWS_LOGF_ERROR(AWS_LS_COMMON_IO, "static: Failed to open file %s with errno %d", filename, errno);
-
-    if (errno == 0) {
-        return aws_raise_error(AWS_ERROR_FILE_INVALID_PATH);
-    }
-    return aws_translate_and_raise_io_error(errno);
+    return AWS_OP_ERR;
 }
 
 bool aws_is_any_directory_separator(char value) {

source/log_writer.c

@@ -76,7 +76,7 @@ static int s_aws_file_writer_init_internal(
         impl->log_file = aws_fopen(file_name_to_open, "a+");
         if (impl->log_file == NULL) {
             aws_mem_release(allocator, impl);
-            return aws_translate_and_raise_io_error(errno);
+            return AWS_OP_ERR;
         }
         impl->close_file_on_cleanup = true;
     } else {

source/logging.c

@@ -561,6 +561,10 @@ int aws_logger_init_noalloc(
     } else { /* _MSC_VER */
         if (options->filename != NULL) {
             impl->file = aws_fopen(options->filename, "w");
+            if (!impl->file) {
+                aws_mem_release(allocator, impl);
+                return AWS_OP_ERR;
+            }
             impl->should_close = true;
         } else {
             impl->file = stderr;

source/posix/file.c

@@ -15,39 +15,28 @@
 #include <unistd.h>
 
 FILE *aws_fopen_safe(const struct aws_string *file_path, const struct aws_string *mode) {
-    return fopen(aws_string_c_str(file_path), aws_string_c_str(mode));
-}
-
-static int s_parse_and_raise_error(int errno_cpy) {
-    if (errno_cpy == 0) {
-        return AWS_OP_SUCCESS;
-    }
-
-    if (errno_cpy == ENOENT || errno_cpy == ENOTDIR) {
-        return aws_raise_error(AWS_ERROR_FILE_INVALID_PATH);
-    }
-
-    if (errno_cpy == EMFILE || errno_cpy == ENFILE) {
-        return aws_raise_error(AWS_ERROR_MAX_FDS_EXCEEDED);
+    FILE *f = fopen(aws_string_c_str(file_path), aws_string_c_str(mode));
+    if (!f) {
+        int errno_cpy = errno;
+        aws_translate_and_raise_io_error(errno_cpy);
+        AWS_LOGF_ERROR(
+            AWS_LS_COMMON_IO,
+            "static: Failed to open file. path:'%s' mode:'%s' errno:%d aws-error:%d(%s)",
+            aws_string_c_str(file_path),
+            aws_string_c_str(mode),
+            errno_cpy,
+            aws_last_error(),
+            aws_error_name(aws_last_error()));
     }
-
-    if (errno_cpy == EACCES) {
-        return aws_raise_error(AWS_ERROR_NO_PERMISSION);
-    }
-
-    if (errno_cpy == ENOTEMPTY) {
-        return aws_raise_error(AWS_ERROR_DIRECTORY_NOT_EMPTY);
-    }
-
-    return aws_raise_error(AWS_ERROR_UNKNOWN);
+    return f;
 }
 
 int aws_directory_create(const struct aws_string *dir_path) {
     int mkdir_ret = mkdir(aws_string_c_str(dir_path), S_IRWXU | S_IRWXG | S_IRWXO);
 
     /** nobody cares if it already existed. */
     if (mkdir_ret != 0 && errno != EEXIST) {
-        return s_parse_and_raise_error(errno);
+        return aws_translate_and_raise_io_error(errno);
     }
 
     return AWS_OP_SUCCESS;
@@ -104,13 +93,13 @@ int aws_directory_delete(const struct aws_string *dir_path, bool recursive) {
 
     int error_code = rmdir(aws_string_c_str(dir_path));
 
-    return error_code == 0 ? AWS_OP_SUCCESS : s_parse_and_raise_error(errno);
+    return error_code == 0 ? AWS_OP_SUCCESS : aws_translate_and_raise_io_error(errno);
 }
 
 int aws_directory_or_file_move(const struct aws_string *from, const struct aws_string *to) {
     int error_code = rename(aws_string_c_str(from), aws_string_c_str(to));
 
-    return error_code == 0 ? AWS_OP_SUCCESS : s_parse_and_raise_error(errno);
+    return error_code == 0 ? AWS_OP_SUCCESS : aws_translate_and_raise_io_error(errno);
 }
 
 int aws_file_delete(const struct aws_string *file_path) {
@@ -120,7 +109,7 @@ int aws_file_delete(const struct aws_string *file_path) {
         return AWS_OP_SUCCESS;
     }
 
-    return s_parse_and_raise_error(errno);
+    return aws_translate_and_raise_io_error(errno);
 }
 
 int aws_directory_traverse(
@@ -132,7 +121,7 @@ int aws_directory_traverse(
     DIR *dir = opendir(aws_string_c_str(path));
 
     if (!dir) {
-        return s_parse_and_raise_error(errno);
+        return aws_translate_and_raise_io_error(errno);
     }
 
     struct aws_byte_cursor current_path = aws_byte_cursor_from_string(path);

source/windows/file.c

@@ -5,6 +5,7 @@
 
 #include <aws/common/environment.h>
 #include <aws/common/file.h>
+#include <aws/common/logging.h>
 #include <aws/common/string.h>
 
 #include <Shlwapi.h>
@@ -22,7 +23,15 @@ static bool s_is_wstring_empty(const struct aws_wstring *str) {
 }
 
 FILE *aws_fopen_safe(const struct aws_string *file_path, const struct aws_string *mode) {
-    if (s_is_string_empty(file_path) || s_is_string_empty(mode)) {
+    if (s_is_string_empty(file_path)) {
+        AWS_LOGF_ERROR(AWS_LS_COMMON_IO, "static: Failed to open file. path is empty");
+        aws_raise_error(AWS_ERROR_FILE_INVALID_PATH);
+        return NULL;
+    }
+
+    if (s_is_string_empty(mode)) {
+        AWS_LOGF_ERROR(AWS_LS_COMMON_IO, "static: Failed to open file. mode is empty");
+        aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
         return NULL;
     }
 
@@ -31,12 +40,19 @@ FILE *aws_fopen_safe(const struct aws_string *file_path, const struct aws_string
 
     FILE *file = NULL;
     errno_t error = _wfopen_s(&file, aws_wstring_c_str(w_file_path), aws_wstring_c_str(w_mode));
-    /* actually handle the error correctly here. */
     aws_wstring_destroy(w_mode);
     aws_wstring_destroy(w_file_path);
 
     if (error) {
-        aws_raise_error(AWS_ERROR_FILE_INVALID_PATH);
+        aws_translate_and_raise_io_error(error);
+        AWS_LOGF_ERROR(
+            AWS_LS_COMMON_IO,
+            "static: Failed to open file. path:'%s' mode:'%s' errno:%d aws-error:%d(%s)",
+            aws_string_c_str(file_path),
+            aws_string_c_str(mode),
+            error,
+            aws_last_error(),
+            aws_error_name(aws_last_error()));
     }
 
     return file;