Refactor the status error message builder. (#9546)

Author: ysiraichi

torch_xla/csrc/init_python_bindings.cpp

@@ -3378,13 +3378,16 @@ void InitXlaModuleBindings(py::module m) {
              absl::StatusOr<std::vector<absl_nonnull XLATensorPtr>>
                  xtensors_status = bridge::GetXlaTensors(tensors);
              ABSL_CHECK(xtensors_status.ok())
-                 << "_get_graph_hash(): error retrieving the XLA tensors from "
-                 << "the given tensor arguments. "
+                 << "\n\n"
+                 << "Internal Error:\n"
+                 << "    _get_graph_hash(): error retrieving the XLA tensors "
+                    "from the given tensor arguments. "
                  << "This is a bug! Please, open an issue in the PyTorch/XLA "
                  << "GitHub repository: https://github.com/pytorch/xla"
-                 << std::endl
-                 << "Status Error: "
-                 << BuildStatusErrorMessage(xtensors_status.status());
+                 << "\n\n"
+                 << "Status Error:\n"
+                 << "    " << BuildStatusErrorMessage(xtensors_status.status())
+                 << "\n";
              std::vector<absl_nonnull XLATensorPtr> xtensors =
                  xtensors_status.value();
              torch::lazy::hash_t hash =

torch_xla/csrc/status.cpp

@@ -103,29 +103,23 @@ static std::string GetFormattedStatusPropagationTrace(
   auto status_propagation_trace = GetStatusPropagationTraceOrEmpty(status);
   return status_propagation_trace.empty()
              ? ""
-             : absl::StrCat("\nStatus Propagation Trace:",
-                            status_propagation_trace.Flatten(), "\n");
-}
-
-// Get the status message followed by a line break, if we are printing the
-// C++ stacktraces.
-//
-// This is needed so we have a blank line in between the status message and
-// the dumped C++ traces (either the status propagation one, or the C++
-// stacktrace).
-static std::string MaybeGetMessageWithLineBreak(const absl::Status& status) {
-  return torch::get_cpp_stacktraces_enabled()
-             ? absl::StrCat(status.message(), "\n")
-             : std::string(status.message());
+             : absl::StrCat("\n\nStatus Propagation Trace:",
+                            status_propagation_trace.Flatten());
 }
 
 std::string BuildStatusErrorMessage(const absl::Status& status) {
-  return absl::StrCat(MaybeGetMessageWithLineBreak(status),
+  return absl::StrCat(status.message(),
                       GetFormattedStatusPropagationTrace(status));
 }
 
+// Return a line break if torch::get_cpp_stacktraces_enabled() is true.
+static std::string LineBreakIfCppStacktracesEnabled() {
+  return torch::get_cpp_stacktraces_enabled() ? "\n" : "";
+}
+
 void MaybeThrow(const absl::Status& status) {
-  TORCH_CHECK(status.ok(), BuildStatusErrorMessage(status));
+  TORCH_CHECK(status.ok(), absl::StrCat(BuildStatusErrorMessage(status),
+                                        LineBreakIfCppStacktracesEnabled()));
 }
 
 void GetValueOrThrow(const absl::Status& status) { MaybeThrow(status); }

torch_xla/csrc/status.h

@@ -179,10 +179,7 @@ absl::Status MaybeWithNewMessage(const absl::Status& status, const char* file,
 // If `TORCH_SHOW_CPP_STACKTRACES` is enabled, returns the concatenation of
 // `status.message()` with its inner status propagation trace.
 //
-// TODO(ysiraichi): this call does not append the C++ stacktrace, which,
-// ideally, should. It can be done by not using `TORCH_CHECK()` macro directly
-// in `MaybeThrow()`, but using PyTorch `c10::get_lazy_backtrace()`
-// (at c10/util/Backtrace.h).
+// It doesn't add a trailing line break.
 std::string BuildStatusErrorMessage(const absl::Status& status);
 
 // Maybe throws an exception if `status` has a non-ok code.