Windows,JNI,logging: improve error messages

In this commit:
- introduce the MakeErrorMessage function, which
  creates a structured error message with file
  and line information of the error's origin
- update all error messages in the Windows JNI
  library
- simplify GetLastErrorMessage to just convert an
  error code to string, without prepending a cause

Change-Id: Ia8162bfdaee37d4b7ccb3a46d6c8a861b0a1bd94
PiperOrigin-RevId: 173402968

7 files changed, 199 insertions, 114 deletions

diff --git a/src/main/native/windows/file-jni.cc b/src/main/native/windows/file-jni.cc
index 7c4f599641..b2c809efeb 100644
--- a/src/main/native/windows/file-jni.cc
+++ b/src/main/native/windows/file-jni.cc
@@ -17,6 +17,7 @@
 #include <windows.h>
 
 #include <memory>
+#include <sstream>
 #include <string>
 
 #include "src/main/native/jni.h"
@@ -24,26 +25,30 @@
 #include "src/main/native/windows/jni-util.h"
 #include "src/main/native/windows/util.h"
 
-static void MaybeReportLastError(const std::wstring& reason, JNIEnv* env,
-                                 jobjectArray error_msg_holder) {
-  if (error_msg_holder != nullptr &&
-      env->GetArrayLength(error_msg_holder) > 0) {
-    std::wstring error_str = bazel::windows::GetLastErrorString(reason);
-    jstring error_msg = env->NewString(
-        reinterpret_cast<const jchar*>(error_str.c_str()), error_str.size());
-    env->SetObjectArrayElement(error_msg_holder, 0, error_msg);
-  }
+static bool CanReportError(JNIEnv* env, jobjectArray error_msg_holder) {
+  return error_msg_holder != nullptr &&
+         env->GetArrayLength(error_msg_holder) > 0;
+}
+
+static void ReportLastError(const std::wstring& error_str, JNIEnv* env,
+                            jobjectArray error_msg_holder) {
+  jstring error_msg = env->NewString(
+      reinterpret_cast<const jchar*>(error_str.c_str()), error_str.size());
+  env->SetObjectArrayElement(error_msg_holder, 0, error_msg);
 }
 
 extern "C" JNIEXPORT jint JNICALL
 Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeIsJunction(
     JNIEnv* env, jclass clazz, jstring path, jobjectArray error_msg_holder) {
-  int result = bazel::windows::IsJunctionOrDirectorySymlink(
-      bazel::windows::GetJavaWstring(env, path).c_str());
-  if (result == bazel::windows::IS_JUNCTION_ERROR) {
-    MaybeReportLastError(std::wstring(L"GetFileAttributes(") +
-                             bazel::windows::GetJavaWstring(env, path) + L")",
-                         env, error_msg_holder);
+  std::wstring wpath(bazel::windows::GetJavaWstring(env, path));
+  int result = bazel::windows::IsJunctionOrDirectorySymlink(wpath.c_str());
+  if (result == bazel::windows::IS_JUNCTION_ERROR &&
+      CanReportError(env, error_msg_holder)) {
+    DWORD err_code = GetLastError();
+    ReportLastError(
+        bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                                         L"nativeIsJunction", wpath, err_code),
+        env, error_msg_holder);
   }
   return result;
 }
@@ -53,13 +58,13 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeGetLo
     JNIEnv* env, jclass clazz, jstring path, jobjectArray result_holder,
     jobjectArray error_msg_holder) {
   std::unique_ptr<WCHAR[]> result;
-  std::wstring err_msg(bazel::windows::GetLongPath(
-      bazel::windows::GetJavaWstring(env, path).c_str(), &result));
-  if (!err_msg.empty()) {
-    MaybeReportLastError(std::wstring(L"GetLongPathName(") +
-                             bazel::windows::GetJavaWstring(env, path) +
-                             L"): " + err_msg,
-                         env, error_msg_holder);
+  std::wstring wpath(bazel::windows::GetJavaWstring(env, path));
+  std::wstring error(bazel::windows::GetLongPath(wpath.c_str(), &result));
+  if (!error.empty() && CanReportError(env, error_msg_holder)) {
+    ReportLastError(
+        bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                                         L"nativeGetLongPath", wpath, error),
+        env, error_msg_holder);
     return JNI_FALSE;
   }
   env->SetObjectArrayElement(
@@ -73,11 +78,14 @@ extern "C" JNIEXPORT jboolean JNICALL
 Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeCreateJunction(
     JNIEnv* env, jclass clazz, jstring name, jstring target,
     jobjectArray error_msg_holder) {
-  std::wstring error(bazel::windows::CreateJunction(
-      bazel::windows::GetJavaWstring(env, name),
-      bazel::windows::GetJavaWstring(env, target)));
-  if (!error.empty()) {
-    MaybeReportLastError(error, env, error_msg_holder);
+  std::wstring wname(bazel::windows::GetJavaWstring(env, name));
+  std::wstring wtarget(bazel::windows::GetJavaWstring(env, target));
+  std::wstring error(bazel::windows::CreateJunction(wname, wtarget));
+  if (!error.empty() && CanReportError(env, error_msg_holder)) {
+    ReportLastError(bazel::windows::MakeErrorMessage(
+                        WSTR(__FILE__), __LINE__, L"nativeCreateJunction",
+                        wname + L", " + wtarget, error),
+                    env, error_msg_holder);
     return JNI_FALSE;
   }
   return JNI_TRUE;
diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc
index d694c3c0e2..e9d7c18f5b 100644
--- a/src/main/native/windows/file.cc
+++ b/src/main/native/windows/file.cc
@@ -45,7 +45,9 @@ int IsJunctionOrDirectorySymlink(const WCHAR* path) {
 wstring GetLongPath(const WCHAR* path, unique_ptr<WCHAR[]>* result) {
   DWORD size = ::GetLongPathNameW(path, NULL, 0);
   if (size == 0) {
-    return GetLastErrorString(L"GetLongPathNameW");
+    DWORD err_code = GetLastError();
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"GetLongPathNameW", path,
+                            err_code);
   }
   result->reset(new WCHAR[size]);
   ::GetLongPathNameW(path, result->get(), size);
@@ -113,10 +115,8 @@ wstring CreateJunction(const wstring& junction_name,
        /* two copies of the string are stored */ 2) /
       sizeof(WCHAR);
   if (target.size() > kMaxJunctionTargetLen) {
-    std::wstringstream error;
-    error << L"junction target is too long (" << target.size()
-          << L" characters, limit: " << kMaxJunctionTargetLen << L")";
-    return error.str();
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateJunction", target,
+                            L"target is too long");
   }
   const wstring name = HasUncPrefix(junction_name.c_str())
                            ? junction_name
@@ -124,12 +124,16 @@ wstring CreateJunction(const wstring& junction_name,
 
   // Junctions are directories, so create one
   if (!::CreateDirectoryW(name.c_str(), NULL)) {
-    return wstring(L"CreateDirectoryW failed");
+    DWORD err_code = GetLastError();
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateJunction", name,
+                            err_code);
   }
 
   AutoHandle handle(OpenDirectory(name.c_str(), true));
   if (!handle.IsValid()) {
-    return wstring(L"OpenDirectory failed");
+    DWORD err_code = GetLastError();
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"OpenDirectory", name,
+                            err_code);
   }
 
   uint8_t reparse_buffer_bytes[MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
@@ -177,7 +181,9 @@ wstring CreateJunction(const wstring& junction_name,
                          reparse_buffer->header.ReparseDataLength +
                              sizeof(JunctionDescription::Header),
                          NULL, 0, &bytes_returned, NULL)) {
-    return wstring(L"DeviceIoControl(FSCTL_SET_REPARSE_POINT) failed");
+    DWORD err_code = GetLastError();
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"DeviceIoControl", L"",
+                            err_code);
   }
   return L"";
 }
diff --git a/src/main/native/windows/processes-jni.cc b/src/main/native/windows/processes-jni.cc
index 78ba8d2b46..27d8b4548d 100644
--- a/src/main/native/windows/processes-jni.cc
+++ b/src/main/native/windows/processes-jni.cc
@@ -18,6 +18,7 @@
 
 #include <atomic>
 #include <memory>
+#include <sstream>
 #include <string>
 #include <type_traits>  // static_assert
 
@@ -25,6 +26,13 @@
 #include "src/main/native/windows/jni-util.h"
 #include "src/main/native/windows/util.h"
 
+template <typename T>
+static std::wstring ToString(const T& e) {
+  std::wstringstream s;
+  s << e;
+  return s.str();
+}
+
 extern "C" JNIEXPORT jint JNICALL
 Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetpid(
     JNIEnv* env, jclass clazz) {
@@ -125,12 +133,6 @@ static_assert(sizeof(jchar) == sizeof(WCHAR),
 
 static jlong PtrAsJlong(void* p) { return reinterpret_cast<jlong>(p); }
 
-static std::wstring AsExecutableForCreateProcess(JNIEnv* env, jstring path,
-                                                 std::wstring* result) {
-  return bazel::windows::AsExecutablePathForCreateProcess(
-      bazel::windows::GetJavaWstring(env, path), result);
-}
-
 // The following CreateProcessWithExplicitHandles function is from oldnewthing.
 // https://blogs.msdn.microsoft.com/oldnewthing/20111216-00/?p=8873
 // We need it to prevent unintended handles being inherited by child process,
@@ -206,9 +208,12 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
   NativeProcess* result = new NativeProcess();
 
   std::wstring argv0;
-  std::wstring error_msg(AsExecutableForCreateProcess(env, java_argv0, &argv0));
+  std::wstring wpath(bazel::windows::GetJavaWstring(env, java_argv0));
+  std::wstring error_msg(
+      bazel::windows::AsExecutablePathForCreateProcess(wpath, &argv0));
   if (!error_msg.empty()) {
-    result->error_ = error_msg;
+    result->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg);
     return PtrAsJlong(result);
   }
 
@@ -219,10 +224,11 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
   std::wstring stderr_redirect = AddUncPrefixMaybe(
       bazel::windows::GetJavaWstring(env, java_stderr_redirect));
   std::wstring cwd;
-  error_msg = bazel::windows::AsShortPath(
-      bazel::windows::GetJavaWstring(env, java_cwd), &cwd);
+  std::wstring wcwd(bazel::windows::GetJavaWstring(env, java_cwd));
+  error_msg = bazel::windows::AsShortPath(wcwd, &cwd);
   if (!error_msg.empty()) {
-    result->error_ = error_msg;
+    result->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg);
     return PtrAsJlong(result);
   }
 
@@ -250,18 +256,24 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
   JavaByteArray env_map(env, java_env);
   if (env_map.ptr() != nullptr) {
     if (env_map.size() < 2) {
-      result->error_ = L"The environment must be at least two bytes long";
+      result->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath,
+          L"the environment must be at least two bytes long");
       return PtrAsJlong(result);
     } else if (env_map.ptr()[env_map.size() - 1] != 0 ||
                env_map.ptr()[env_map.size() - 2] != 0) {
-      result->error_ = L"Environment array must end with two null bytes";
+      result->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath,
+          L"environment array must end with two null bytes");
       return PtrAsJlong(result);
     }
   }
 
   HANDLE temp_stdin_handle = INVALID_HANDLE_VALUE;
   if (!CreatePipe(&temp_stdin_handle, &result->stdin_, &sa, 0)) {
-    result->error_ = bazel::windows::GetLastErrorString(L"CreatePipe(stdin)");
+    DWORD err_code = GetLastError();
+    result->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
     CloseHandle(temp_stdin_handle);
     return PtrAsJlong(result);
   }
@@ -280,15 +292,17 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
         /* hTemplateFile */ NULL);
 
     if (!stdout_process.IsValid()) {
-      result->error_ =
-          bazel::windows::GetLastErrorString(L"CreateFile(stdout)");
+      DWORD err_code = GetLastError();
+      result->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
       return PtrAsJlong(result);
     }
   } else {
     HANDLE temp_stdout_handle = INVALID_HANDLE_VALUE;
     if (!CreatePipe(&result->stdout_.handle_, &temp_stdout_handle, &sa, 0)) {
-      result->error_ =
-          bazel::windows::GetLastErrorString(L"CreatePipe(stdout)");
+      DWORD err_code = GetLastError();
+      result->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
       CloseHandle(temp_stdout_handle);
       return PtrAsJlong(result);
     }
@@ -326,8 +340,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
           /* hTemplateFile */ NULL);
 
       if (stderr_handle == INVALID_HANDLE_VALUE) {
-        result->error_ =
-            bazel::windows::GetLastErrorString(L"CreateFile(stderr)");
+        DWORD err_code = GetLastError();
+        result->error_ = bazel::windows::MakeErrorMessage(
+            WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
         return PtrAsJlong(result);
       }
       // stderr_process != stdout_process, so set its handle, so the AutoHandle
@@ -336,8 +351,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
     }
   } else {
     if (!CreatePipe(&result->stderr_.handle_, &stderr_handle, &sa, 0)) {
-      result->error_ =
-          bazel::windows::GetLastErrorString(L"CreatePipe(stderr)");
+      DWORD err_code = GetLastError();
+      result->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
       return PtrAsJlong(result);
     }
     stderr_process = stderr_handle;
@@ -347,7 +363,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
   // allowed. Thus, we don't need to do any more setup here.
   HANDLE job = CreateJobObject(NULL, NULL);
   if (job == NULL) {
-    result->error_ = bazel::windows::GetLastErrorString(L"CreateJobObject()");
+    DWORD err_code = GetLastError();
+    result->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
     return PtrAsJlong(result);
   }
 
@@ -357,8 +375,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
       JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
   if (!SetInformationJobObject(job, JobObjectExtendedLimitInformation,
                                &job_info, sizeof(job_info))) {
-    result->error_ =
-        bazel::windows::GetLastErrorString(L"SetInformationJobObject()");
+    DWORD err_code = GetLastError();
+    result->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
     return PtrAsJlong(result);
   }
 
@@ -386,7 +405,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
       /* rgHandlesToInherit */ handlesToInherit);
 
   if (!ok) {
-    result->error_ = bazel::windows::GetLastErrorString(L"CreateProcess()");
+    DWORD err_code = GetLastError();
+    result->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
     return PtrAsJlong(result);
   }
 
@@ -405,15 +426,18 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc
       CloseHandle(result->job_);
       result->job_ = INVALID_HANDLE_VALUE;
     } else {
-      result->error_ =
-          bazel::windows::GetLastErrorString(L"AssignProcessToJobObject()");
+      DWORD err_code = GetLastError();
+      result->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
       return PtrAsJlong(result);
     }
   }
 
   // Now that we put the process in a new job object, we can start executing it
   if (ResumeThread(thread) == -1) {
-    result->error_ = bazel::windows::GetLastErrorString(L"ResumeThread()");
+    DWORD err_code = GetLastError();
+    result->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
     return PtrAsJlong(result);
   }
 
@@ -429,7 +453,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeWriteStdin
 
   JavaByteArray bytes(env, java_bytes);
   if (offset < 0 || length <= 0 || offset > bytes.size() - length) {
-    process->error_ = L"Array index out of bounds";
+    process->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeWriteStdin", ToString(process->pid_),
+        L"Array index out of bounds");
     return -1;
   }
 
@@ -437,7 +463,10 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeWriteStdin
 
   if (!::WriteFile(process->stdin_, bytes.ptr() + offset, length,
                    &bytes_written, NULL)) {
-    process->error_ = bazel::windows::GetLastErrorString(L"WriteFile()");
+    DWORD err_code = GetLastError();
+    process->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeWriteStdin", ToString(process->pid_),
+        err_code);
     bytes_written = -1;
   }
 
@@ -468,7 +497,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeReadStream
 
   JavaByteArray bytes(env, java_bytes);
   if (offset < 0 || length <= 0 || offset > bytes.size() - length) {
-    stream->error_ = L"Array index out of bounds";
+    stream->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeReadStream", L"",
+        L"Array index out of bounds");
     return -1;
   }
 
@@ -488,7 +519,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeReadStream
       stream->error_ = L"";
       bytes_read = 0;
     } else {
-      stream->error_ = bazel::windows::GetLastErrorString(L"ReadFile()");
+      DWORD err_code = GetLastError();
+      stream->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeReadStream", L"", err_code);
       bytes_read = -1;
     }
   } else {
@@ -504,8 +537,10 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetExitCod
   NativeProcess* process = reinterpret_cast<NativeProcess*>(process_long);
   DWORD exit_code;
   if (!GetExitCodeProcess(process->process_, &exit_code)) {
-    process->error_ =
-        bazel::windows::GetLastErrorString(L"GetExitCodeProcess()");
+    DWORD err_code = GetLastError();
+    process->error_ = bazel::windows::MakeErrorMessage(
+        WSTR(__FILE__), __LINE__, L"nativeGetExitCode", ToString(process->pid_),
+        err_code);
     return -1;
   }
 
@@ -537,8 +572,10 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeWaitFor(
       break;
 
     default:
-      process->error_ = L"WaitForMultipleObjects() returned unknown result";
-      result = 2;
+      DWORD err_code = GetLastError();
+      process->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeWaitFor", ToString(process->pid_),
+          err_code);
       break;
   }
 
@@ -566,14 +603,18 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeTerminate(
 
   if (process->job_ != INVALID_HANDLE_VALUE) {
     if (!TerminateJobObject(process->job_, exit_code)) {
-      process->error_ =
-          bazel::windows::GetLastErrorString(L"TerminateJobObject()");
+      DWORD err_code = GetLastError();
+      process->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeTerminate", ToString(process->pid_),
+          err_code);
       return JNI_FALSE;
     }
   } else if (process->process_ != INVALID_HANDLE_VALUE) {
     if (!TerminateProcess(process->process_, exit_code)) {
-      process->error_ =
-          bazel::windows::GetLastErrorString(L"TerminateProcess()");
+      DWORD err_code = GetLastError();
+      process->error_ = bazel::windows::MakeErrorMessage(
+          WSTR(__FILE__), __LINE__, L"nativeTerminate", ToString(process->pid_),
+          err_code);
       return JNI_FALSE;
     }
   }
diff --git a/src/main/native/windows/util.cc b/src/main/native/windows/util.cc
index 156b2e39c6..9ed930d1bf 100644
--- a/src/main/native/windows/util.cc
+++ b/src/main/native/windows/util.cc
@@ -28,11 +28,23 @@ namespace windows {
 using std::wstring;
 using std::wstringstream;
 
-wstring GetLastErrorString(const wstring& cause) {
-  return GetLastErrorString(cause, GetLastError());
+wstring MakeErrorMessage(const wchar_t* file, int line,
+                         const wchar_t* failed_func, const wstring& func_arg,
+                         const wstring& message) {
+  wstringstream result;
+  result << L"ERROR: " << file << L"(" << line << L"): " << failed_func << L"("
+         << func_arg << L"): " << message;
+  return result.str();
 }
 
-wstring GetLastErrorString(const wstring& cause, DWORD error_code) {
+wstring MakeErrorMessage(const wchar_t* file, int line,
+                         const wchar_t* failed_func, const wstring& func_arg,
+                         DWORD error_code) {
+  return MakeErrorMessage(file, line, failed_func, func_arg,
+                          GetLastErrorString(error_code));
+}
+
+wstring GetLastErrorString(DWORD error_code) {
   if (error_code == 0) {
     return L"";
   }
@@ -46,12 +58,13 @@ wstring GetLastErrorString(const wstring& cause, DWORD error_code) {
   if (size == 0) {
     wstringstream err;
     DWORD format_message_error = GetLastError();
-    err << cause << L": Error " << error_code
-        << L"; cannot format message due to error " << format_message_error;
+    err << L"Error code " << error_code
+        << L"; cannot format message due to error code "
+        << format_message_error;
     return err.str();
   }
 
-  wstring result(cause + L": " + message);
+  wstring result(message);
   HeapFree(GetProcessHeap(), LMEM_FIXED, message);
   return result;
 }
@@ -77,21 +90,26 @@ wstring AsShortPath(wstring path, wstring* result) {
     return L"";
   }
   if (path[0] == '"') {
-    return wstring(L"path should not be quoted");
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path,
+                            L"path should not be quoted");
   }
   if (IsSeparator(path[0])) {
-    return wstring(L"path='") + path + L"' is absolute";
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path,
+                            L"path is absolute without a drive letter");
   }
   if (Contains(path, L"/./") || Contains(path, L"\\.\\") ||
       Contains(path, L"/..") || Contains(path, L"\\..")) {
-    return wstring(L"path='") + path + L"' is not normalized";
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path,
+                            L"path is not normalized");
   }
   if (path.size() >= MAX_PATH && !HasSeparator(path)) {
-    return wstring(L"path='") + path + L"' is just a file name but too long";
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path,
+                            L"path is just a file name but too long");
   }
   if (HasSeparator(path) &&
       !(isalpha(path[0]) && path[1] == L':' && IsSeparator(path[2]))) {
-    return wstring(L"path='") + path + L"' is not an absolute path";
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path,
+                            L"path is not absolute");
   }
   // At this point we know the path is either just a file name (shorter than
   // MAX_PATH), or an absolute, normalized, Windows-style path (of any length).
@@ -120,14 +138,15 @@ wstring AsShortPath(wstring path, wstring* result) {
   WCHAR wshort[kMaxShortPath];
   DWORD wshort_size = ::GetShortPathNameW(wlong.c_str(), NULL, 0);
   if (wshort_size == 0) {
-    return GetLastErrorString(wstring(L"GetShortPathName failed (path=") +
-                              path + L")");
+    DWORD err_code = GetLastError();
+    wstring res = MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                                   L"GetShortPathNameW", wlong, err_code);
+    return res;
   }
 
   if (wshort_size >= kMaxShortPath) {
-    return wstring(
-               L"GetShortPathName would not shorten the path enough (path=") +
-           path + L")";
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"GetShortPathNameW",
+                            wlong, L"cannot shorten the path enough");
   }
   GetShortPathNameW(wlong.c_str(), wshort, kMaxShortPath);
   result->assign(wshort + 4);
@@ -136,16 +155,20 @@ wstring AsShortPath(wstring path, wstring* result) {
 
 wstring AsExecutablePathForCreateProcess(const wstring& path, wstring* result) {
   if (path.empty()) {
-    return L"path should not be empty";
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                            L"AsExecutablePathForCreateProcess", path,
+                            L"path should not be empty");
   }
   wstring error = AsShortPath(path, result);
-  if (error.empty()) {
-    // Quote the path in case it's something like "c:\foo\app name.exe".
-    // Do this unconditionally, there's no harm in quoting. Quotes are not
-    // allowed inside paths so we don't need to escape quotes.
-    QuotePath(*result, result);
+  if (!error.empty()) {
+    return MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                            L"AsExecutablePathForCreateProcess", path, error);
   }
-  return error;
+  // Quote the path in case it's something like "c:\foo\app name.exe".
+  // Do this unconditionally, there's no harm in quoting. Quotes are not
+  // allowed inside paths so we don't need to escape quotes.
+  QuotePath(*result, result);
+  return L"";
 }
 
 }  // namespace windows
diff --git a/src/main/native/windows/util.h b/src/main/native/windows/util.h
index 0069eeb9ab..730a1d46f9 100644
--- a/src/main/native/windows/util.h
+++ b/src/main/native/windows/util.h
@@ -49,8 +49,16 @@ struct AutoHandle {
   HANDLE handle_;
 };
 
-wstring GetLastErrorString(const wstring& cause);
-wstring GetLastErrorString(const wstring& cause, DWORD error_code);
+#define WSTR1(x) L##x
+#define WSTR(x) WSTR1(x)
+
+wstring MakeErrorMessage(const wchar_t* file, int line,
+                         const wchar_t* failed_func, const wstring& func_arg,
+                         const wstring& message);
+wstring MakeErrorMessage(const wchar_t* file, int line,
+                         const wchar_t* failed_func, const wstring& func_arg,
+                         DWORD error_code);
+wstring GetLastErrorString(DWORD error_code);
 
 // Same as `AsExecutablePathForCreateProcess` except it won't quote the result.
 wstring AsShortPath(wstring path, wstring* result);
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java
index 383b5c8ad1..2f7483a0fd 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java
@@ -115,7 +115,7 @@ public class WindowsFileOperationsTest {
       WindowsFileOperations.isJunction(root + "/non-existent");
       fail("expected to throw");
     } catch (IOException e) {
-      assertThat(e.getMessage()).contains("GetFileAttributes");
+      assertThat(e.getMessage()).contains("nativeIsJunction");
     }
     assertThat(Arrays.asList(new File(root + "/shrtpath/a").list())).containsExactly("file1.txt");
     assertThat(Arrays.asList(new File(root + "/shrtpath/b").list())).containsExactly("file2.txt");
diff --git a/src/test/native/windows/util_test.cc b/src/test/native/windows/util_test.cc
index 83177b92da..2a131e77b1 100644
--- a/src/test/native/windows/util_test.cc
+++ b/src/test/native/windows/util_test.cc
@@ -234,8 +234,8 @@ static void DeleteDirsUnder(const wstring& basedir,
                                 /* const WCHAR* */ error_msg)         \
   {                                                                   \
     wstring actual;                                                   \
-    ASSERT_CONTAINS(AsExecutablePathForCreateProcess(input, &actual), \
-                    error_msg);                                       \
+    wstring result(AsExecutablePathForCreateProcess(input, &actual)); \
+    ASSERT_CONTAINS(result, error_msg);                               \
   }
 
 // This is a macro so the assertions will have the correct line number.
@@ -249,13 +249,12 @@ static void DeleteDirsUnder(const wstring& basedir,
 
 TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessBadInputs) {
   ASSERT_SHORTENING_FAILS(L"", L"should not be empty");
-  ASSERT_SHORTENING_FAILS(L"\"cmd.exe\"", L"should not be quoted");
-  ASSERT_SHORTENING_FAILS(L"/dev/null", L"path='/dev/null' is absolute");
-  ASSERT_SHORTENING_FAILS(L"/usr/bin/bash",
-                          L"path='/usr/bin/bash' is absolute");
-  ASSERT_SHORTENING_FAILS(L"foo\\bar.exe", L"absolute");
-  ASSERT_SHORTENING_FAILS(L"foo\\..\\bar.exe", L"normalized");
-  ASSERT_SHORTENING_FAILS(L"\\bar.exe", L"path='\\bar.exe' is absolute");
+  ASSERT_SHORTENING_FAILS(L"\"cmd.exe\"", L"path should not be quoted");
+  ASSERT_SHORTENING_FAILS(L"/dev/null", L"path is absolute without a drive");
+  ASSERT_SHORTENING_FAILS(L"/usr/bin/bash", L"path is absolute without a");
+  ASSERT_SHORTENING_FAILS(L"foo\\bar.exe", L"path is not absolute");
+  ASSERT_SHORTENING_FAILS(L"foo\\..\\bar.exe", L"path is not normalized");
+  ASSERT_SHORTENING_FAILS(L"\\bar.exe", L"path is absolute");
 
   wstring dummy = L"hello";
   while (dummy.size() < MAX_PATH) {
@@ -295,7 +294,7 @@ TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessConversions) {
     if (i > 0) {
       ASSERT_EQ(::GetFileAttributesW(wfilename.c_str()),
                 INVALID_FILE_ATTRIBUTES);
-      ASSERT_SHORTENING_FAILS(wfilename.c_str(), L"GetShortPathName failed");
+      ASSERT_SHORTENING_FAILS(wfilename.c_str(), L"GetShortPathNameW");
     }
 
     // Create the file, now we should be able to shorten it when i=0, but not
@@ -309,7 +308,7 @@ TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessConversions) {
       // The wfilename was too long to begin with, and it was impossible to
       // shorten any of the segments (since we deliberately created them that
       // way), so shortening failed.
-      ASSERT_SHORTENING_FAILS(wfilename.c_str(), L"would not shorten");
+      ASSERT_SHORTENING_FAILS(wfilename.c_str(), L"cannot shorten the path");
     }
     DELETE_FILE(wfilename);
   }
@@ -327,7 +326,7 @@ TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessConversions) {
   ASSERT_GT(wshortenable.size(), MAX_PATH);
 
   // Attempt to shorten. It will fail because the file doesn't exist yet.
-  ASSERT_SHORTENING_FAILS(wshortenable, L"GetShortPathName failed");
+  ASSERT_SHORTENING_FAILS(wshortenable, L"GetShortPathNameW");
 
   // Create the file so shortening will succeed.
   CREATE_FILE(wshortenable);